Re: [PATCH v7 10/14] usb: otg: add hcd companion support

From: Roger Quadros
Date: Thu May 12 2016 - 08:14:22 EST


Hi,

On 12/05/16 13:31, Yoshihiro Shimoda wrote:
> Hi,
>
>> From: Roger Quadros
>> Sent: Thursday, May 12, 2016 6:32 PM
>>
>> Hi,
>>
>> On 12/05/16 11:34, Roger Quadros wrote:
>>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
>>>> Hi,
>>>>
>>>>> From: Alan Stern
>>>>> Sent: Wednesday, May 11, 2016 11:47 PM
>>>>>
>>>>> On Wed, 11 May 2016, Roger Quadros wrote:
>>>>>
>>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>>>>> controllers, don't you need to know which companion goes with which EHCI
>>>>>>> controller? Just like you do for the otg-controller property.
>>>>>>>
>>>>>>
>>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>>>>> with multiple EHCI + companion instances.
>>>>
>>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
>>>>
>>>> < My environment >
>>>> - an otg controller: Sets hcd-needs-companion.
>>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
>>>> - ehci1 and ohci1: No "otg-controller" property.
>>>> - ehci2 and ohci2: No "otg-controller" property.
>>>>
>>>> In this environment, all hosts works correctly.
>>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
>>>
>>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
>>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
>>>
>>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
>>>> I'm not sure such environment actually exists.
>>>
>>> No it is not about that.
>
> Thank you for the reply. I understood it.
>
>>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>>>>> or the handoff is software transparent?
>>>>>
>>>>> The core knows. It doesn't use the information for a whole lot of
>>>>> things, but it does use it in a couple of places. Search for
>>>>> "companion" in core/hcd-pci.c and you'll see.
>>>>
>>>> Thank you for the information. I didn't know this code.
>>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
>>>
>>> That is correct.
>>>
>>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
>>>> So, I will try to add such a code if needed.
>>>
>>> I think OTG core would have to rely on USB core in providing the right companion device,
>>> just like we rely on it for the primary vs shared HCD case.
>>>
>>
>> OK, it is not so simple.
>>
>> EHCI and companion port handoff is really meant to be software transparent.
>>
>> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
>> With device tree we could provide this mapping but for non-device tree case we can't do
>> anything.
>>
>> So my suggestion would be to keep dual role implementation limited to one instance for
>> EHCI + companion case for non-DT.
>> For PCI case I don't see how dual role can be implemented. I don't think we have any
>> dual-role PCI cards.
>
> R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
> one high speed function controller via AXI bus.
> One of channel can be used as host or function.
>
>> For DT case we could have a DT binding to tie the EHCI and companion and use that
>> in the OTG framework.

After looking at the code it seems we don't need this special binding as we are already
linking the EHCI controller and companion controller to the single otg controller instance
using the otg-controller property.

So all is good as of now.

For non DT case, it is the responsibility of platform support code to ensure that
it calls usb_otg_add_hcd() with the correct otg controller instance for both EHCI and
companion controller and things should work fine there as well.

--
cheers,
-roger

>
> R-Car Gen3 SoC (r8a7795 / arm64) will be this type.
> (Both USB 2.0 host/function controllers connect to AXI bus.)
>
>> Any objections?
>
> I don't have any objections because I'm just focus on R-Car Gen3 SoC for now.
> If someone needs for PCI case, I think it is possible to add such a code somehow later.
>
> Best regards,
> Yoshihiro Shimoda
>
>> cheers,
>> -roger