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

From: [email-masked] (Benoit Cousson)
Date: Tue, 6 Nov 2012 14:50:46 +0100

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