* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-26 21:32 Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-07-26 21:32 UTC (permalink / raw)
To: Thinh Nguyen; +Cc: Felipe Balbi, USB, John Youn
On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
<Thinh.Nguyen@synopsys.com> wrote:
> dwc_usb31 does not support OTG mode. If the controller supports DRD but
> the dr_mode is not specified or set to OTG, then set the mode to
> peripheral.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
> drivers/usb/dwc3/core.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 21e4931d0cc0..64ba664d467c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
> mode = USB_DR_MODE_HOST;
> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
> mode = USB_DR_MODE_PERIPHERAL;
> +
> + /*
> + * dwc_usb31 does not support OTG mode. If the controller
> + * supports DRD but the dr_mode is not specified or set to OTG,
> + * then set the mode to peripheral.
> + */
> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
shouldn't be simple
else if (dwc3_is_usb31(...))
?
> + mode = USB_DR_MODE_PERIPHERAL;
> }
>
> if (mode != dwc->dr_mode) {
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-27 10:22 Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-07-27 10:22 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Thinh Nguyen, USB, John Youn
On Fri, Jul 27, 2018 at 1:14 PM, Felipe Balbi <balbi@kernel.org> wrote:
>>>>>> +
>>>>>> + /*
>>>>>> + * dwc_usb31 does not support OTG mode. If the controller
>>>>>> + * supports DRD but the dr_mode is not specified or set to OTG,
>>>>>> + * then set the mode to peripheral.
>>>>>> + */
>>>>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>>>>> shouldn't be simple
>>>>>
>>>>> else if (dwc3_is_usb31(...))
>>>>>
>>>>> ?
>>>
>>> Actually, no. We want to set the mode to peripheral _only_ when dr_mode
>>> was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
>>> not enough.
>>
>> How come?
>>
>> If I read the code correctly...
>>
>> When you go to default case in this switch it's possible if and only
>> if you have mode _exactly_ OTG. You can't have mode unknown here
>> either.
>> The check is redundant and absence of else adds additional burden on
>> the all the rest cases.
>
> Look a little closer
>
>> mode = dwc->dr_mode;
>> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>
>> switch (hw_mode) {
> ^^^^^^^
> Switching on hw_mode, not mode.
Ah, indeed. That's what I have missed.
Thanks for detailed explanation!
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-27 10:14 Felipe Balbi
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2018-07-27 10:14 UTC (permalink / raw)
To: Andy Shevchenko, Thinh Nguyen; +Cc: USB, John Youn
Hi,
Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> On Fri, Jul 27, 2018 at 2:02 AM, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>> On 7/26/2018 2:59 PM, Thinh Nguyen wrote:
>>> On 7/26/2018 2:32 PM, Andy Shevchenko wrote:
>>>> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
>>>> <Thinh.Nguyen@synopsys.com> wrote:
>>>>> dwc_usb31 does not support OTG mode. If the controller supports DRD but
>>>>> the dr_mode is not specified or set to OTG, then set the mode to
>>>>> peripheral.
>>>>>
>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>> ---
>>>>> drivers/usb/dwc3/core.c | 8 ++++++++
>>>>> 1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index 21e4931d0cc0..64ba664d467c 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
>>>>> mode = USB_DR_MODE_HOST;
>>>>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
>>>>> mode = USB_DR_MODE_PERIPHERAL;
>>>>> +
>>>>> + /*
>>>>> + * dwc_usb31 does not support OTG mode. If the controller
>>>>> + * supports DRD but the dr_mode is not specified or set to OTG,
>>>>> + * then set the mode to peripheral.
>>>>> + */
>>>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>>>> shouldn't be simple
>>>>
>>>> else if (dwc3_is_usb31(...))
>>>>
>>>> ?
>>
>> Actually, no. We want to set the mode to peripheral _only_ when dr_mode
>> was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
>> not enough.
>
> How come?
>
> If I read the code correctly...
>
> When you go to default case in this switch it's possible if and only
> if you have mode _exactly_ OTG. You can't have mode unknown here
> either.
> The check is redundant and absence of else adds additional burden on
> the all the rest cases.
Look a little closer
> mode = dwc->dr_mode;
> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>
> switch (hw_mode) {
^^^^^^^
Switching on hw_mode, not mode.
> case DWC3_GHWPARAMS0_MODE_GADGET:
> if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) {
> dev_err(dev,
> "Controller does not support host mode.\n");
> return -EINVAL;
> }
> mode = USB_DR_MODE_PERIPHERAL;
> break;
> case DWC3_GHWPARAMS0_MODE_HOST:
> if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) {
> dev_err(dev,
> "Controller does not support device mode.\n");
> return -EINVAL;
> }
> mode = USB_DR_MODE_HOST;
> break;
> default:
> if (IS_ENABLED(CONFIG_USB_DWC3_HOST))
> mode = USB_DR_MODE_HOST;
> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
> mode = USB_DR_MODE_PERIPHERAL;
both of these are checking for Kconfig choices.
> /*
> * dwc_usb31 does not support OTG mode. If the controller
> * supports DRD but the dr_mode is not specified or set to OTG,
> * then set the mode to peripheral.
> */
> if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
^^^^^^^^^^^^^^^^^^^^^^^
making sure device_property "dr_mode" is OTG. Note if I
don't very that mode is OTG, all I know for sure is that HW
is configured for DRD operation and Kconfig enabled support
for both Host and Peripheral roles, but I have not yet
verified e.g. DeviceTree.
> mode = USB_DR_MODE_PERIPHERAL;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-27 10:08 Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-07-27 10:08 UTC (permalink / raw)
To: Thinh Nguyen; +Cc: Felipe Balbi, USB, John Youn
On Fri, Jul 27, 2018 at 2:02 AM, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> On 7/26/2018 2:59 PM, Thinh Nguyen wrote:
>> On 7/26/2018 2:32 PM, Andy Shevchenko wrote:
>>> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
>>> <Thinh.Nguyen@synopsys.com> wrote:
>>>> dwc_usb31 does not support OTG mode. If the controller supports DRD but
>>>> the dr_mode is not specified or set to OTG, then set the mode to
>>>> peripheral.
>>>>
>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>> ---
>>>> drivers/usb/dwc3/core.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 21e4931d0cc0..64ba664d467c 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
>>>> mode = USB_DR_MODE_HOST;
>>>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
>>>> mode = USB_DR_MODE_PERIPHERAL;
>>>> +
>>>> + /*
>>>> + * dwc_usb31 does not support OTG mode. If the controller
>>>> + * supports DRD but the dr_mode is not specified or set to OTG,
>>>> + * then set the mode to peripheral.
>>>> + */
>>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>>> shouldn't be simple
>>>
>>> else if (dwc3_is_usb31(...))
>>>
>>> ?
>
> Actually, no. We want to set the mode to peripheral _only_ when dr_mode
> was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
> not enough.
How come?
If I read the code correctly...
When you go to default case in this switch it's possible if and only
if you have mode _exactly_ OTG. You can't have mode unknown here
either.
The check is redundant and absence of else adds additional burden on
the all the rest cases.
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-26 23:02 Thinh Nguyen
0 siblings, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2018-07-26 23:02 UTC (permalink / raw)
To: Thinh Nguyen, Andy Shevchenko; +Cc: Felipe Balbi, USB, John Youn
On 7/26/2018 2:59 PM, Thinh Nguyen wrote:
> On 7/26/2018 2:32 PM, Andy Shevchenko wrote:
>> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
>> <Thinh.Nguyen@synopsys.com> wrote:
>>> dwc_usb31 does not support OTG mode. If the controller supports DRD but
>>> the dr_mode is not specified or set to OTG, then set the mode to
>>> peripheral.
>>>
>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>> ---
>>> drivers/usb/dwc3/core.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 21e4931d0cc0..64ba664d467c 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
>>> mode = USB_DR_MODE_HOST;
>>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
>>> mode = USB_DR_MODE_PERIPHERAL;
>>> +
>>> + /*
>>> + * dwc_usb31 does not support OTG mode. If the controller
>>> + * supports DRD but the dr_mode is not specified or set to OTG,
>>> + * then set the mode to peripheral.
>>> + */
>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>> shouldn't be simple
>>
>> else if (dwc3_is_usb31(...))
>>
>> ?
Actually, no. We want to set the mode to peripheral _only_ when dr_mode
was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
not enough.
Thanks,
Thinh
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-26 21:59 Thinh Nguyen
0 siblings, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2018-07-26 21:59 UTC (permalink / raw)
To: Andy Shevchenko, Thinh Nguyen; +Cc: Felipe Balbi, USB, John Youn
On 7/26/2018 2:32 PM, Andy Shevchenko wrote:
> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
> <Thinh.Nguyen@synopsys.com> wrote:
>> dwc_usb31 does not support OTG mode. If the controller supports DRD but
>> the dr_mode is not specified or set to OTG, then set the mode to
>> peripheral.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>> drivers/usb/dwc3/core.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 21e4931d0cc0..64ba664d467c 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
>> mode = USB_DR_MODE_HOST;
>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
>> mode = USB_DR_MODE_PERIPHERAL;
>> +
>> + /*
>> + * dwc_usb31 does not support OTG mode. If the controller
>> + * supports DRD but the dr_mode is not specified or set to OTG,
>> + * then set the mode to peripheral.
>> + */
>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
> shouldn't be simple
>
> else if (dwc3_is_usb31(...))
>
> ?
Yes you're right. :)
Thanks,
Thinh
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* usb: dwc3: Set default mode for dwc_usb31
@ 2018-07-26 20:52 Thinh Nguyen
0 siblings, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2018-07-26 20:52 UTC (permalink / raw)
To: Felipe Balbi, linux-usb; +Cc: John Youn
dwc_usb31 does not support OTG mode. If the controller supports DRD but
the dr_mode is not specified or set to OTG, then set the mode to
peripheral.
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
drivers/usb/dwc3/core.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 21e4931d0cc0..64ba664d467c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
mode = USB_DR_MODE_HOST;
else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
mode = USB_DR_MODE_PERIPHERAL;
+
+ /*
+ * dwc_usb31 does not support OTG mode. If the controller
+ * supports DRD but the dr_mode is not specified or set to OTG,
+ * then set the mode to peripheral.
+ */
+ if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
+ mode = USB_DR_MODE_PERIPHERAL;
}
if (mode != dwc->dr_mode) {
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-07-27 10:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-26 21:32 usb: dwc3: Set default mode for dwc_usb31 Andy Shevchenko
-- strict thread matches above, loose matches on Subject: below --
2018-07-27 10:22 Andy Shevchenko
2018-07-27 10:14 Felipe Balbi
2018-07-27 10:08 Andy Shevchenko
2018-07-26 23:02 Thinh Nguyen
2018-07-26 21:59 Thinh Nguyen
2018-07-26 20:52 Thinh Nguyen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.