From: grygorii.strashko@linaro•org (Grygorii.Strashko@linaro•org)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
Date: Mon, 09 Feb 2015 18:28:03 +0800 [thread overview]
Message-ID: <54D88BB3.4060104@linaro.org> (raw)
In-Reply-To: <54D874E9.1040302@ti.com>
Hi Kishon,
On 02/09/2015 04:50 PM, Kishon Vijay Abraham I wrote:
> On Tuesday 03 February 2015 09:21 PM, grygorii.strashko at linaro.org wrote:
>> From: Grygorii Strashko <grygorii.strashko@linaro•org>
>>
>> Now DRA7xx pcie1/2 hwmods define PRCM configuration as following:
>> .clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
>> .rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
>> .modulemode = MODULEMODE_SWCTRL,
>> which is completely wrong because DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET
>> is clockdomain ctrl register and NOT module ctrl register.
>> And they have diffrent allowed values for bits[0,1]:
>> CLKTRCTRL?????????MODULEMODE??????????????????????????????????
>> 0x0:?NO_SLEEP?????0x0:?Module?is?disabled?by?SW.??????????????
>> 0x1:?SW_SLEEP?????0x1:?Module?is?managed?automatically?by?HW??
>> 0x2:?SW_WKUP??????0x2:?Module?is?explicitly?enabled.??????????
>> 0x3:?HW_AUTO??????0x3:?Reserved??
>>
>> As result, following message can be seen during suspend:
>> "omap_hwmod: pcie1: _wait_target_disable failed"
>>
>> Fix it by removing .modulemode from pcie1/2 hwmods and, in that
>> way, prevent clockdomain ctrl register writing from HWMOD core.
>
> Looks correct except for one change.
>
> Acked-by: Kishon Vijay Abraham I <kishon@ti•com>
>>
>> Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro•org>
>> ---
>>
>> More over, it looks like pcie1/2 hwmods are fake and have to be dropped at all.
>> The real HWMODs are PCIESS1/2.
>
> Not sure I get this. You mean "dra7xx_pcie1_hwmod" should be replaced with
> "dra7xx_pciess1_hwmod"? Or you mean an entire new hwmod is missing?
>
> Please note we still have to enable the clock domain and main clock. We've also
> purposefully omitted sysconfig from hwmod data since pcie reset
> (RM_PCIESS_RSTCTRL) should be done before accessing the syconfig register and
> the infrastructure for that is currently not present.
What I'm trying to say is that now PM control data mixed between "pcieX" and "pcieX-phy" hwmods.
After this patch "pcieX" hwmods will actually do nothing (I assume that "pciex-phy" will be
enabled before "pcieX"), and probably can be removed if "pcie_clkdm" could be attached to "pcieX-phy" hwmod
instead.
More over, now, "pcie_clkdm" is connected to "pcieX" hwmod while MODULEMODE register is controlled
by "pciex-phy" hwmod, so when pciess is going to be enabled the "l3init_clkdm" will be waken-up by
hwmode core and not "pcie_clkdm" - as I can remember this is not good (we should alway wake-up clockdomain
and keep it in SWSUP mode while changing MODULEMODE and SYSC registers).
static struct omap_hwmod dra7xx_pcie1_hwmod = {
.name = "pcie1",
.class = &dra7xx_pcie_hwmod_class,
.clkdm_name = "pcie_clkdm",
.main_clk = "l4_root_clk_div",
static struct omap_hwmod dra7xx_pcie1_phy_hwmod = {
.name = "pcie1-phy",
.class = &dra7xx_pcie_phy_hwmod_class,
.clkdm_name = "l3init_clkdm",
.main_clk = "l4_root_clk_div",
So, in my opinion, some rework may be needed here.
Am I right?
>
>> Unfortunatelly, not all information on PCIE is public, so
>> I could be wrong here.
>> ---
>> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index ffd6604..a428b2d 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1478,7 +1478,6 @@ static struct omap_hwmod dra7xx_pcie1_hwmod = {
>> .prcm = {
>> .omap4 = {
>> .clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
>> - .modulemode = MODULEMODE_SWCTRL,
>
> I think the entire .prcm can be removed here.
not sure. I've tried it and Kernel boot failed (on 3.14)
>> },
>> },
>> };
>> @@ -1492,7 +1491,6 @@ static struct omap_hwmod dra7xx_pcie2_hwmod = {
>> .prcm = {
>> .omap4 = {
>> .clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
>> - .modulemode = MODULEMODE_SWCTRL,
>> },
>> },
>> };
>>
>
> Thanks
> Kishon
>
--
regards,
-grygorii
next prev parent reply other threads:[~2015-02-09 10:28 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-03 15:51 [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods grygorii.strashko at linaro.org
2015-02-09 8:50 ` Kishon Vijay Abraham I
2015-02-09 10:28 ` Grygorii.Strashko@linaro•org [this message]
2015-02-09 13:24 ` Kishon Vijay Abraham I
2015-02-09 14:52 ` Grygorii.Strashko@linaro•org
2015-02-12 6:43 ` Kishon Vijay Abraham I
2015-02-12 8:35 ` Grygorii.Strashko@linaro•org
2015-02-12 15:08 ` Paul Walmsley
2015-02-12 16:24 ` Grygorii.Strashko@linaro•org
2015-02-12 16:27 ` Paul Walmsley
2015-02-12 16:45 ` Paul Walmsley
2015-02-13 14:15 ` Kishon Vijay Abraham I
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54D88BB3.4060104@linaro.org \
--to=grygorii.strashko@linaro$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox