[v1,4/9] ARM: tegra: Enable UDC on TrimSlice

Message ID 82321d32d968c6b9481a77fbecae8c0756d37f0f.1499273075.git.digetx@gmail.com
State Superseded
Headers show

Commit Message

Dmitry Osipenko July 5, 2017, 5:19 p.m.
From: Thierry Reding <treding@nvidia.com>

Override the compatible string of the first USB controller to enable
device mode.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
 1 file changed, 2 insertions(+)

Comments

Peter Chen July 6, 2017, 1:24 a.m. | #1
On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Override the compatible string of the first USB controller to enable
> device mode.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
> index b902ab594afa..96b4f3f9827b 100644
> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
> @@ -336,7 +336,9 @@
>  	};
>  
>  	usb@c5000000 {
> +		compatible = "nvidia,tegra20-udc";
>  		status = "okay";
> +		dr_mode = "otg";

If this board supports peripheral-only, you need to
set dr_mode as "peripheral".
Dmitry Osipenko July 6, 2017, 10:24 a.m. | #2
On 06.07.2017 04:24, Peter Chen wrote:
> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Override the compatible string of the first USB controller to enable
>> device mode.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
>> index b902ab594afa..96b4f3f9827b 100644
>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>> @@ -336,7 +336,9 @@
>>  	};
>>  
>>  	usb@c5000000 {
>> +		compatible = "nvidia,tegra20-udc";
>>  		status = "okay";
>> +		dr_mode = "otg";
> 
> If this board supports peripheral-only, you need to
> set dr_mode as "peripheral".
> 

It should support OTG, but we aren't going to implement the host mode switch for
now. Stephen claims that the host mode is working with the tegra-ehci driver, it
would be nice if anyone could confirm this.
Dmitry Osipenko July 6, 2017, 10:43 a.m. | #3
On 06.07.2017 13:24, Dmitry Osipenko wrote:
> On 06.07.2017 04:24, Peter Chen wrote:
>> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Override the compatible string of the first USB controller to enable
>>> device mode.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
>>> index b902ab594afa..96b4f3f9827b 100644
>>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>>> @@ -336,7 +336,9 @@
>>>  	};
>>>  
>>>  	usb@c5000000 {
>>> +		compatible = "nvidia,tegra20-udc";
>>>  		status = "okay";
>>> +		dr_mode = "otg";
>>
>> If this board supports peripheral-only, you need to
>> set dr_mode as "peripheral".
>>
> 
> It should support OTG, but we aren't going to implement the host mode switch for
> now. Stephen claims that the host mode is working with the tegra-ehci driver, it
> would be nice if anyone could confirm this.
> 

Stephen, git blame tells that it is you enabled that port on TrimSlice and
AC100. Could you test them again please?
Marcel Ziswiler July 6, 2017, 12:51 p.m. | #4
On Thu, 2017-07-06 at 13:24 +0300, Dmitry Osipenko wrote:
> On 06.07.2017 04:24, Peter Chen wrote:

> > On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:

> > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane

> > > .org>

> > > 

> > > Override the compatible string of the first USB controller to

> > > enable

> > > device mode.

> > > 

> > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@pub

> > > lic.gmane.org>

> > > ---

> > >  arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++

> > >  1 file changed, 2 insertions(+)

> > > 

> > > diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts

> > > b/arch/arm/boot/dts/tegra20-trimslice.dts

> > > index b902ab594afa..96b4f3f9827b 100644

> > > --- a/arch/arm/boot/dts/tegra20-trimslice.dts

> > > +++ b/arch/arm/boot/dts/tegra20-trimslice.dts

> > > @@ -336,7 +336,9 @@

> > >  	};

> > >  

> > >  	usb@c5000000 {

> > > +		compatible = "nvidia,tegra20-udc";

> > >  		status = "okay";

> > > +		dr_mode = "otg";

> > 

> > If this board supports peripheral-only, you need to

> > set dr_mode as "peripheral".

> > 

> 

> It should support OTG, but we aren't going to implement the host mode

> switch for

> now. Stephen claims that the host mode is working with the tegra-ehci 

> driver, it

> would be nice if anyone could confirm this.


I can confirm that on Colibri T20, Apalis/Colibri T30 and Apalis TK1
host mode works just fine with the regular tegra-ehci driver.

I'm about to give your patch set a try on those boards as well and will
give further feedback.
Dmitry Osipenko July 6, 2017, 4:15 p.m. | #5
On 06.07.2017 15:51, Marcel Ziswiler wrote:
> On Thu, 2017-07-06 at 13:24 +0300, Dmitry Osipenko wrote:
>> On 06.07.2017 04:24, Peter Chen wrote:
>>> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>>>> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane
>>>> .org>
>>>>
>>>> Override the compatible string of the first USB controller to
>>>> enable
>>>> device mode.
>>>>
>>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@pub
>>>> lic.gmane.org>
>>>> ---
>>>>  arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> index b902ab594afa..96b4f3f9827b 100644
>>>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> @@ -336,7 +336,9 @@
>>>>  	};
>>>>  
>>>>  	usb@c5000000 {
>>>> +		compatible = "nvidia,tegra20-udc";
>>>>  		status = "okay";
>>>> +		dr_mode = "otg";
>>>
>>> If this board supports peripheral-only, you need to
>>> set dr_mode as "peripheral".
>>>
>>
>> It should support OTG, but we aren't going to implement the host mode
>> switch for
>> now. Stephen claims that the host mode is working with the tegra-ehci 
>> driver, it
>> would be nice if anyone could confirm this.
> 
> I can confirm that on Colibri T20, Apalis/Colibri T30 and Apalis TK1
> host mode works just fine with the regular tegra-ehci driver.
> 

Good to know, thank you :)

> I'm about to give your patch set a try on those boards as well and will
> give further feedback.
> 

Okay, but we won't be able to switch those boards to the UDC driver until the
peripheral/host mode switching would be implemented. As Stephen pointed, it
would be a user-visible regression.
Stephen Warren July 6, 2017, 4:35 p.m. | #6
On 07/05/2017 11:19 AM, Dmitry Osipenko wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Override the compatible string of the first USB controller to enable
> device mode.

> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts

>   	usb@c5000000 {
> +		compatible = "nvidia,tegra20-udc";
>   		status = "okay";
> +		dr_mode = "otg";
>   	};

As mentioned elsewhere in this thread, there is a mux between this Tegra 
USB port and the external micro USB connector and the internal USB 
USB-SATA adapter, such that the micro USB connector is routed to Tegra 
in USB recovery mode, but the USB-SATA adapter is routed to Tegra at 
other times. Making the change above will prevent using the USB-SATA 
adapter, so I don't think we should make this change.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren July 6, 2017, 4:46 p.m. | #7
On 07/05/2017 07:24 PM, Peter Chen wrote:
> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Override the compatible string of the first USB controller to enable
>> device mode.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>   arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
>> index b902ab594afa..96b4f3f9827b 100644
>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>> @@ -336,7 +336,9 @@
>>   	};
>>   
>>   	usb@c5000000 {
>> +		compatible = "nvidia,tegra20-udc";
>>   		status = "okay";
>> +		dr_mode = "otg";
> 
> If this board supports peripheral-only, you need to
> set dr_mode as "peripheral".

I agree here. Even if the ports can theoretically do otg, and DT is 
supposed to represent HW not SW, given we have no SW support for actual 
OTG, it'd be best not to change the DT to OTG until it's actually useful 
to do so. This comment applies to the patches I ack'd already too.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko July 6, 2017, 5:16 p.m. | #8
On 06.07.2017 19:46, Stephen Warren wrote:
> On 07/05/2017 07:24 PM, Peter Chen wrote:
>> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Override the compatible string of the first USB controller to enable
>>> device mode.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>   arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts
>>> b/arch/arm/boot/dts/tegra20-trimslice.dts
>>> index b902ab594afa..96b4f3f9827b 100644
>>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>>> @@ -336,7 +336,9 @@
>>>       };
>>>         usb@c5000000 {
>>> +        compatible = "nvidia,tegra20-udc";
>>>           status = "okay";
>>> +        dr_mode = "otg";
>>
>> If this board supports peripheral-only, you need to
>> set dr_mode as "peripheral".
> 
> I agree here. Even if the ports can theoretically do otg, and DT is supposed to
> represent HW not SW, given we have no SW support for actual OTG, it'd be best
> not to change the DT to OTG until it's actually useful to do so. This comment
> applies to the patches I ack'd already too.

In case of absence of the "dr_mode" option or without a support of host mode
switching, UDC driver fallbacks to the peripheral mode. Given that DT is
supposed to be stable, I'm not sure what to do... should I specify the
peripheral mode for now (which doesn't represent the HW) or completely remove
the "dr_mode"?
Stephen Warren July 6, 2017, 6:03 p.m. | #9
On 07/06/2017 11:16 AM, Dmitry Osipenko wrote:
> On 06.07.2017 19:46, Stephen Warren wrote:
>> On 07/05/2017 07:24 PM, Peter Chen wrote:
>>> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> Override the compatible string of the first USB controller to enable
>>>> device mode.
>>>>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>    arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> index b902ab594afa..96b4f3f9827b 100644
>>>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>> @@ -336,7 +336,9 @@
>>>>        };
>>>>          usb@c5000000 {
>>>> +        compatible = "nvidia,tegra20-udc";
>>>>            status = "okay";
>>>> +        dr_mode = "otg";
>>>
>>> If this board supports peripheral-only, you need to
>>> set dr_mode as "peripheral".
>>
>> I agree here. Even if the ports can theoretically do otg, and DT is supposed to
>> represent HW not SW, given we have no SW support for actual OTG, it'd be best
>> not to change the DT to OTG until it's actually useful to do so. This comment
>> applies to the patches I ack'd already too.
> 
> In case of absence of the "dr_mode" option or without a support of host mode
> switching, UDC driver fallbacks to the peripheral mode. Given that DT is
> supposed to be stable, I'm not sure what to do... should I specify the
> peripheral mode for now (which doesn't represent the HW) or completely remove
> the "dr_mode"?

I think specifying dr_mode="peripheral" now is fine; we can change it later.

DT ABI compatibility isn't about never changing the DT content, it's 
about making sure that any schema changes occur in a 
backwards-compatible fashion.

Also in the case of a USB port that does support OTG, the dr_mode 
property represents both (a) the HW's capabilities, and (b) the user's 
desired (initial, if e.g. sysfs modification is possible) mode of 
operation if the HW has multiple capabilities. Given (b), I think it's 
fine to specify peripheral mode for now, and allow the user to change it 
back to host or OTG mode if they want (although OTG mode isn't 
useful/valid until SW support exists).
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko July 6, 2017, 7:14 p.m. | #10
On 06.07.2017 21:03, Stephen Warren wrote:
> On 07/06/2017 11:16 AM, Dmitry Osipenko wrote:
>> On 06.07.2017 19:46, Stephen Warren wrote:
>>> On 07/05/2017 07:24 PM, Peter Chen wrote:
>>>> On Wed, Jul 05, 2017 at 08:19:53PM +0300, Dmitry Osipenko wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> Override the compatible string of the first USB controller to enable
>>>>> device mode.
>>>>>
>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>> ---
>>>>>    arch/arm/boot/dts/tegra20-trimslice.dts | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>>> b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>>> index b902ab594afa..96b4f3f9827b 100644
>>>>> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
>>>>> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
>>>>> @@ -336,7 +336,9 @@
>>>>>        };
>>>>>          usb@c5000000 {
>>>>> +        compatible = "nvidia,tegra20-udc";
>>>>>            status = "okay";
>>>>> +        dr_mode = "otg";
>>>>
>>>> If this board supports peripheral-only, you need to
>>>> set dr_mode as "peripheral".
>>>
>>> I agree here. Even if the ports can theoretically do otg, and DT is supposed to
>>> represent HW not SW, given we have no SW support for actual OTG, it'd be best
>>> not to change the DT to OTG until it's actually useful to do so. This comment
>>> applies to the patches I ack'd already too.
>>
>> In case of absence of the "dr_mode" option or without a support of host mode
>> switching, UDC driver fallbacks to the peripheral mode. Given that DT is
>> supposed to be stable, I'm not sure what to do... should I specify the
>> peripheral mode for now (which doesn't represent the HW) or completely remove
>> the "dr_mode"?
> 
> I think specifying dr_mode="peripheral" now is fine; we can change it later.
> 
> DT ABI compatibility isn't about never changing the DT content, it's about
> making sure that any schema changes occur in a backwards-compatible fashion.
> 
> Also in the case of a USB port that does support OTG, the dr_mode property
> represents both (a) the HW's capabilities, and (b) the user's desired (initial,
> if e.g. sysfs modification is possible) mode of operation if the HW has multiple
> capabilities. Given (b), I think it's fine to specify peripheral mode for now,
> and allow the user to change it back to host or OTG mode if they want (although
> OTG mode isn't useful/valid until SW support exists).

Thank you for the clarification.

Patch

diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
index b902ab594afa..96b4f3f9827b 100644
--- a/arch/arm/boot/dts/tegra20-trimslice.dts
+++ b/arch/arm/boot/dts/tegra20-trimslice.dts
@@ -336,7 +336,9 @@ 
 	};
 
 	usb@c5000000 {
+		compatible = "nvidia,tegra20-udc";
 		status = "okay";
+		dr_mode = "otg";
 	};
 
 	usb-phy@c5000000 {