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

[Igor Grinberg]

Hey Marek,

We haven't spoken for great while... I'm glad to hear from you!

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 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?

> 
> [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...

> 
>> 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.

> 
>>>> 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?

-- 
Regards,
Igor.