[U-Boot] [PATCH v3 2/7] arm: usb: dra7xx: xHCI registers based on USB port index

Wed Feb 22 12:58:44 UTC 2017

On 02/20/2017 10:28 AM, Marek Vasut wrote:
> On 02/20/2017 08:47 AM, Igor Grinberg wrote:
>> Hey Marek,
>
> Hi,
>
>> We haven't spoken for great while... I'm glad to hear from you!
>
> yeah
>
>> On 02/19/17 19:39, Marek Vasut wrote:
>>> On 02/19/2017 05:26 PM, Igor Grinberg wrote:
>>>> Hi guys,
>>>>
>>>> On 02/19/17 17:15, Tom Rini wrote:
>>>>> On Sun, Feb 19, 2017 at 04:13:02PM +0100, Marek Vasut wrote:
>>>>>> On 02/19/2017 03:55 PM, Uri Mashiach wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/19/2017 04:27 PM, Marek Vasut wrote:
>>>>>>>> On 02/19/2017 02:27 PM, Uri Mashiach wrote:
>>>>>>>>> Modify the determination of the base address of xHCI registers of DRA7XX
>>>>>>>>> targets.
>>>>>>>>> Before the commit: by the target.
>>>>>>>>> After the commit: by the USB port index.
>>>>>>>>>
>>>>>>>>> Cc: Lokesh Vutla <lokeshvutla at ti.com>
>>>>>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>>>>>> Signed-off-by: Uri Mashiach <uri.mashiach at compulab.co.il>
>>>>>>>>> ---
>>>>>>>>> V1 -> V2: Replace the commit "fix XHCI registers base address".
>>>>>>>>> V2 -> V3: Replace the commit "reintroduce the CONFIG_AM57XX symbol"
>>>>>>>>>
>>>>>>>>>  configs/dra7xx_evm_defconfig    |  1 +
>>>>>>>>>  configs/dra7xx_hs_evm_defconfig |  1 +
>>>>>>>>>  drivers/usb/host/Kconfig        | 16 ++++++++++++++++
>>>>>>>>>  include/linux/usb/xhci-omap.h   |  6 ++++--
>>>>>>>>>  4 files changed, 22 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig
>>>>>>>>> index 26b26cc..1f47f92 100644
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>>>>>> index 5129a57..440fbcf 100644
>>>>>>>>> --- a/drivers/usb/host/Kconfig
>>>>>>>>> +++ b/drivers/usb/host/Kconfig
>>>>>>>>> @@ -43,6 +43,22 @@ config USB_XHCI_ZYNQMP
>>>>>>>>>      help
>>>>>>>>>        Enables support for the on-chip xHCI controller on Xilinx
>>>>>>>>> ZynqMP SoCs.
>>>>>>>>>
>>>>>>>>> +choice
>>>>>>>>> +    prompt "DRA7XX xHCI USB index select"
>>>>>>>>> +    depends on DRA7XX
>>>>>>>>> +
>>>>>>>>> +config USB_XHCI_DRA7XX_INDEX0
>>>>>>>>> +    bool "USB0"
>>>>>>>>> +    help
>>>>>>>>> +      DRA7XX xHCI USB0.
>>>>>>>>> +
>>>>>>>>> +config USB_XHCI_DRA7XX_INDEX1
>>>>>>>>> +    bool "USB1"
>>>>>>>>> +    help
>>>>>>>>> +      DRA7XX xHCI USB1.
>>>>>>>>
>>>>>>>> What is this all about ? Shouldn't this come from DT ? And what if I
>>>>>>>> want to use both XHCI ? This looks totally bogus ...
>>>>
>>>> Right, both XHCIs cannot be used with current driver and we do not have
>>>> the time to fix it by our own... may be TI has?
>>>> Remember, you've accepted the driver and following patches, right?
>>>
>>> You mean this patch [1] which actually adds the board dependency ?
>>> Actually , no , I DID NOT. It's been going on for a while that my
>>> role as a USB custodian was actively circumvented and patches which
>>> should've gone through the USB tree just did not.
>>
>> Ahh.. Sorry, thought you are collecting all the USB patches or at least
>> provide your Ack so patches can go through other trees..
>
> So did I ...
>
>>> So please, next time you start blaming someone, get your facts right.
>>
>> Yeah, I haven't looked at it thoroughly... That's why the "right?" in the
>> end of my sentence. Sorry once again, I did not mean to offend you.
>>
>>>
>>> I already have almost zero motivation to maintain USB in my free time,
>>> being paid nothing for it, not ever hearing a single "thank you" and
>>> just spending time I could use otherwise. Being circumvented and only
>>> catching shit for problematic patches I did NOT even apply though, that
>>> is completely off-putting .
>>
>> Ahhhhh... First of all thank you Marek! You've done a great job AFAIR.
>> I really mean it.
>> And it is a pity, things have gone this way.
>> May be this can still be fixed? Tom?
>
> I already had a long discussion with Tom yesterday ... so let's see .
>
>>> [1] https://patchwork.ozlabs.org/patch/700372/
>>>
>>>> Regarding DT, do we have a DT as a requirement to run USB in U-Boot?
>>>
>>> It is highly recommended.
>>
>> Hmmm... I don't really like this approach as it does not help during the board
>> production, where I'd like to see the bootloader detect as much a it can
>> during runtime and then flash the board with correct DT...
>
> You can enable all during production, plug USB sticks into every port,
> check which controllers detect devices and then only keep those enabled
> via the "status" DT prop. As a bonus, you get functional test.
>
> Does the OMAP5 not use DT yet ?
>
>>>> I don't remember this happening and I think it shouldn't be a requirement.
>>>>
>>>>>>>>
>>>>>>>
>>>>>>> The support for both XHCI is currently missing.
>>>>>>> This could be a temporary solution until the DT solution.
>>>>>>> The current situation is worse - selecting USB0 or USB1 based on the
>>>>>>> target.
>>>>>>
>>>>>> So we're replacing it with equally bad solution ?
>>>>
>>>> I don't think equally applies here...
>>>> This IS an improvement. Of course it is not like you would want it to be,
>>>> but still it is from a platform POV.
>>>
>>> TBH, revering 042fdb7cabb8270eb86c45f11263fa91c12e3b65 might be the way
>>> to go.
>>
>> Ok. I'm fine with this approach.
>> I'm also fine with if the driver gets fixed and both XHCI instances can be used.
>
> Someone would have to put that effort in.
>
> I wonder if instead of defining INDEX0, INDEX1 ... this could be turned
> into integer ...
>

Something like the following?

#ifdef CONFIG_DRA7XX
#if CONFIG_USB_XHCI_DRA7XX_INDEX == 1
...
#elif CONFIG_USB_XHCI_DRA7XX_INDEX == 0
...
#endif /* CONFIG_USB_XHCI_DRA7XX_INDEX == 1 */
#elif defined CONFIG_AM43XX
...

>>>
>>>>>> Hmmm , no.
>>>>>> The MW will open mid-march, there's about a month to fix this,
>>>>>> so please do.
>>>>>
>>>>> Do note that the relevant driver here is not yet using DM_USB.
>>>>
>>>> Yes, the driver should be fixed some day. We would really like to take
>>>> this task, but unfortunately, we cannot, at least not right now.
>>>> But we do need that USB working on our board and not only on TI EVMs...
>>>
>>> See above, I believe 042fdb7cabb8270eb86c45f11263fa91c12e3b65 is bogus.
>>
>> Well, agreed.
>>
>>>
>>>> Tom,
>>>> Should we fall back to v1 and have a worse solution for the base addresses?
>>>
>>> OK, I see me being USB custodian has exactly zero value now ... good to
>>> know.
>>
>> I believe we should start fixing things and probably the best time
>> would be... now? I mean, we can't fix things in the past, right?
>>
>
>

-- 
Thanks and regards,
Uri