[U-Boot] [PATCH 1/6] arm: am57xx: cl-som-am57x: fix XHCI registers base address

[Uri Mashiach]

On 02/13/2017 02:33 PM, Lokesh Vutla wrote:
>
>
> On Monday 13 February 2017 05:42 PM, Uri Mashiach wrote:
>>
>>
>> On 02/13/2017 01:46 PM, Lokesh Vutla wrote:
>>>
>>>
>>> On Sunday 12 February 2017 09:17 PM, Tom Rini wrote:
>>>> On Sun, Feb 12, 2017 at 10:55:27AM +0200, Uri Mashiach wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On 02/09/2017 10:29 PM, Tom Rini wrote:
>>>>>> On Thu, Feb 09, 2017 at 09:00:26AM +0200, Uri Mashiach wrote:
>>>>>>
>>>>>>> The following XHCI registers base address are set to OMAP5 values:
>>>>>>> OMAP_XHCI_BASE, OMAP_OCP1_SCP_BASE, OMAP_OTG_WRAPPER_BASE
>>>>>>>
>>>>>>> Captured crash for "usb start" command:
>>>>>>> ----------------------------------cut----------------------------------
>>>>>>>
>>>>>>> => usb start
>>>>>>> starting USB...
>>>>>>> USB0:   data abort
>>>>>>> pc : [<fff63d22>]          lr : [<fff63b63>]
>>>>>>> reloc pc : [<8081cd22>]    lr : [<8081cb63>]
>>>>>>> sp : fdf42d08  ip : fff9e040     fp : fdf42d50
>>>>>>> r10: fff8a998  r9 : fdf42ef0     r8 : 00000000
>>>>>>> r7 : fdf42d28  r6 : fdf42d2c     r5 : fffa5c00  r4 : 00000000
>>>>>>> r3 : 4a020000  r2 : 00000000     r1 : fdf42e78  r0 : fffa5c00
>>>>>>> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
>>>>>>> Resetting CPU ...
>>>>>>>
>>>>>>> resetting ...
>>>>>>> ----------------------------------cut----------------------------------
>>>>>>>
>>>>>>>
>>>>>>> Fix by adding the CL-SOM-AM57x target to the XHCI registers base
>>>>>>> address
>>>>>>> ifdef'ery.
>>>>>>> A better fix should be based on a SOC family defines (currently
>>>>>>> missing).
>>>>>>
>>>>>> Can you please go add the Kconfig symbols that would be the better
>>>>>> solution please?  Thanks!
>>>>>
>>>>> The SOC family symbol CONFIG_AM57XX was removed by the commit
>>>>> 3891a54: "ARM: DRA7x/AM57xx: Get rid of CONFIG_AM57XX".
>>>>> Maybe the symbol should be reintroduced just for the XHCI registers
>>>>> section?
>>>>
>>>> Yes, sounds like we do have a case where DRA7xx is not the same as
>>>> AM57xx then?
>>>>
>>>
>>> No that's not right. It is just DRA74x_EVM and AM57XX evm is using
>>> different instances of XHCI. Ideally this base address should be coming
>>> from DT. I don't think it is a good idea to introduce CONFIG_AM57XX.
>>>
>>> Thanks and regards,
>>> Lokesh
>>>
>> It seems that the address of register is SOC dependent.
>> In the current method all the target boards that use the relevant SOC
>> should be included in the ifdef'ery - redundant maintenance.
>
> Please have a look at the TRMs. DRA7xx[1], AM57xx[2]. Both has the above
> two instances of XHCI. There is no difference.
>
> [1] http://www.ti.com/lit/ug/sprui30b/sprui30b.pdf
> [2] http://www.ti.com/lit/ug/spruhz6h/spruhz6h.pdf
>
> Thanks and regards,
> Lokesh
>
The fact that the two SOCs share the same registers base doesn't 
contradict the approach of selecting the registers based on the SOC.

I agree that the resolution should be higher. The exact SOC id 
(CONFIG_AM572X, CONFIG_DRA74X, CONFIG_DRA75X...) should be determined by 
the target selection - board file or Kconfig.

The ifdef'ery should include the exact SOCs id. According to the 
provided TRMs, CONFIG_AM572X and CONFIG_DRA74X will be located in the 
same ifdef'ery case.
-- 
Regards,
Uri