diff mbox

[08/15] ARM: OMAP2+: hwmod: Fix the omap_hwmod_addr_space for CPGMAC0

Message ID 1351859566-24818-9-git-send-email-vaibhav.bedia@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vaibhav Bedia Nov. 2, 2012, 12:32 p.m. UTC
The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT
instead of ADDR_TYPE_RT to ensure the omap hwmod code
maps the memory space at init and writes to the SYSCONFIG
registers.

Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Santosh Shilimkar Nov. 3, 2012, 4:18 p.m. UTC | #1
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
> The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT
> instead of ADDR_TYPE_RT to ensure the omap hwmod code
> maps the memory space at init and writes to the SYSCONFIG
> registers.
>
> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> ---
Sorry again similar question.

Why CPGMAC0 should be mapped and sysconfig updated early ?

Regards
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 4, 2012, 3:26 p.m. UTC | #2
On Sat, Nov 03, 2012 at 21:48:48, Shilimkar, Santosh wrote:
> On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
> > The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT
> > instead of ADDR_TYPE_RT to ensure the omap hwmod code
> > maps the memory space at init and writes to the SYSCONFIG
> > registers.
> >
> > Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> > ---
> Sorry again similar question.
> 
> Why CPGMAC0 should be mapped and sysconfig updated early ?
> 

Hmm I need to revisit this one. CPGMAC0 was not going to standby
without this. Maybe something else is wrong in the hwmod data and
needs fixing.

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 5, 2012, 9:10 a.m. UTC | #3
On Sun, Nov 04, 2012 at 20:54:17, Bedia, Vaibhav wrote:
> On Sat, Nov 03, 2012 at 21:48:48, Shilimkar, Santosh wrote:
> > On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
> > > The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT
> > > instead of ADDR_TYPE_RT to ensure the omap hwmod code
> > > maps the memory space at init and writes to the SYSCONFIG
> > > registers.
> > >
> > > Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> > > ---
> > Sorry again similar question.
> > 
> > Why CPGMAC0 should be mapped and sysconfig updated early ?
> > 
> 
> Hmm I need to revisit this one. CPGMAC0 was not going to standby
> without this. Maybe something else is wrong in the hwmod data and
> needs fixing.
> 

Ok I checked this one. The change I made was indirectly fixing another
issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
entries and the SYSC register is part of the second entry. The function
_find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
entry in am33xx_cpgmac0_addr_space[] become the first memory space with
the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
SYSC address of CPGMAC0 and the IP went to standby during bootup. 
After changing the order of the entries in am33xx_cpgmac0_addr_space[]
things work fine.

I'll make the changes in the next version.

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Hiremath Nov. 6, 2012, 9:29 a.m. UTC | #4
On 11/5/2012 2:40 PM, Bedia, Vaibhav wrote:
> On Sun, Nov 04, 2012 at 20:54:17, Bedia, Vaibhav wrote:
>> On Sat, Nov 03, 2012 at 21:48:48, Shilimkar, Santosh wrote:
>>> On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
>>>> The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT
>>>> instead of ADDR_TYPE_RT to ensure the omap hwmod code
>>>> maps the memory space at init and writes to the SYSCONFIG
>>>> registers.
>>>>
>>>> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
>>>> ---
>>> Sorry again similar question.
>>>
>>> Why CPGMAC0 should be mapped and sysconfig updated early ?
>>>
>>
>> Hmm I need to revisit this one. CPGMAC0 was not going to standby
>> without this. Maybe something else is wrong in the hwmod data and
>> needs fixing.
>>
> 
> Ok I checked this one. The change I made was indirectly fixing another
> issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
> entries and the SYSC register is part of the second entry. The function
> _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
> the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
> entry in am33xx_cpgmac0_addr_space[] become the first memory space with
> the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
> SYSC address of CPGMAC0 and the IP went to standby during bootup. 
> After changing the order of the entries in am33xx_cpgmac0_addr_space[]
> things work fine.
> 

Good catch.

Just a side note on this, driver expects the addresses in this order
only, first SS and then WR.

Thanks,
Vaibhav
> I'll make the changes in the next version.
> 
> Regards,
> Vaibhav
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 6, 2012, 10:09 a.m. UTC | #5
On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote:
[...]
> > 
> > Ok I checked this one. The change I made was indirectly fixing another
> > issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
> > entries and the SYSC register is part of the second entry. The function
> > _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
> > the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
> > entry in am33xx_cpgmac0_addr_space[] become the first memory space with
> > the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
> > SYSC address of CPGMAC0 and the IP went to standby during bootup. 
> > After changing the order of the entries in am33xx_cpgmac0_addr_space[]
> > things work fine.
> > 
> 
> Good catch.
> 
> Just a side note on this, driver expects the addresses in this order
> only, first SS and then WR.
> 

Sorry I didn't understand your comment. For HWMOD code to work as expected,
we need to change the order. Are you saying that I should not be doing this
because the driver will get affected?

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Hiremath Nov. 6, 2012, 1:08 p.m. UTC | #6
On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote:
> On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote:
> [...]
> > > 
> > > Ok I checked this one. The change I made was indirectly fixing another
> > > issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
> > > entries and the SYSC register is part of the second entry. The function
> > > _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
> > > the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
> > > entry in am33xx_cpgmac0_addr_space[] become the first memory space with
> > > the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
> > > SYSC address of CPGMAC0 and the IP went to standby during bootup. 
> > > After changing the order of the entries in am33xx_cpgmac0_addr_space[]
> > > things work fine.
> > > 
> > 
> > Good catch.
> > 
> > Just a side note on this, driver expects the addresses in this order
> > only, first SS and then WR.
> > 
> 
> Sorry I didn't understand your comment. For HWMOD code to work as expected,
> we need to change the order. 

Why do you want to change the order? Just specify "ADDR_TYPE_RT", that 
should be it.

Thanks,
Vaibhav

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 6, 2012, 1:46 p.m. UTC | #7
On Tue, Nov 06, 2012 at 18:38:08, Hiremath, Vaibhav wrote:
> On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote:
> > On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote:
> > [...]
> > > > 
> > > > Ok I checked this one. The change I made was indirectly fixing another
> > > > issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
> > > > entries and the SYSC register is part of the second entry. The function
> > > > _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
> > > > the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
> > > > entry in am33xx_cpgmac0_addr_space[] become the first memory space with
> > > > the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
> > > > SYSC address of CPGMAC0 and the IP went to standby during bootup. 
> > > > After changing the order of the entries in am33xx_cpgmac0_addr_space[]
> > > > things work fine.
> > > > 
> > > 
> > > Good catch.
> > > 
> > > Just a side note on this, driver expects the addresses in this order
> > > only, first SS and then WR.
> > > 
> > 
> > Sorry I didn't understand your comment. For HWMOD code to work as expected,
> > we need to change the order. 
> 
> Why do you want to change the order? Just specify "ADDR_TYPE_RT", that 
> should be it.
> 

The problem is that the memory space without the SYSC comes first and is labeled
as ADDR_TYPE_RT. I think this is not correct and hence either we change the order
or remove the flag from the first entry. If we do the latter then taking the logic
of putting in the flag only for memory spaces with SYSC further we need to fixup
the entries for ephrpwm0/1/2 and ecap0/1/2.

Regards,
Vaibhav 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson Nov. 6, 2012, 1:50 p.m. UTC | #8
Hi Vaibhav & Vaibhav,

On 11/06/2012 02:46 PM, Bedia, Vaibhav wrote:
> On Tue, Nov 06, 2012 at 18:38:08, Hiremath, Vaibhav wrote:
>> On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote:
>>> On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote:
>>> [...]
>>>>>
>>>>> Ok I checked this one. The change I made was indirectly fixing another
>>>>> issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
>>>>> entries and the SYSC register is part of the second entry. The function
>>>>> _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
>>>>> the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
>>>>> entry in am33xx_cpgmac0_addr_space[] become the first memory space with
>>>>> the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
>>>>> SYSC address of CPGMAC0 and the IP went to standby during bootup. 
>>>>> After changing the order of the entries in am33xx_cpgmac0_addr_space[]
>>>>> things work fine.
>>>>>
>>>>
>>>> Good catch.
>>>>
>>>> Just a side note on this, driver expects the addresses in this order
>>>> only, first SS and then WR.
>>>>
>>>
>>> Sorry I didn't understand your comment. For HWMOD code to work as expected,
>>> we need to change the order. 
>>
>> Why do you want to change the order? Just specify "ADDR_TYPE_RT", that 
>> should be it.
>>
> 
> The problem is that the memory space without the SYSC comes first and is labeled
> as ADDR_TYPE_RT. I think this is not correct and hence either we change the order
> or remove the flag from the first entry. If we do the latter then taking the logic
> of putting in the flag only for memory spaces with SYSC further we need to fixup
> the entries for ephrpwm0/1/2 and ecap0/1/2.

The order should not matter, just use ADDR_TYPE_RT for the relevant
entry. I have a patch ongoing to remove this flag for the non-SYSC entry
to avoid this kind of confusion.
The name should probably be changed as well to reflect that at some point.
Since these entries will be removed anyway with pure DT boot, that
should be a temporary issue.

Regards,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 6, 2012, 1:56 p.m. UTC | #9
Hi Benoit,

On Tue, Nov 06, 2012 at 19:20:46, Cousson, Benoit wrote:
> Hi Vaibhav & Vaibhav,
> 
> On 11/06/2012 02:46 PM, Bedia, Vaibhav wrote:
> > On Tue, Nov 06, 2012 at 18:38:08, Hiremath, Vaibhav wrote:
> >> On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote:
> >>> On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote:
> >>> [...]
> >>>>>
> >>>>> Ok I checked this one. The change I made was indirectly fixing another
> >>>>> issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two
> >>>>> entries and the SYSC register is part of the second entry. The function
> >>>>> _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with
> >>>>> the flag ADDR_TYPE_RT flag. The change I made indirectly made the second
> >>>>> entry in am33xx_cpgmac0_addr_space[] become the first memory space with
> >>>>> the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct
> >>>>> SYSC address of CPGMAC0 and the IP went to standby during bootup. 
> >>>>> After changing the order of the entries in am33xx_cpgmac0_addr_space[]
> >>>>> things work fine.
> >>>>>
> >>>>
> >>>> Good catch.
> >>>>
> >>>> Just a side note on this, driver expects the addresses in this order
> >>>> only, first SS and then WR.
> >>>>
> >>>
> >>> Sorry I didn't understand your comment. For HWMOD code to work as expected,
> >>> we need to change the order. 
> >>
> >> Why do you want to change the order? Just specify "ADDR_TYPE_RT", that 
> >> should be it.
> >>
> > 
> > The problem is that the memory space without the SYSC comes first and is labeled
> > as ADDR_TYPE_RT. I think this is not correct and hence either we change the order
> > or remove the flag from the first entry. If we do the latter then taking the logic
> > of putting in the flag only for memory spaces with SYSC further we need to fixup
> > the entries for ephrpwm0/1/2 and ecap0/1/2.
> 
> The order should not matter, just use ADDR_TYPE_RT for the relevant
> entry. I have a patch ongoing to remove this flag for the non-SYSC entry
> to avoid this kind of confusion.
> The name should probably be changed as well to reflect that at some point.
> Since these entries will be removed anyway with pure DT boot, that
> should be a temporary issue.
> 

Thanks for the clarification. I'll make the change accordingly.

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
index 7772c29..881b570 100644
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -2500,7 +2500,7 @@  static struct omap_hwmod_addr_space am33xx_cpgmac0_addr_space[] = {
 	{
 		.pa_start	= 0x4a100000,
 		.pa_end		= 0x4a100000 + SZ_2K - 1,
-		.flags		= ADDR_TYPE_RT,
+		.flags		= ADDR_MAP_ON_INIT,
 	},
 	/* cpsw wr */
 	{