[PATCH v3 00/12] Remove unnecessary USBx dr_mode configuration.
For all TI platforms handled in this series, USBx dr_mode was configured as peripheral in '<board>-u-boot.dtsi'. This series removes these fragments, and modifies dwc3 and cdns3 USB drivers so that they override 'otg' to 'peripheral' mode.
This follows a discussion about a similar topic for am3 boards: https://lore.kernel.org/u-boot/20230621-fix_usb_ether_init-v4-0-5f4977bb7678...
Signed-off-by: Julien Panis jpanis@baylibre.com --- Changes in v3: - Handle 'otg' mode as 'peripheral' in cdns3 usb driver, which is used by am64/j7200/j721e platforms. - Link to v2: https://lore.kernel.org/r/20230706-handle-otg-as-periph-v2-0-e26c028a2b28@ba...
Changes in v2: - Handle 'unknown' mode as 'otg' in dwc3 usb driver. - Link to v1: https://lore.kernel.org/r/20230706-handle-otg-as-periph-v1-0-bd85d15ffd50@ba...
--- Julien Panis (12): usb: dwc3: Handle unknown mode as otg arm: dts: dra7-evm-u-boot: Remove usb1 mode configuration arm: dts: dra71-evm-u-boot: Remove usb1 mode configuration arm: dts: dra72-evm-revc-u-boot: Remove usb1 mode configuration arm: dts: dra72-evm-u-boot: Remove usb1 mode configuration arm: dts: dra76-evm-u-boot: Remove usb1 mode configuration arm: dts: keystone-k2e-evm-u-boot: Remove usb1 mode configuration arm: dts: k3-am654-r5-base-board-u-boot: Remove usb mode configuration usb: cdns3: Handle otg mode as peripheral arm: dts: k3-am642-evm-u-boot: Remove usb0 mode configuration arm: dts: k3-j7200-common-proc-board-u-boot: Remove usb0 mode configuration arm: dts: k3-j721e-common-proc-board-u-boot: Remove usb0 mode configuration
arch/arm/dts/dra7-evm-u-boot.dtsi | 1 - arch/arm/dts/dra71-evm-u-boot.dtsi | 1 - arch/arm/dts/dra72-evm-revc-u-boot.dtsi | 1 - arch/arm/dts/dra72-evm-u-boot.dtsi | 1 - arch/arm/dts/dra76-evm-u-boot.dtsi | 1 - arch/arm/dts/k3-am642-evm-u-boot.dtsi | 1 - arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi | 5 ----- arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi | 1 - arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi | 1 - arch/arm/dts/keystone-k2e-evm-u-boot.dtsi | 1 - drivers/usb/cdns3/core.c | 4 ++++ drivers/usb/dwc3/dwc3-generic.c | 4 ++-- 12 files changed, 6 insertions(+), 16 deletions(-) --- base-commit: 19b77d3d23966a0d6dbb3c86187765f11100fb6f change-id: 20230706-handle-otg-as-periph-4546e874cd15
Best regards,
In case dr_mode attribute is not passed via DT, USB DRD controllers should default to OTG.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- drivers/usb/dwc3/dwc3-generic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 66da5a8d6f8c..857ae806e11d 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -182,8 +182,8 @@ static int dwc3_generic_of_to_plat(struct udevice *dev) 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; + pr_info("No USB mode specified. Using OTG.\n"); + plat->dr_mode = USB_DR_MODE_OTG; } }
On Thu, Jul 13, 2023 at 03:45:34PM +0200, Julien Panis wrote:
In case dr_mode attribute is not passed via DT, USB DRD controllers should default to OTG.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org
drivers/usb/dwc3/dwc3-generic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 66da5a8d6f8c..857ae806e11d 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -182,8 +182,8 @@ static int dwc3_generic_of_to_plat(struct udevice *dev) 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;
pr_info("No USB mode specified. Using OTG.\n");plat->dr_mode = USB_DR_MODE_OTG;
Is this part OK with you Marek?
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/dra7-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/dra7-evm-u-boot.dtsi b/arch/arm/dts/dra7-evm-u-boot.dtsi index 87b2451a8e8b..f6fd21f2c660 100644 --- a/arch/arm/dts/dra7-evm-u-boot.dtsi +++ b/arch/arm/dts/dra7-evm-u-boot.dtsi @@ -39,7 +39,6 @@
&usb1 { bootph-pre-ram; - dr_mode = "peripheral"; };
&usb2_phy1 {
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/dra71-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/dra71-evm-u-boot.dtsi b/arch/arm/dts/dra71-evm-u-boot.dtsi index 8e7dc719bf85..48a194c941d7 100644 --- a/arch/arm/dts/dra71-evm-u-boot.dtsi +++ b/arch/arm/dts/dra71-evm-u-boot.dtsi @@ -51,7 +51,6 @@
&usb1 { bootph-pre-ram; - dr_mode = "peripheral"; };
&usb2_phy1 {
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/dra72-evm-revc-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/dra72-evm-revc-u-boot.dtsi b/arch/arm/dts/dra72-evm-revc-u-boot.dtsi index 8e7dc719bf85..48a194c941d7 100644 --- a/arch/arm/dts/dra72-evm-revc-u-boot.dtsi +++ b/arch/arm/dts/dra72-evm-revc-u-boot.dtsi @@ -51,7 +51,6 @@
&usb1 { bootph-pre-ram; - dr_mode = "peripheral"; };
&usb2_phy1 {
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/dra72-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/dra72-evm-u-boot.dtsi b/arch/arm/dts/dra72-evm-u-boot.dtsi index 91a3b6b742a0..18350ed08909 100644 --- a/arch/arm/dts/dra72-evm-u-boot.dtsi +++ b/arch/arm/dts/dra72-evm-u-boot.dtsi @@ -11,7 +11,6 @@
&usb1 { bootph-pre-ram; - dr_mode = "peripheral"; };
&usb2_phy1 {
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/dra76-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/dra76-evm-u-boot.dtsi b/arch/arm/dts/dra76-evm-u-boot.dtsi index 1216d93bdcd6..47a92e214d27 100644 --- a/arch/arm/dts/dra76-evm-u-boot.dtsi +++ b/arch/arm/dts/dra76-evm-u-boot.dtsi @@ -31,7 +31,6 @@
&usb1 { bootph-pre-ram; - dr_mode = "peripheral"; };
&usb2_phy1 {
USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/keystone-k2e-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/keystone-k2e-evm-u-boot.dtsi b/arch/arm/dts/keystone-k2e-evm-u-boot.dtsi index 970d452f0804..a75f78377c28 100644 --- a/arch/arm/dts/keystone-k2e-evm-u-boot.dtsi +++ b/arch/arm/dts/keystone-k2e-evm-u-boot.dtsi @@ -39,7 +39,6 @@ &usb1 { dwc3@25010000 { phys = <&usb1_phy>; - dr_mode = "peripheral"; snps,u2ss_inp3_quirk; status = "okay"; };
USB0 and USB1 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi b/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi index 4516ab1437e7..3ccd42985fd8 100644 --- a/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi +++ b/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi @@ -169,10 +169,6 @@ bootph-pre-ram; };
-&usb1 { - dr_mode = "peripheral"; -}; - &fss { bootph-pre-ram; }; @@ -198,7 +194,6 @@ &usb0 { pinctrl-names = "default"; pinctrl-0 = <&usb0_pins_default>; - dr_mode = "peripheral"; bootph-pre-ram; };
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org --- drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
+ /* u-boot doesn't yet support OTG so limit to PERIPHERAL */ + if (dr_mode == USB_DR_MODE_OTG) + dr_mode = USB_DR_MODE_PERIPHERAL; + #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
- /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
- if (dr_mode == USB_DR_MODE_OTG)
dr_mode = USB_DR_MODE_PERIPHERAL;#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
- /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
- if (dr_mode == USB_DR_MODE_OTG)
dr_mode = USB_DR_MODE_PERIPHERAL;#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
- /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
- if (dr_mode == USB_DR_MODE_OTG)
dr_mode = USB_DR_MODE_PERIPHERAL;#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
On 18/08/2023 21:48, Tom Rini wrote:
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
- /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
- if (dr_mode == USB_DR_MODE_OTG)
dr_mode = USB_DR_MODE_PERIPHERAL;#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
CDNS3 does not have an internal OTG state machine. We mostly really care about role switching and not the full OTG stack. There needs to be some kind of SW framework to do that in u-boot.
Marek could you please advise what can be done here?
On 8/18/23 21:26, Roger Quadros wrote:
On 18/08/2023 21:48, Tom Rini wrote:
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
dr_mode = best_dr_mode;
- /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
- if (dr_mode == USB_DR_MODE_OTG)
dr_mode = USB_DR_MODE_PERIPHERAL;- #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
CDNS3 does not have an internal OTG state machine. We mostly really care about role switching and not the full OTG stack. There needs to be some kind of SW framework to do that in u-boot.
Marek could you please advise what can be done here?
As far as I can tell, MX8MM (CI HDRC), STM32MP1 (DWC2) all can do the host/peripheral switching. Why is that a problem with CDNS3 ? I am not familiar with this controller btw.
+Peter & Pawel.
On 19/08/2023 02:47, Marek Vasut wrote:
On 8/18/23 21:26, Roger Quadros wrote:
On 18/08/2023 21:48, Tom Rini wrote:
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
Override 'otg' to 'peripheral' mode, since 'otg' mode is not yet supported by u-boot.
Signed-off-by: Julien Panis jpanis@baylibre.com Suggested-by: Roger Quadros rogerq@kernel.org
drivers/usb/cdns3/core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 644a9791b9c9..bd763fc593e1 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns) dr_mode = best_dr_mode; + /* u-boot doesn't yet support OTG so limit to PERIPHERAL */ + if (dr_mode == USB_DR_MODE_OTG) + dr_mode = USB_DR_MODE_PERIPHERAL;
#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
CDNS3 does not have an internal OTG state machine. We mostly really care about role switching and not the full OTG stack. There needs to be some kind of SW framework to do that in u-boot.
Marek could you please advise what can be done here?
As far as I can tell, MX8MM (CI HDRC), STM32MP1 (DWC2) all can do the host/peripheral switching. Why is that a problem with CDNS3 ? I am not familiar with this controller btw.
So for MX8MM and STM32MP1, do we support on the fly host/peripheral switching based on USB plug type? As we don't have interrupts, do you keep polling for ID status change?
CDNS3 does support host/peripheral switching. i.e. cdsn3_hw_role_state_machine() and drd.c. What we are missing is the interrupt logic to map ID pin status change to role change.
On 8/19/23 09:00, Roger Quadros wrote:
+Peter & Pawel.
On 19/08/2023 02:47, Marek Vasut wrote:
On 8/18/23 21:26, Roger Quadros wrote:
On 18/08/2023 21:48, Tom Rini wrote:
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote:
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
> Override 'otg' to 'peripheral' mode, since 'otg' mode > is not yet supported by u-boot. > > Signed-off-by: Julien Panis jpanis@baylibre.com > Suggested-by: Roger Quadros rogerq@kernel.org > --- > drivers/usb/cdns3/core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > index 644a9791b9c9..bd763fc593e1 100644 > --- a/drivers/usb/cdns3/core.c > +++ b/drivers/usb/cdns3/core.c > @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns) > dr_mode = best_dr_mode; > + /* u-boot doesn't yet support OTG so limit to PERIPHERAL */ > + if (dr_mode == USB_DR_MODE_OTG) > + dr_mode = USB_DR_MODE_PERIPHERAL; > + > #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) > if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { > ret = cdns3_host_init(cdns);
Julien, why don't we support otg mode here?
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
CDNS3 does not have an internal OTG state machine. We mostly really care about role switching and not the full OTG stack. There needs to be some kind of SW framework to do that in u-boot.
Marek could you please advise what can be done here?
As far as I can tell, MX8MM (CI HDRC), STM32MP1 (DWC2) all can do the host/peripheral switching. Why is that a problem with CDNS3 ? I am not familiar with this controller btw.
So for MX8MM and STM32MP1, do we support on the fly host/peripheral switching based on USB plug type? As we don't have interrupts, do you keep polling for ID status change?
Nope, you just plug in one cable or the other and run the matching command -- either 'usb reset' for Host or 'ums/dfu/...' for Peripheral .
CDNS3 does support host/peripheral switching. i.e. cdsn3_hw_role_state_machine() and drd.c. What we are missing is the interrupt logic to map ID pin status change to role change.
I don't think you need any interrupt logic for this to work ?
On 19/08/2023 15:20, Marek Vasut wrote:
On 8/19/23 09:00, Roger Quadros wrote:
+Peter & Pawel.
On 19/08/2023 02:47, Marek Vasut wrote:
On 8/18/23 21:26, Roger Quadros wrote:
On 18/08/2023 21:48, Tom Rini wrote:
On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
Hi Tom,
On 14/08/2023 20:17, Tom Rini wrote: > On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote: > >> Override 'otg' to 'peripheral' mode, since 'otg' mode >> is not yet supported by u-boot. >> >> Signed-off-by: Julien Panis jpanis@baylibre.com >> Suggested-by: Roger Quadros rogerq@kernel.org >> --- >> drivers/usb/cdns3/core.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >> index 644a9791b9c9..bd763fc593e1 100644 >> --- a/drivers/usb/cdns3/core.c >> +++ b/drivers/usb/cdns3/core.c >> @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns) >> dr_mode = best_dr_mode; >> + /* u-boot doesn't yet support OTG so limit to PERIPHERAL */ >> + if (dr_mode == USB_DR_MODE_OTG) >> + dr_mode = USB_DR_MODE_PERIPHERAL; >> + >> #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) >> if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { >> ret = cdns3_host_init(cdns); > > Julien, why don't we support otg mode here? >
dr_mode will never be OTG at this point as the previous if condition would have forced it to PERIPHERAL.
My understanding was that u-boot USB framework doesn't support OTG mode so we force it to PERIPHERAL.
Well, the first part of this series is "make unknown state be OTG" for DWC3, so we do in general (and Marek told me off-list he's used it on DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing / updating.
CDNS3 does not have an internal OTG state machine. We mostly really care about role switching and not the full OTG stack. There needs to be some kind of SW framework to do that in u-boot.
Marek could you please advise what can be done here?
As far as I can tell, MX8MM (CI HDRC), STM32MP1 (DWC2) all can do the host/peripheral switching. Why is that a problem with CDNS3 ? I am not familiar with this controller btw.
So for MX8MM and STM32MP1, do we support on the fly host/peripheral switching based on USB plug type? As we don't have interrupts, do you keep polling for ID status change?
Nope, you just plug in one cable or the other and run the matching command -- either 'usb reset' for Host or 'ums/dfu/...' for Peripheral .
OK.
CDNS3 does support host/peripheral switching. i.e. cdsn3_hw_role_state_machine() and drd.c. What we are missing is the interrupt logic to map ID pin status change to role change.
I don't think you need any interrupt logic for this to work ?
For the use case you described, no. Julien, I think this can be easily achieved with the current driver. Would be nice if driver prevents user from setting incorrect role than what cable is set to.
USB0 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/k3-am642-evm-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi b/arch/arm/dts/k3-am642-evm-u-boot.dtsi index 64857b09099d..f3f0d07df49f 100644 --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi @@ -56,7 +56,6 @@ };
&usb0 { - dr_mode="peripheral"; bootph-pre-ram; };
USB0 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi b/arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi index f57c2306ba1a..dbe22fc3a459 100644 --- a/arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi +++ b/arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi @@ -157,7 +157,6 @@ };
&usb0 { - dr_mode = "peripheral"; bootph-pre-ram; };
USB0 dual-role feature is already handled as peripheral only in dwc3-generic driver.
Signed-off-by: Julien Panis jpanis@baylibre.com Acked-by: Roger Quadros rogerq@kernel.org --- arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi index 867ec2bb1aff..d14f3346caf7 100644 --- a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi +++ b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi @@ -143,7 +143,6 @@ };
&usb0 { - dr_mode = "peripheral"; bootph-pre-ram; };
On 13/07/2023 16:45, Julien Panis wrote:
For all TI platforms handled in this series, USBx dr_mode was configured as peripheral in '<board>-u-boot.dtsi'. This series removes these fragments, and modifies dwc3 and cdns3 USB drivers so that they override 'otg' to 'peripheral' mode.
This follows a discussion about a similar topic for am3 boards: https://lore.kernel.org/u-boot/20230621-fix_usb_ether_init-v4-0-5f4977bb7678...
Signed-off-by: Julien Panis jpanis@baylibre.com
Changes in v3:
- Handle 'otg' mode as 'peripheral' in cdns3 usb driver, which is used by am64/j7200/j721e platforms.
- Link to v2: https://lore.kernel.org/r/20230706-handle-otg-as-periph-v2-0-e26c028a2b28@ba...
Changes in v2:
- Handle 'unknown' mode as 'otg' in dwc3 usb driver.
- Link to v1: https://lore.kernel.org/r/20230706-handle-otg-as-periph-v1-0-bd85d15ffd50@ba...
Julien Panis (12): usb: dwc3: Handle unknown mode as otg arm: dts: dra7-evm-u-boot: Remove usb1 mode configuration arm: dts: dra71-evm-u-boot: Remove usb1 mode configuration arm: dts: dra72-evm-revc-u-boot: Remove usb1 mode configuration arm: dts: dra72-evm-u-boot: Remove usb1 mode configuration arm: dts: dra76-evm-u-boot: Remove usb1 mode configuration arm: dts: keystone-k2e-evm-u-boot: Remove usb1 mode configuration arm: dts: k3-am654-r5-base-board-u-boot: Remove usb mode configuration usb: cdns3: Handle otg mode as peripheral arm: dts: k3-am642-evm-u-boot: Remove usb0 mode configuration arm: dts: k3-j7200-common-proc-board-u-boot: Remove usb0 mode configuration arm: dts: k3-j721e-common-proc-board-u-boot: Remove usb0 mode configuration
arch/arm/dts/dra7-evm-u-boot.dtsi | 1 - arch/arm/dts/dra71-evm-u-boot.dtsi | 1 - arch/arm/dts/dra72-evm-revc-u-boot.dtsi | 1 - arch/arm/dts/dra72-evm-u-boot.dtsi | 1 - arch/arm/dts/dra76-evm-u-boot.dtsi | 1 - arch/arm/dts/k3-am642-evm-u-boot.dtsi | 1 - arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi | 5 ----- arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi | 1 - arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi | 1 - arch/arm/dts/keystone-k2e-evm-u-boot.dtsi | 1 - drivers/usb/cdns3/core.c | 4 ++++ drivers/usb/dwc3/dwc3-generic.c | 4 ++-- 12 files changed, 6 insertions(+), 16 deletions(-)
base-commit: 19b77d3d23966a0d6dbb3c86187765f11100fb6f change-id: 20230706-handle-otg-as-periph-4546e874cd15
for this series: Acked-by: Roger Quadros rogerq@kernel.org
participants (4)
-
Julien Panis -
Marek Vasut -
Roger Quadros -
Tom Rini