linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
@ 2015-02-03 15:51 grygorii.strashko at linaro.org
  2015-02-09  8:50 ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 12+ messages in thread
From: grygorii.strashko at linaro.org @ 2015-02-03 15:51 UTC (permalink / raw)
  To: linux-arm-kernel

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.

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.
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,
 		},
 	},
 };
@@ -1492,7 +1491,6 @@ static struct omap_hwmod dra7xx_pcie2_hwmod = {
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
-			.modulemode   = MODULEMODE_SWCTRL,
 		},
 	},
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Kishon Vijay Abraham I @ 2015-02-09  8:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

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.

> 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.
>  		},
>  	},
>  };
> @@ -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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-09  8:50 ` Kishon Vijay Abraham I
@ 2015-02-09 10:28   ` Grygorii.Strashko@linaro.org
  2015-02-09 13:24     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 12+ messages in thread
From: Grygorii.Strashko@linaro.org @ 2015-02-09 10:28 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-09 10:28   ` Grygorii.Strashko@linaro.org
@ 2015-02-09 13:24     ` Kishon Vijay Abraham I
  2015-02-09 14:52       ` Grygorii.Strashko@linaro.org
  0 siblings, 1 reply; 12+ messages in thread
From: Kishon Vijay Abraham I @ 2015-02-09 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Monday 09 February 2015 03:58 PM, Grygorii.Strashko at linaro.org wrote:
> 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?

you are right. We should have a single hwmod like dra7xx_pciess1_hwmod whose
clkdm should be "pcie_clkdm" and whose clkctrl_offs should be
DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE). PCIE PHY
shouldn't have a hwmod entry at all.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Grygorii.Strashko@linaro.org @ 2015-02-09 14:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/09/2015 09:24 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 09 February 2015 03:58 PM, Grygorii.Strashko at linaro.org wrote:
>> 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?
>
> you are right. We should have a single hwmod like dra7xx_pciess1_hwmod whose
> clkdm should be "pcie_clkdm" and whose clkctrl_offs should be
> DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE). PCIE PHY
> shouldn't have a hwmod entry at all.
>
> Thanks
> Kishon
>

Could this patch be applied any way? It fixes real issue for me.

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Kishon Vijay Abraham I @ 2015-02-12  6:43 UTC (permalink / raw)
  To: linux-arm-kernel

+tero

Hi,

On Monday 09 February 2015 08:22 PM, Grygorii.Strashko at linaro.org wrote:
> On 02/09/2015 09:24 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Monday 09 February 2015 03:58 PM, Grygorii.Strashko at linaro.org wrote:
>>> 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?
>>
>> you are right. We should have a single hwmod like dra7xx_pciess1_hwmod
>> whose
>> clkdm should be "pcie_clkdm" and whose clkctrl_offs should be
>> DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE).
>> PCIE PHY
>> shouldn't have a hwmod entry at all.
>>
>> Thanks
>> Kishon
>>
>
> Could this patch be applied any way? It fixes real issue for me.

A proper fix should look something like below IMO

Thanks
Kishon

 From 1c177d37ac46885a4dc17bacec33071ac23c56da Mon Sep 17 00:00:00 2001
From: Kishon Vijay Abraham I <kishon@ti.com>
Date: Thu, 12 Feb 2015 11:55:08 +0530
Subject: [RFC PATCH] ARM: DRA7: hwmod_data: Fix hwmod data for pcie

Fixed hwmod data for pcie by having the correct module mode offset.
Previously this module mode offset was part of pcie PHY which was wrong.
Now this module mode offset was moved to pcie hwmod and removed the 
hwmod data
for pcie phy. While at that renamed pcie_hwmod to pciess_hwmod in order
to match with the name given in TRM.

This helps to get rid of the following warning
"omap_hwmod: pcie1: _wait_target_disable failed"

[Grygorii.Strashko at linaro.org: Found the issue that actually caused
  "omap_hwmod: pcie1: _wait_target_disable failed"]
Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
this patch was created on 3.14 kernel after applying reset framework patches
required for testing PCIe. I can port this to mainline if this patch is 
fine.

  arch/arm/boot/dts/dra7.dtsi               |    2 -
  arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  107 
+++++++----------------------
  2 files changed, 26 insertions(+), 83 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index a4b1337..d7a1ff9 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1364,7 +1364,6 @@
  					      "wkupclk", "refclk",
  					      "div-clk", "phy-div";
  				#phy-cells = <0>;
-				ti,hwmods = "pcie1-phy";
  			};

  			pcie2_phy: pciephy at 4a095000 {
@@ -1383,7 +1382,6 @@
  					      "wkupclk", "refclk",
  					      "div-clk", "phy-div";
  				#phy-cells = <0>;
-				ti,hwmods = "pcie2-phy";
  			};
  		};

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 5ae755c..56f2c58 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1968,25 +1968,26 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
   *
   */

-static struct omap_hwmod_class dra7xx_pcie_hwmod_class = {
+static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
  	.name	= "pcie",
  };

  /* pcie1 */
-static struct omap_hwmod_rst_info dra7xx_pcie1_resets[] = {
+static struct omap_hwmod_rst_info dra7xx_pciess1_resets[] = {
  	{ .name = "pcie", .rst_shift = 0 },
  };

-static struct omap_hwmod dra7xx_pcie1_hwmod = {
+static struct omap_hwmod dra7xx_pciess1_hwmod = {
  	.name		= "pcie1",
-	.class		= &dra7xx_pcie_hwmod_class,
+	.class		= &dra7xx_pciess_hwmod_class,
  	.clkdm_name	= "pcie_clkdm",
-	.rst_lines	= dra7xx_pcie1_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pcie1_resets),
+	.rst_lines	= dra7xx_pciess1_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess1_resets),
  	.main_clk	= "l4_root_clk_div",
  	.prcm = {
  		.omap4 = {
-			.clkctrl_offs	= DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
+			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
+			.context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
  			.rstctrl_offs	= DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
  			.modulemode	= MODULEMODE_SWCTRL,
  		},
@@ -1994,60 +1995,22 @@ static struct omap_hwmod dra7xx_pcie1_hwmod = {
  };

  /* pcie2 */
-static struct omap_hwmod_rst_info dra7xx_pcie2_resets[] = {
+static struct omap_hwmod_rst_info dra7xx_pciess2_resets[] = {
  	{ .name = "pcie", .rst_shift = 1 },
  };

-static struct omap_hwmod dra7xx_pcie2_hwmod = {
+static struct omap_hwmod dra7xx_pciess2_hwmod = {
  	.name		= "pcie2",
-	.class		= &dra7xx_pcie_hwmod_class,
+	.class		= &dra7xx_pciess_hwmod_class,
  	.clkdm_name	= "pcie_clkdm",
-	.rst_lines	= dra7xx_pcie2_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pcie2_resets),
-	.main_clk	= "l4_root_clk_div",
-	.prcm = {
-		.omap4 = {
-			.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
-			.rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
-			.modulemode   = MODULEMODE_SWCTRL,
-		},
-	},
-};
-
-/*
- * 'PCIE PHY' class
- *
- */
-
-static struct omap_hwmod_class dra7xx_pcie_phy_hwmod_class = {
-	.name	= "pcie-phy",
-};
-
-/* pcie1 phy */
-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",
-	.prcm = {
-		.omap4 = {
-			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
-			.context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
-			.modulemode   = MODULEMODE_SWCTRL,
-		},
-	},
-};
-
-/* pcie2 phy */
-static struct omap_hwmod dra7xx_pcie2_phy_hwmod = {
-	.name		= "pcie2-phy",
-	.class		= &dra7xx_pcie_phy_hwmod_class,
-	.clkdm_name	= "l3init_clkdm",
+	.rst_lines	= dra7xx_pciess2_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess2_resets),
  	.main_clk	= "l4_root_clk_div",
  	.prcm = {
  		.omap4 = {
  			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS2_CLKCTRL_OFFSET,
  			.context_offs = DRA7XX_RM_L3INIT_PCIESS2_CONTEXT_OFFSET,
+			.rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
  			.modulemode   = MODULEMODE_SWCTRL,
  		},
  	},
@@ -3621,49 +3584,33 @@ static struct omap_hwmod_ocp_if 
dra7xx_l4_cfg__ocp2scp3 = {
  };

  /* l3_main_1 -> pcie1 */
-static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie1 = {
+static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess1 = {
  	.master		= &dra7xx_l3_main_1_hwmod,
-	.slave		= &dra7xx_pcie1_hwmod,
+	.slave		= &dra7xx_pciess1_hwmod,
  	.clk		= "l3_iclk_div",
  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
  };

  /* l4_cfg -> pcie1 */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1 = {
+static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess1 = {
  	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie1_hwmod,
+	.slave		= &dra7xx_pciess1_hwmod,
  	.clk		= "l4_root_clk_div",
  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
  };

  /* l3_main_1 -> pcie2 */
-static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie2 = {
+static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess2 = {
  	.master		= &dra7xx_l3_main_1_hwmod,
-	.slave		= &dra7xx_pcie2_hwmod,
+	.slave		= &dra7xx_pciess2_hwmod,
  	.clk		= "l3_iclk_div",
  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
  };

  /* l4_cfg -> pcie2 */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2 = {
-	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie2_hwmod,
-	.clk		= "l4_root_clk_div",
-	.user		= OCP_USER_MPU | OCP_USER_SDMA,
-};
-
-/* l4_cfg -> pcie1 phy */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1_phy = {
-	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie1_phy_hwmod,
-	.clk		= "l4_root_clk_div",
-	.user		= OCP_USER_MPU | OCP_USER_SDMA,
-};
-
-/* l4_cfg -> pcie2 phy */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2_phy = {
+static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess2 = {
  	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie2_phy_hwmod,
+	.slave		= &dra7xx_pciess2_hwmod,
  	.clk		= "l4_root_clk_div",
  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
  };
@@ -4153,12 +4100,10 @@ static struct omap_hwmod_ocp_if 
*dra7xx_hwmod_ocp_ifs[] __initdata = {
  	&dra7xx_l4_cfg__mpu,
  	&dra7xx_l4_cfg__ocp2scp1,
  	&dra7xx_l4_cfg__ocp2scp3,
-	&dra7xx_l3_main_1__pcie1,
-	&dra7xx_l4_cfg__pcie1,
-	&dra7xx_l3_main_1__pcie2,
-	&dra7xx_l4_cfg__pcie2,
-	&dra7xx_l4_cfg__pcie1_phy,
-	&dra7xx_l4_cfg__pcie2_phy,
+	&dra7xx_l3_main_1__pciess1,
+	&dra7xx_l4_cfg__pciess1,
+	&dra7xx_l3_main_1__pciess2,
+	&dra7xx_l4_cfg__pciess2,
  	&dra7xx_l4_cfg__pruss1, /* AM57xx only */
  	&dra7xx_l4_cfg__pruss2, /* AM57xx only */
  	&dra7xx_l3_main_1__qspi,
-- 
1.7.9.5

>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Grygorii.Strashko@linaro.org @ 2015-02-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/12/2015 02:43 PM, Kishon Vijay Abraham I wrote:
> On Monday 09 February 2015 08:22 PM, Grygorii.Strashko at linaro.org wrote:
>> On 02/09/2015 09:24 PM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Monday 09 February 2015 03:58 PM, Grygorii.Strashko at linaro.org wrote:
>>>> 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?
>>>
>>> you are right. We should have a single hwmod like dra7xx_pciess1_hwmod
>>> whose
>>> clkdm should be "pcie_clkdm" and whose clkctrl_offs should be
>>> DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE).
>>> PCIE PHY
>>> shouldn't have a hwmod entry at all.
>>>
>>> Thanks
>>> Kishon
>>>
>>
>> Could this patch be applied any way? It fixes real issue for me.
>
> A proper fix should look something like below IMO
>

Looks good for me and seems working.

>
>  From 1c177d37ac46885a4dc17bacec33071ac23c56da Mon Sep 17 00:00:00 2001
> From: Kishon Vijay Abraham I <kishon@ti.com>
> Date: Thu, 12 Feb 2015 11:55:08 +0530
> Subject: [RFC PATCH] ARM: DRA7: hwmod_data: Fix hwmod data for pcie
>
> Fixed hwmod data for pcie by having the correct module mode offset.
> Previously this module mode offset was part of pcie PHY which was wrong.
> Now this module mode offset was moved to pcie hwmod and removed the
> hwmod data
> for pcie phy. While at that renamed pcie_hwmod to pciess_hwmod in order
> to match with the name given in TRM.
>
> This helps to get rid of the following warning
> "omap_hwmod: pcie1: _wait_target_disable failed"
>
> [Grygorii.Strashko at linaro.org: Found the issue that actually caused
>   "omap_hwmod: pcie1: _wait_target_disable failed"]
> Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> this patch was created on 3.14 kernel after applying reset framework
> patches
> required for testing PCIe. I can port this to mainline if this patch is
> fine.
>
>   arch/arm/boot/dts/dra7.dtsi               |    2 -
>   arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  107
> +++++++----------------------
>   2 files changed, 26 insertions(+), 83 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index a4b1337..d7a1ff9 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1364,7 +1364,6 @@
>                             "wkupclk", "refclk",
>                             "div-clk", "phy-div";
>                   #phy-cells = <0>;
> -                ti,hwmods = "pcie1-phy";
>               };
>
>               pcie2_phy: pciephy at 4a095000 {
> @@ -1383,7 +1382,6 @@
>                             "wkupclk", "refclk",
>                             "div-clk", "phy-div";
>                   #phy-cells = <0>;
> -                ti,hwmods = "pcie2-phy";
>               };
>           };
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 5ae755c..56f2c58 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1968,25 +1968,26 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>    *
>    */
>
> -static struct omap_hwmod_class dra7xx_pcie_hwmod_class = {
> +static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>       .name    = "pcie",
>   };
>
>   /* pcie1 */
> -static struct omap_hwmod_rst_info dra7xx_pcie1_resets[] = {
> +static struct omap_hwmod_rst_info dra7xx_pciess1_resets[] = {
>       { .name = "pcie", .rst_shift = 0 },
>   };
>
> -static struct omap_hwmod dra7xx_pcie1_hwmod = {
> +static struct omap_hwmod dra7xx_pciess1_hwmod = {
>       .name        = "pcie1",
> -    .class        = &dra7xx_pcie_hwmod_class,
> +    .class        = &dra7xx_pciess_hwmod_class,
>       .clkdm_name    = "pcie_clkdm",
> -    .rst_lines    = dra7xx_pcie1_resets,
> -    .rst_lines_cnt    = ARRAY_SIZE(dra7xx_pcie1_resets),
> +    .rst_lines    = dra7xx_pciess1_resets,
> +    .rst_lines_cnt    = ARRAY_SIZE(dra7xx_pciess1_resets),
>       .main_clk    = "l4_root_clk_div",
>       .prcm = {
>           .omap4 = {
> -            .clkctrl_offs    = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
> +            .clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
> +            .context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
>               .rstctrl_offs    = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
>               .modulemode    = MODULEMODE_SWCTRL,
>           },
> @@ -1994,60 +1995,22 @@ static struct omap_hwmod dra7xx_pcie1_hwmod = {
>   };
>
>   /* pcie2 */
> -static struct omap_hwmod_rst_info dra7xx_pcie2_resets[] = {
> +static struct omap_hwmod_rst_info dra7xx_pciess2_resets[] = {
>       { .name = "pcie", .rst_shift = 1 },
>   };
>
> -static struct omap_hwmod dra7xx_pcie2_hwmod = {
> +static struct omap_hwmod dra7xx_pciess2_hwmod = {
>       .name        = "pcie2",
> -    .class        = &dra7xx_pcie_hwmod_class,
> +    .class        = &dra7xx_pciess_hwmod_class,
>       .clkdm_name    = "pcie_clkdm",
> -    .rst_lines    = dra7xx_pcie2_resets,
> -    .rst_lines_cnt    = ARRAY_SIZE(dra7xx_pcie2_resets),
> -    .main_clk    = "l4_root_clk_div",
> -    .prcm = {
> -        .omap4 = {
> -            .clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
> -            .rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
> -            .modulemode   = MODULEMODE_SWCTRL,
> -        },
> -    },
> -};
> -
> -/*
> - * 'PCIE PHY' class
> - *
> - */
> -
> -static struct omap_hwmod_class dra7xx_pcie_phy_hwmod_class = {
> -    .name    = "pcie-phy",
> -};
> -
> -/* pcie1 phy */
> -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",
> -    .prcm = {
> -        .omap4 = {
> -            .clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
> -            .context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
> -            .modulemode   = MODULEMODE_SWCTRL,
> -        },
> -    },
> -};
> -
> -/* pcie2 phy */
> -static struct omap_hwmod dra7xx_pcie2_phy_hwmod = {
> -    .name        = "pcie2-phy",
> -    .class        = &dra7xx_pcie_phy_hwmod_class,
> -    .clkdm_name    = "l3init_clkdm",
> +    .rst_lines    = dra7xx_pciess2_resets,
> +    .rst_lines_cnt    = ARRAY_SIZE(dra7xx_pciess2_resets),
>       .main_clk    = "l4_root_clk_div",
>       .prcm = {
>           .omap4 = {
>               .clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS2_CLKCTRL_OFFSET,
>               .context_offs = DRA7XX_RM_L3INIT_PCIESS2_CONTEXT_OFFSET,
> +            .rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
>               .modulemode   = MODULEMODE_SWCTRL,
>           },
>       },
> @@ -3621,49 +3584,33 @@ static struct omap_hwmod_ocp_if
> dra7xx_l4_cfg__ocp2scp3 = {
>   };
>
>   /* l3_main_1 -> pcie1 */
> -static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie1 = {
> +static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess1 = {
>       .master        = &dra7xx_l3_main_1_hwmod,
> -    .slave        = &dra7xx_pcie1_hwmod,
> +    .slave        = &dra7xx_pciess1_hwmod,
>       .clk        = "l3_iclk_div",
>       .user        = OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
>   /* l4_cfg -> pcie1 */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1 = {
> +static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess1 = {
>       .master        = &dra7xx_l4_cfg_hwmod,
> -    .slave        = &dra7xx_pcie1_hwmod,
> +    .slave        = &dra7xx_pciess1_hwmod,
>       .clk        = "l4_root_clk_div",
>       .user        = OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
>   /* l3_main_1 -> pcie2 */
> -static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie2 = {
> +static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess2 = {
>       .master        = &dra7xx_l3_main_1_hwmod,
> -    .slave        = &dra7xx_pcie2_hwmod,
> +    .slave        = &dra7xx_pciess2_hwmod,
>       .clk        = "l3_iclk_div",
>       .user        = OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
>   /* l4_cfg -> pcie2 */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2 = {
> -    .master        = &dra7xx_l4_cfg_hwmod,
> -    .slave        = &dra7xx_pcie2_hwmod,
> -    .clk        = "l4_root_clk_div",
> -    .user        = OCP_USER_MPU | OCP_USER_SDMA,
> -};
> -
> -/* l4_cfg -> pcie1 phy */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1_phy = {
> -    .master        = &dra7xx_l4_cfg_hwmod,
> -    .slave        = &dra7xx_pcie1_phy_hwmod,
> -    .clk        = "l4_root_clk_div",
> -    .user        = OCP_USER_MPU | OCP_USER_SDMA,
> -};
> -
> -/* l4_cfg -> pcie2 phy */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2_phy = {
> +static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess2 = {
>       .master        = &dra7xx_l4_cfg_hwmod,
> -    .slave        = &dra7xx_pcie2_phy_hwmod,
> +    .slave        = &dra7xx_pciess2_hwmod,
>       .clk        = "l4_root_clk_div",
>       .user        = OCP_USER_MPU | OCP_USER_SDMA,
>   };
> @@ -4153,12 +4100,10 @@ static struct omap_hwmod_ocp_if
> *dra7xx_hwmod_ocp_ifs[] __initdata = {
>       &dra7xx_l4_cfg__mpu,
>       &dra7xx_l4_cfg__ocp2scp1,
>       &dra7xx_l4_cfg__ocp2scp3,
> -    &dra7xx_l3_main_1__pcie1,
> -    &dra7xx_l4_cfg__pcie1,
> -    &dra7xx_l3_main_1__pcie2,
> -    &dra7xx_l4_cfg__pcie2,
> -    &dra7xx_l4_cfg__pcie1_phy,
> -    &dra7xx_l4_cfg__pcie2_phy,
> +    &dra7xx_l3_main_1__pciess1,
> +    &dra7xx_l4_cfg__pciess1,
> +    &dra7xx_l3_main_1__pciess2,
> +    &dra7xx_l4_cfg__pciess2,
>       &dra7xx_l4_cfg__pruss1, /* AM57xx only */
>       &dra7xx_l4_cfg__pruss2, /* AM57xx only */
>       &dra7xx_l3_main_1__qspi,


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2015-02-12 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

Thanks guys.

On Thu, 12 Feb 2015, Grygorii.Strashko at linaro.org wrote:

> Looks good for me and seems working.

Grygorii, can I add your Acked-by?


- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-12 15:08             ` Paul Walmsley
@ 2015-02-12 16:24               ` Grygorii.Strashko@linaro.org
  2015-02-12 16:27                 ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Grygorii.Strashko@linaro.org @ 2015-02-12 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/12/2015 11:08 PM, Paul Walmsley wrote:
> Thanks guys.
>
> On Thu, 12 Feb 2015, Grygorii.Strashko at linaro.org wrote:
>
>> Looks good for me and seems working.
>
> Grygorii, can I add your Acked-by?
>
>
> - Paul
>

There is my "Signed-off-by" :)

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-12 16:24               ` Grygorii.Strashko@linaro.org
@ 2015-02-12 16:27                 ` Paul Walmsley
  2015-02-12 16:45                   ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2015-02-12 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 13 Feb 2015, Grygorii.Strashko at linaro.org wrote:

> On 02/12/2015 11:08 PM, Paul Walmsley wrote:
> > Thanks guys.
> > 
> > On Thu, 12 Feb 2015, Grygorii.Strashko at linaro.org wrote:
> > 
> > > Looks good for me and seems working.
> > 
> > Grygorii, can I add your Acked-by?
> > 
> > 
> > - Paul
> > 
> 
> There is my "Signed-off-by" :)

OK thanks, queued for v3.20-rc.


- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-12 16:27                 ` Paul Walmsley
@ 2015-02-12 16:45                   ` Paul Walmsley
  2015-02-13 14:15                     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2015-02-12 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 12 Feb 2015, Paul Walmsley wrote:

> On Fri, 13 Feb 2015, Grygorii.Strashko at linaro.org wrote:
> 
> > On 02/12/2015 11:08 PM, Paul Walmsley wrote:
> > > Thanks guys.
> > > 
> > > On Thu, 12 Feb 2015, Grygorii.Strashko at linaro.org wrote:
> > > 
> > > > Looks good for me and seems working.
> > > 
> > > Grygorii, can I add your Acked-by?
> > > 
> > > 
> > > - Paul
> > > 
> > 
> > There is my "Signed-off-by" :)
> 
> OK thanks, queued for v3.20-rc.

Actually, could you guys test the updated patch?  It had to be changed to 
apply on mainline and I don't have a DRA7xx board to test.


- Paul

>From 0f9a1ee083a7adbb2c867d5c8d25d7e5fcb38b07 Mon Sep 17 00:00:00 2001
From: Kishon Vijay Abraham I <kishon@ti.com>
Date: Thu, 12 Feb 2015 09:29:31 -0700
Subject: [PATCH] ARM: DRA7: hwmod_data: Fix hwmod data for pcie

Fixed hwmod data for pcie by having the correct module mode offset.
Previously this module mode offset was part of pcie PHY which was wrong.
Now this module mode offset was moved to pcie hwmod and removed the
hwmod data
for pcie phy. While at that renamed pcie_hwmod to pciess_hwmod in order
to match with the name given in TRM.

This helps to get rid of the following warning
"omap_hwmod: pcie1: _wait_target_disable failed"

[Grygorii.Strashko at linaro.org: Found the issue that actually caused
  "omap_hwmod: pcie1: _wait_target_disable failed"]
Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
[paul at pwsan.com: updated to apply on mainline]
---
 arch/arm/boot/dts/dra7.dtsi               |   2 -
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 111 ++++++++++--------------------
 2 files changed, 35 insertions(+), 78 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 5827fedafd43..18a904db32bb 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1111,7 +1111,6 @@
 					      "wkupclk", "refclk",
 					      "div-clk", "phy-div";
 				#phy-cells = <0>;
-				ti,hwmods = "pcie1-phy";
 			};
 
 			pcie2_phy: pciephy at 4a095000 {
@@ -1130,7 +1129,6 @@
 					      "wkupclk", "refclk",
 					      "div-clk", "phy-div";
 				#phy-cells = <0>;
-				ti,hwmods = "pcie2-phy";
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index e8692e7675b8..6a74a7ea8c95 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1466,29 +1466,43 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
  *
  */
 
-static struct omap_hwmod_class dra7xx_pcie_hwmod_class = {
+static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
 	.name	= "pcie",
 };
 
 /* pcie1 */
-static struct omap_hwmod dra7xx_pcie1_hwmod = {
+static struct omap_hwmod_rst_info dra7xx_pciess1_resets[] = {
+	{ .name = "pcie", .rst_shift = 0 },
+};
+
+static struct omap_hwmod dra7xx_pciess1_hwmod = {
 	.name		= "pcie1",
-	.class		= &dra7xx_pcie_hwmod_class,
+	.class		= &dra7xx_pciess_hwmod_class,
 	.clkdm_name	= "pcie_clkdm",
 	.main_clk	= "l4_root_clk_div",
+	.rst_lines	= dra7xx_pciess1_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess1_resets),
 	.prcm = {
 		.omap4 = {
-			.clkctrl_offs	= DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
+			.clkctrl_offs	= DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
+			.context_offs	= DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
 			.modulemode	= MODULEMODE_SWCTRL,
 		},
 	},
 };
 
 /* pcie2 */
-static struct omap_hwmod dra7xx_pcie2_hwmod = {
+
+static struct omap_hwmod_rst_info dra7xx_pciess2_resets[] = {
+	{ .name = "pcie", .rst_shift = 1 },
+};
+
+static struct omap_hwmod dra7xx_pciess2_hwmod = {
 	.name		= "pcie2",
-	.class		= &dra7xx_pcie_hwmod_class,
+	.class		= &dra7xx_pciess_hwmod_class,
 	.clkdm_name	= "pcie_clkdm",
+	.rst_lines	= dra7xx_pciess2_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess2_resets),
 	.main_clk	= "l4_root_clk_div",
 	.prcm = {
 		.omap4 = {
@@ -1498,44 +1512,7 @@ static struct omap_hwmod dra7xx_pcie2_hwmod = {
 	},
 };
 
-/*
- * 'PCIE PHY' class
- *
- */
 
-static struct omap_hwmod_class dra7xx_pcie_phy_hwmod_class = {
-	.name	= "pcie-phy",
-};
-
-/* pcie1 phy */
-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",
-	.prcm = {
-		.omap4 = {
-			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
-			.context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
-			.modulemode   = MODULEMODE_SWCTRL,
-		},
-	},
-};
-
-/* pcie2 phy */
-static struct omap_hwmod dra7xx_pcie2_phy_hwmod = {
-	.name		= "pcie2-phy",
-	.class		= &dra7xx_pcie_phy_hwmod_class,
-	.clkdm_name	= "l3init_clkdm",
-	.main_clk	= "l4_root_clk_div",
-	.prcm = {
-		.omap4 = {
-			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS2_CLKCTRL_OFFSET,
-			.context_offs = DRA7XX_RM_L3INIT_PCIESS2_CONTEXT_OFFSET,
-			.modulemode   = MODULEMODE_SWCTRL,
-		},
-	},
-};
 
 /*
  * 'qspi' class
@@ -2877,50 +2854,34 @@ static struct omap_hwmod_ocp_if dra7xx_l4_cfg__ocp2scp3 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
-/* l3_main_1 -> pcie1 */
-static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie1 = {
+/* l3_main_1 -> pciess1 */
+static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess1 = {
 	.master		= &dra7xx_l3_main_1_hwmod,
-	.slave		= &dra7xx_pcie1_hwmod,
+	.slave		= &dra7xx_pciess1_hwmod,
 	.clk		= "l3_iclk_div",
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
-/* l4_cfg -> pcie1 */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1 = {
+/* l4_cfg -> pciess1 */
+static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess1 = {
 	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie1_hwmod,
+	.slave		= &dra7xx_pciess1_hwmod,
 	.clk		= "l4_root_clk_div",
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
-/* l3_main_1 -> pcie2 */
-static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie2 = {
+/* l3_main_1 -> pciess2 */
+static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess2 = {
 	.master		= &dra7xx_l3_main_1_hwmod,
-	.slave		= &dra7xx_pcie2_hwmod,
+	.slave		= &dra7xx_pciess2_hwmod,
 	.clk		= "l3_iclk_div",
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
 /* l4_cfg -> pcie2 */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2 = {
-	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie2_hwmod,
-	.clk		= "l4_root_clk_div",
-	.user		= OCP_USER_MPU | OCP_USER_SDMA,
-};
-
-/* l4_cfg -> pcie1 phy */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1_phy = {
-	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie1_phy_hwmod,
-	.clk		= "l4_root_clk_div",
-	.user		= OCP_USER_MPU | OCP_USER_SDMA,
-};
-
-/* l4_cfg -> pcie2 phy */
-static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2_phy = {
+static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess2 = {
 	.master		= &dra7xx_l4_cfg_hwmod,
-	.slave		= &dra7xx_pcie2_phy_hwmod,
+	.slave		= &dra7xx_pciess2_hwmod,
 	.clk		= "l4_root_clk_div",
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
@@ -3327,12 +3288,10 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
 	&dra7xx_l4_cfg__mpu,
 	&dra7xx_l4_cfg__ocp2scp1,
 	&dra7xx_l4_cfg__ocp2scp3,
-	&dra7xx_l3_main_1__pcie1,
-	&dra7xx_l4_cfg__pcie1,
-	&dra7xx_l3_main_1__pcie2,
-	&dra7xx_l4_cfg__pcie2,
-	&dra7xx_l4_cfg__pcie1_phy,
-	&dra7xx_l4_cfg__pcie2_phy,
+	&dra7xx_l3_main_1__pciess1,
+	&dra7xx_l4_cfg__pciess1,
+	&dra7xx_l3_main_1__pciess2,
+	&dra7xx_l4_cfg__pciess2,
 	&dra7xx_l3_main_1__qspi,
 	&dra7xx_l4_per3__rtcss,
 	&dra7xx_l4_cfg__sata,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
  2015-02-12 16:45                   ` Paul Walmsley
@ 2015-02-13 14:15                     ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 12+ messages in thread
From: Kishon Vijay Abraham I @ 2015-02-13 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Paul,

On Thursday 12 February 2015 10:15 PM, Paul Walmsley wrote:
> On Thu, 12 Feb 2015, Paul Walmsley wrote:
>
>> On Fri, 13 Feb 2015, Grygorii.Strashko at linaro.org wrote:
>>
>>> On 02/12/2015 11:08 PM, Paul Walmsley wrote:
>>>> Thanks guys.
>>>>
>>>> On Thu, 12 Feb 2015, Grygorii.Strashko at linaro.org wrote:
>>>>
>>>>> Looks good for me and seems working.
>>>>
>>>> Grygorii, can I add your Acked-by?
>>>>
>>>>
>>>> - Paul
>>>>
>>>
>>> There is my "Signed-off-by" :)
>>
>> OK thanks, queued for v3.20-rc.
>
> Actually, could you guys test the updated patch?  It had to be changed to
> apply on mainline and I don't have a DRA7xx board to test.

We don't use resets used here in mainline. I'll send an updated patch for mainline.

Thanks
Kishon

>
>
> - Paul
>
>  From 0f9a1ee083a7adbb2c867d5c8d25d7e5fcb38b07 Mon Sep 17 00:00:00 2001
> From: Kishon Vijay Abraham I <kishon@ti.com>
> Date: Thu, 12 Feb 2015 09:29:31 -0700
> Subject: [PATCH] ARM: DRA7: hwmod_data: Fix hwmod data for pcie
>
> Fixed hwmod data for pcie by having the correct module mode offset.
> Previously this module mode offset was part of pcie PHY which was wrong.
> Now this module mode offset was moved to pcie hwmod and removed the
> hwmod data
> for pcie phy. While at that renamed pcie_hwmod to pciess_hwmod in order
> to match with the name given in TRM.
>
> This helps to get rid of the following warning
> "omap_hwmod: pcie1: _wait_target_disable failed"
>
> [Grygorii.Strashko at linaro.org: Found the issue that actually caused
>    "omap_hwmod: pcie1: _wait_target_disable failed"]
> Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> [paul at pwsan.com: updated to apply on mainline]
> ---
>   arch/arm/boot/dts/dra7.dtsi               |   2 -
>   arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 111 ++++++++++--------------------
>   2 files changed, 35 insertions(+), 78 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 5827fedafd43..18a904db32bb 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1111,7 +1111,6 @@
>   					      "wkupclk", "refclk",
>   					      "div-clk", "phy-div";
>   				#phy-cells = <0>;
> -				ti,hwmods = "pcie1-phy";
>   			};
>
>   			pcie2_phy: pciephy at 4a095000 {
> @@ -1130,7 +1129,6 @@
>   					      "wkupclk", "refclk",
>   					      "div-clk", "phy-div";
>   				#phy-cells = <0>;
> -				ti,hwmods = "pcie2-phy";
>   				status = "disabled";
>   			};
>   		};
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index e8692e7675b8..6a74a7ea8c95 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1466,29 +1466,43 @@ static struct omap_hwmod dra7xx_ocp2scp3_hwmod = {
>    *
>    */
>
> -static struct omap_hwmod_class dra7xx_pcie_hwmod_class = {
> +static struct omap_hwmod_class dra7xx_pciess_hwmod_class = {
>   	.name	= "pcie",
>   };
>
>   /* pcie1 */
> -static struct omap_hwmod dra7xx_pcie1_hwmod = {
> +static struct omap_hwmod_rst_info dra7xx_pciess1_resets[] = {
> +	{ .name = "pcie", .rst_shift = 0 },
> +};
> +
> +static struct omap_hwmod dra7xx_pciess1_hwmod = {
>   	.name		= "pcie1",
> -	.class		= &dra7xx_pcie_hwmod_class,
> +	.class		= &dra7xx_pciess_hwmod_class,
>   	.clkdm_name	= "pcie_clkdm",
>   	.main_clk	= "l4_root_clk_div",
> +	.rst_lines	= dra7xx_pciess1_resets,
> +	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess1_resets),
>   	.prcm = {
>   		.omap4 = {
> -			.clkctrl_offs	= DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
> +			.clkctrl_offs	= DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
> +			.context_offs	= DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
>   			.modulemode	= MODULEMODE_SWCTRL,
>   		},
>   	},
>   };
>
>   /* pcie2 */
> -static struct omap_hwmod dra7xx_pcie2_hwmod = {
> +
> +static struct omap_hwmod_rst_info dra7xx_pciess2_resets[] = {
> +	{ .name = "pcie", .rst_shift = 1 },
> +};
> +
> +static struct omap_hwmod dra7xx_pciess2_hwmod = {
>   	.name		= "pcie2",
> -	.class		= &dra7xx_pcie_hwmod_class,
> +	.class		= &dra7xx_pciess_hwmod_class,
>   	.clkdm_name	= "pcie_clkdm",
> +	.rst_lines	= dra7xx_pciess2_resets,
> +	.rst_lines_cnt	= ARRAY_SIZE(dra7xx_pciess2_resets),
>   	.main_clk	= "l4_root_clk_div",
>   	.prcm = {
>   		.omap4 = {
> @@ -1498,44 +1512,7 @@ static struct omap_hwmod dra7xx_pcie2_hwmod = {
>   	},
>   };
>
> -/*
> - * 'PCIE PHY' class
> - *
> - */
>
> -static struct omap_hwmod_class dra7xx_pcie_phy_hwmod_class = {
> -	.name	= "pcie-phy",
> -};
> -
> -/* pcie1 phy */
> -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",
> -	.prcm = {
> -		.omap4 = {
> -			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET,
> -			.context_offs = DRA7XX_RM_L3INIT_PCIESS1_CONTEXT_OFFSET,
> -			.modulemode   = MODULEMODE_SWCTRL,
> -		},
> -	},
> -};
> -
> -/* pcie2 phy */
> -static struct omap_hwmod dra7xx_pcie2_phy_hwmod = {
> -	.name		= "pcie2-phy",
> -	.class		= &dra7xx_pcie_phy_hwmod_class,
> -	.clkdm_name	= "l3init_clkdm",
> -	.main_clk	= "l4_root_clk_div",
> -	.prcm = {
> -		.omap4 = {
> -			.clkctrl_offs = DRA7XX_CM_L3INIT_PCIESS2_CLKCTRL_OFFSET,
> -			.context_offs = DRA7XX_RM_L3INIT_PCIESS2_CONTEXT_OFFSET,
> -			.modulemode   = MODULEMODE_SWCTRL,
> -		},
> -	},
> -};
>
>   /*
>    * 'qspi' class
> @@ -2877,50 +2854,34 @@ static struct omap_hwmod_ocp_if dra7xx_l4_cfg__ocp2scp3 = {
>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
> -/* l3_main_1 -> pcie1 */
> -static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie1 = {
> +/* l3_main_1 -> pciess1 */
> +static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess1 = {
>   	.master		= &dra7xx_l3_main_1_hwmod,
> -	.slave		= &dra7xx_pcie1_hwmod,
> +	.slave		= &dra7xx_pciess1_hwmod,
>   	.clk		= "l3_iclk_div",
>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
> -/* l4_cfg -> pcie1 */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1 = {
> +/* l4_cfg -> pciess1 */
> +static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess1 = {
>   	.master		= &dra7xx_l4_cfg_hwmod,
> -	.slave		= &dra7xx_pcie1_hwmod,
> +	.slave		= &dra7xx_pciess1_hwmod,
>   	.clk		= "l4_root_clk_div",
>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
> -/* l3_main_1 -> pcie2 */
> -static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pcie2 = {
> +/* l3_main_1 -> pciess2 */
> +static struct omap_hwmod_ocp_if dra7xx_l3_main_1__pciess2 = {
>   	.master		= &dra7xx_l3_main_1_hwmod,
> -	.slave		= &dra7xx_pcie2_hwmod,
> +	.slave		= &dra7xx_pciess2_hwmod,
>   	.clk		= "l3_iclk_div",
>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>   };
>
>   /* l4_cfg -> pcie2 */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2 = {
> -	.master		= &dra7xx_l4_cfg_hwmod,
> -	.slave		= &dra7xx_pcie2_hwmod,
> -	.clk		= "l4_root_clk_div",
> -	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> -};
> -
> -/* l4_cfg -> pcie1 phy */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie1_phy = {
> -	.master		= &dra7xx_l4_cfg_hwmod,
> -	.slave		= &dra7xx_pcie1_phy_hwmod,
> -	.clk		= "l4_root_clk_div",
> -	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> -};
> -
> -/* l4_cfg -> pcie2 phy */
> -static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pcie2_phy = {
> +static struct omap_hwmod_ocp_if dra7xx_l4_cfg__pciess2 = {
>   	.master		= &dra7xx_l4_cfg_hwmod,
> -	.slave		= &dra7xx_pcie2_phy_hwmod,
> +	.slave		= &dra7xx_pciess2_hwmod,
>   	.clk		= "l4_root_clk_div",
>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>   };
> @@ -3327,12 +3288,10 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>   	&dra7xx_l4_cfg__mpu,
>   	&dra7xx_l4_cfg__ocp2scp1,
>   	&dra7xx_l4_cfg__ocp2scp3,
> -	&dra7xx_l3_main_1__pcie1,
> -	&dra7xx_l4_cfg__pcie1,
> -	&dra7xx_l3_main_1__pcie2,
> -	&dra7xx_l4_cfg__pcie2,
> -	&dra7xx_l4_cfg__pcie1_phy,
> -	&dra7xx_l4_cfg__pcie2_phy,
> +	&dra7xx_l3_main_1__pciess1,
> +	&dra7xx_l4_cfg__pciess1,
> +	&dra7xx_l3_main_1__pciess2,
> +	&dra7xx_l4_cfg__pciess2,
>   	&dra7xx_l3_main_1__qspi,
>   	&dra7xx_l4_per3__rtcss,
>   	&dra7xx_l4_cfg__sata,
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-02-13 14:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).