| Message ID | 20241016-topic-usb-dwc3-dr-mode-otg-v1-1-4fda56072723@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3-generic: use DR_MODE_OTG when dr_mode isn't specified | expand |
On 10/16/24 10:26 AM, Neil Armstrong wrote: > The USB DRD bindingsa at [1] are clear, if not specified the dr_mode > property defaults to otg, and this is how Linux behaves. > > A cleanup is ongoing on the Linux Device Trees to remove dr_mode when > set to "otg", so take this in account and do not fail anymore when > dr_mode isn't specified in the glue or dwc3 node. > > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-drd.yaml > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> Is there a matching Linux kernel commit which implements this behavior in Linux ?
Hi, On 16/10/2024 13:22, Marek Vasut wrote: > On 10/16/24 10:26 AM, Neil Armstrong wrote: >> The USB DRD bindingsa at [1] are clear, if not specified the dr_mode >> property defaults to otg, and this is how Linux behaves. >> >> A cleanup is ongoing on the Linux Device Trees to remove dr_mode when >> set to "otg", so take this in account and do not fail anymore when >> dr_mode isn't specified in the glue or dwc3 node. >> >> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-drd.yaml >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > Is there a matching Linux kernel commit which implements this behavior in Linux ? In DWC3, the logic is there since v3.12-rc1 in commit a45c82b84c844 ("usb: dwc3: adapt to use dr_mode device tree helper") [1] ``` if (dwc->dr_mode == USB_DR_MODE_UNKNOWN) dwc->dr_mode = USB_DR_MODE_OTG; ``` [1] https://github.com/torvalds/linux/commit/a45c82b84c844cd85b2ed1aa32596f1bffa32716 Neil
On 16/10/2024 10:26, Neil Armstrong via groups.io wrote: > The USB DRD bindingsa at [1] are clear, if not specified the dr_mode > property defaults to otg, and this is how Linux behaves. I guess this is (kind of?) a subset of https://lore.kernel.org/u-boot/20240621021135.1600847-2-caleb.connolly@linaro.org/ I need to respin that one... > > A cleanup is ongoing on the Linux Device Trees to remove dr_mode when > set to "otg", so take this in account and do not fail anymore when > dr_mode isn't specified in the glue or dwc3 node. Definitely an improvement over the status quo heh Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> > > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-drd.yaml > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/usb/dwc3/dwc3-generic.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c > index 2ab41cbae45..d62d687ece3 100644 > --- a/drivers/usb/dwc3/dwc3-generic.c > +++ b/drivers/usb/dwc3/dwc3-generic.c > @@ -183,10 +183,10 @@ static int dwc3_generic_of_to_plat(struct udevice *dev) > /* might be a leaf so check the parent for mode */ > node = dev_ofnode(dev->parent); > plat->dr_mode = usb_get_dr_mode(node); > - if (plat->dr_mode == USB_DR_MODE_UNKNOWN) { > - pr_err("Invalid usb mode setup\n"); > - return -ENODEV; > - } > + > + /* If none of the nodes have dr_mode, bindings says default is OTG */ > + if (!plat->dr_mode) > + plat->dr_mode = USB_DR_MODE_OTG; > } > > return 0; > @@ -527,6 +527,10 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node) > if (!dr_mode) > dr_mode = usb_get_dr_mode(node); > > + /* If none of the nodes have dr_mode, bindings says default is OTG */ > + if (!dr_mode) > + dr_mode = USB_DR_MODE_OTG; > + > if (CONFIG_IS_ENABLED(DM_USB_GADGET) && > (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) { > debug("%s: dr_mode: OTG or Peripheral\n", __func__); > > --- > base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae > change-id: 20241016-topic-usb-dwc3-dr-mode-otg-88ec307e0970 > > Best regards,
On 10/17/24 1:07 PM, Caleb Connolly wrote: > > > On 16/10/2024 10:26, Neil Armstrong via groups.io wrote: >> The USB DRD bindingsa at [1] are clear, if not specified the dr_mode >> property defaults to otg, and this is how Linux behaves. > > I guess this is (kind of?) a subset of > > https://lore.kernel.org/u-boot/20240621021135.1600847-2- > caleb.connolly@linaro.org/ > > I need to respin that one... It would be good to include that information in the commit message ... >> A cleanup is ongoing on the Linux Device Trees to remove dr_mode when >> set to "otg", so take this in account and do not fail anymore when >> dr_mode isn't specified in the glue or dwc3 node. > > Definitely an improvement over the status quo heh > > Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> ... and eventually synchronize the DWC3 driver with current Linux kernel state of it.
On 17/10/2024 15:52, Marek Vasut wrote: > On 10/17/24 1:07 PM, Caleb Connolly wrote: >> >> >> On 16/10/2024 10:26, Neil Armstrong via groups.io wrote: >>> The USB DRD bindingsa at [1] are clear, if not specified the dr_mode >>> property defaults to otg, and this is how Linux behaves. >> >> I guess this is (kind of?) a subset of >> >> https://lore.kernel.org/u-boot/20240621021135.1600847-2- caleb.connolly@linaro.org/ Yes and no, it should be on top of mine >> >> I need to respin that one... > > It would be good to include that information in the commit message ... Will do > >>> A cleanup is ongoing on the Linux Device Trees to remove dr_mode when >>> set to "otg", so take this in account and do not fail anymore when >>> dr_mode isn't specified in the glue or dwc3 node. >> >> Definitely an improvement over the status quo heh >> >> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> > ... and eventually synchronize the DWC3 driver with current Linux kernel state of it. Well, this behavior should've been in U-Boot since the beginning! But yeah, it's should be done and it's another subject because it would impact a lot of boards. Neil >
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..d62d687ece3 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -183,10 +183,10 @@ static int dwc3_generic_of_to_plat(struct udevice *dev) /* might be a leaf so check the parent for mode */ node = dev_ofnode(dev->parent); plat->dr_mode = usb_get_dr_mode(node); - if (plat->dr_mode == USB_DR_MODE_UNKNOWN) { - pr_err("Invalid usb mode setup\n"); - return -ENODEV; - } + + /* If none of the nodes have dr_mode, bindings says default is OTG */ + if (!plat->dr_mode) + plat->dr_mode = USB_DR_MODE_OTG; } return 0; @@ -527,6 +527,10 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node) if (!dr_mode) dr_mode = usb_get_dr_mode(node); + /* If none of the nodes have dr_mode, bindings says default is OTG */ + if (!dr_mode) + dr_mode = USB_DR_MODE_OTG; + if (CONFIG_IS_ENABLED(DM_USB_GADGET) && (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) { debug("%s: dr_mode: OTG or Peripheral\n", __func__);
The USB DRD bindingsa at [1] are clear, if not specified the dr_mode property defaults to otg, and this is how Linux behaves. A cleanup is ongoing on the Linux Device Trees to remove dr_mode when set to "otg", so take this in account and do not fail anymore when dr_mode isn't specified in the glue or dwc3 node. [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-drd.yaml Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/usb/dwc3/dwc3-generic.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) --- base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae change-id: 20241016-topic-usb-dwc3-dr-mode-otg-88ec307e0970 Best regards,