| Message ID | 1351859566-24818-9-git-send-email-vaibhav.bedia@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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 >
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
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
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
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
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
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 */ {
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(-)