[PATCH v2 0/5] rockchip: rk3568: Use dwc3-generic driver

[PATCH v2 0/5] rockchip: rk3568: Use dwc3-generic driver

[Jonas Karlman]

This series add support for rk3568 in dwc3-generic driver and change to
use the dwc3-generic driver for rk3568 devices having usb enabled.

After these changes it should be possible to support usb gadget on
rk3568 with e.g.:

  # CONFIG_USB_FUNCTION_FASTBOOT is not set
  CONFIG_DM_USB_GADGET=y
  CONFIG_USB_GADGET=y

Patch 1 reverts addition of now unnecessary regulator-boot-on props.
Patch 2 improve support for childless glue/ctrl dt node.
Patch 3 relax a hard error when usb gadget support is not enabled.
Patch 4 add support for the glue/ctrl dt node used by rk3568.
Patch 5 change rk3568 devices to use dwc3-generic driver.

Changes in v2:
- Change to use CONFIG_IS_ENABLED for USB_HOST
- Refactor switch to if statements (Marek Vasut)
- Update commit message
- Collect r-b tag

This series is also available at [1]

[1] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3568-dwc3-generic-v2

Jonas Karlman (5):
  Revert "arm: dts: rockchip: radxa-cm3-io, rock-3a: enable regulators
    for usb"
  usb: dwc3-generic: Return early when there is no child node
  usb: dwc3-generic: Relax unsupported dr_mode check
  usb: dwc3-generic: Add rk3568 support
  rockchip: rk3568: Use dwc3-generic driver

 arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi |  4 --
 arch/arm/dts/rk3568-rock-3a-u-boot.dtsi      |  8 ----
 configs/radxa-cm3-io-rk3566_defconfig        |  2 +-
 configs/rock-3a-rk3568_defconfig             |  2 +-
 drivers/usb/dwc3/dwc3-generic.c              | 48 +++++++++++---------
 5 files changed, 29 insertions(+), 35 deletions(-)

-- 
2.41.0

[PATCH v2 1/5] Revert "arm: dts: rockchip: radxa-cm3-io, rock-3a: enable regulators for usb"

[Jonas Karlman]

Remove regulator-boot-on prop from regulators now that the phy core has
support for phy-supply after the commit c57e0dcd9384 ("phy: add support
for phy-supply").

This reverts commit 7911f409ff20dce5995cc1b703a6e30c94022f6b.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- No change

 arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi | 4 ----
 arch/arm/dts/rk3568-rock-3a-u-boot.dtsi      | 8 --------
 2 files changed, 12 deletions(-)

diff --git a/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi b/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
index f91740c1c0c8..57b77151c57c 100644
--- a/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
+++ b/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
@@ -77,7 +77,3 @@
 	bootph-all;
 	status = "okay";
 };
-
-&vcc5v0_usb30 {
-	regulator-boot-on;
-};
diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
index bbf54f888fa0..ecbbdeee32f8 100644
--- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
+++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
@@ -110,11 +110,3 @@
 	bootph-all;
 	status = "okay";
 };
-
-&vcc5v0_usb_host {
-	regulator-boot-on;
-};
-
-&vcc5v0_usb_hub {
-	regulator-boot-on;
-};
-- 
2.41.0

[PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Jonas Karlman]

The current error check for device_find_first_child is not working as
expected, the documentation for device_find_first_child mention:

  @devp: Returns first child device, or NULL if none
  Return: 0

Change to return early when there is no child node to avoid any possible
null pointer dereference.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- Update commit message

 drivers/usb/dwc3/dwc3-generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 35e4b36a695e..4d5d500aefab 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
 			return ret;
 	}
 
-	ret = device_find_first_child(dev, &child);
-	if (ret)
-		return ret;
+	device_find_first_child(dev, &child);
+	if (!child)
+		return 0;
 
 	if (glue->clks.count == 0) {
 		ret = dwc3_glue_clk_init(child, glue);
-- 
2.41.0

[PATCH v2 3/5] usb: dwc3-generic: Relax unsupported dr_mode check

[Jonas Karlman]

When dr_mode is peripheral or otg and U-Boot has not been built with
DM_USB_GADGET support, booting such device may end up with:

  dwc3_glue_bind_common: subnode name: usb@fcc00000
  Error binding driver 'dwc3-generic-wrapper': -6
  Some drivers failed to bind
  initcall sequence 00000000effbca08 failed at call 0000000000a217c8 (err=-6)
  ### ERROR ### Please RESET the board ###

Instead fail gracfully with ENODEV to allow board continue booting.

  dwc3_glue_bind_common: subnode name: usb@fcc00000
  dwc3_glue_bind_common: unsupported dr_mode

Also use CONFIG_IS_ENABLED(USB_HOST) and change switch to if statements
to improve readability of the code.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- Change to use CONFIG_IS_ENABLED for USB_HOST
- Refactor switch to if statements (Marek Vasut)

 drivers/usb/dwc3/dwc3-generic.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 4d5d500aefab..6507219d6fc0 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -226,8 +226,7 @@ U_BOOT_DRIVER(dwc3_generic_peripheral) = {
 };
 #endif
 
-#if defined(CONFIG_SPL_USB_HOST) || \
-	!defined(CONFIG_SPL_BUILD) && defined(CONFIG_USB_HOST)
+#if CONFIG_IS_ENABLED(USB_HOST)
 static int dwc3_generic_host_probe(struct udevice *dev)
 {
 	struct xhci_hcor *hcor;
@@ -409,7 +408,7 @@ struct dwc3_glue_ops ti_ops = {
 static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
 {
 	const char *name = ofnode_get_name(node);
-	const char *driver = NULL;
+	const char *driver;
 	enum usb_dr_mode dr_mode;
 	struct udevice *dev;
 	int ret;
@@ -421,27 +420,17 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
 	if (!dr_mode)
 		dr_mode = usb_get_dr_mode(node);
 
-	switch (dr_mode) {
-	case USB_DR_MODE_PERIPHERAL:
-	case USB_DR_MODE_OTG:
-#if CONFIG_IS_ENABLED(DM_USB_GADGET)
+	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__);
 		driver = "dwc3-generic-peripheral";
-#endif
-		break;
-#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
-	case USB_DR_MODE_HOST:
+	} else if (CONFIG_IS_ENABLED(USB_HOST) && dr_mode == USB_DR_MODE_HOST) {
 		debug("%s: dr_mode: HOST\n", __func__);
 		driver = "dwc3-generic-host";
-		break;
-#endif
-	default:
+	} else {
 		debug("%s: unsupported dr_mode\n", __func__);
 		return -ENODEV;
-	};
-
-	if (!driver)
-		return -ENXIO;
+	}
 
 	ret = device_bind_driver_to_node(parent, driver, name,
 					 node, &dev);
-- 
2.41.0

[PATCH v2 4/5] usb: dwc3-generic: Add rk3568 support

[Jonas Karlman]

RK3568 share glue and ctrl in a single node. Use glue_get_ctrl_dev to
return the glue node as the ctrl node.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
---
v2:
- No change
- Collect r-b tag

 drivers/usb/dwc3/dwc3-generic.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 6507219d6fc0..54740e4e0abe 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -405,6 +405,22 @@ struct dwc3_glue_ops ti_ops = {
 	.glue_configure = dwc3_ti_glue_configure,
 };
 
+static int dwc3_rk_glue_get_ctrl_dev(struct udevice *dev, ofnode *node)
+{
+	if (!device_is_compatible(dev, "snps,dwc3"))
+		return -EINVAL;
+
+	*node = dev_ofnode(dev);
+	if (!ofnode_valid(*node))
+		return -EINVAL;
+
+	return 0;
+}
+
+struct dwc3_glue_ops rk_ops = {
+	.glue_get_ctrl_dev = dwc3_rk_glue_get_ctrl_dev,
+};
+
 static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
 {
 	const char *name = ofnode_get_name(node);
@@ -596,6 +612,7 @@ static const struct udevice_id dwc3_glue_ids[] = {
 	{ .compatible = "ti,am654-dwc3" },
 	{ .compatible = "rockchip,rk3328-dwc3" },
 	{ .compatible = "rockchip,rk3399-dwc3" },
+	{ .compatible = "rockchip,rk3568-dwc3", .data = (ulong)&rk_ops },
 	{ .compatible = "qcom,dwc3" },
 	{ .compatible = "fsl,imx8mp-dwc3", .data = (ulong)&imx8mp_ops },
 	{ .compatible = "fsl,imx8mq-dwc3" },
-- 
2.41.0

[PATCH v2 5/5] rockchip: rk3568: Use dwc3-generic driver

[Jonas Karlman]

Change RK3568 devices to use the newer dwc3-generic driver instead of
the old xhci-dwc3 driver for USB 3.0 support.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- No change

 configs/radxa-cm3-io-rk3566_defconfig | 2 +-
 configs/rock-3a-rk3568_defconfig      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/radxa-cm3-io-rk3566_defconfig b/configs/radxa-cm3-io-rk3566_defconfig
index 488723dfaa30..f89777184ceb 100644
--- a/configs/radxa-cm3-io-rk3566_defconfig
+++ b/configs/radxa-cm3-io-rk3566_defconfig
@@ -75,8 +75,8 @@ CONFIG_SYS_NS16550_MEM32=y
 CONFIG_SYSRESET=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
-CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
 CONFIG_ERRNO_STR=y
diff --git a/configs/rock-3a-rk3568_defconfig b/configs/rock-3a-rk3568_defconfig
index 753d03914d90..ddb43249654e 100644
--- a/configs/rock-3a-rk3568_defconfig
+++ b/configs/rock-3a-rk3568_defconfig
@@ -85,10 +85,10 @@ CONFIG_ROCKCHIP_SFC=y
 CONFIG_SYSRESET=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
-CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_OHCI_HCD=y
 CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
 CONFIG_ERRNO_STR=y
-- 
2.41.0

Re: [PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Marek Vasut]

On 7/16/23 01:10, Jonas Karlman wrote:
> The current error check for device_find_first_child is not working as
> expected, the documentation for device_find_first_child mention:
> 
>    @devp: Returns first child device, or NULL if none
>    Return: 0
> 
> Change to return early when there is no child node to avoid any possible
> null pointer dereference.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - Update commit message
> 
>   drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 35e4b36a695e..4d5d500aefab 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
>   			return ret;
>   	}
>   
> -	ret = device_find_first_child(dev, &child);
> -	if (ret)
> -		return ret;
> +	device_find_first_child(dev, &child);
> +	if (!child)
> +		return 0;

What does device_find_first_child() return in your case ? ENODEV or ENOSYS ?

Re: [PATCH v2 3/5] usb: dwc3-generic: Relax unsupported dr_mode check

[Marek Vasut]

On 7/16/23 01:10, Jonas Karlman wrote:
> When dr_mode is peripheral or otg and U-Boot has not been built with
> DM_USB_GADGET support, booting such device may end up with:
> 
>    dwc3_glue_bind_common: subnode name: usb@fcc00000
>    Error binding driver 'dwc3-generic-wrapper': -6
>    Some drivers failed to bind
>    initcall sequence 00000000effbca08 failed at call 0000000000a217c8 (err=-6)
>    ### ERROR ### Please RESET the board ###
> 
> Instead fail gracfully with ENODEV to allow board continue booting.
> 
>    dwc3_glue_bind_common: subnode name: usb@fcc00000
>    dwc3_glue_bind_common: unsupported dr_mode
> 
> Also use CONFIG_IS_ENABLED(USB_HOST) and change switch to if statements
> to improve readability of the code.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - Change to use CONFIG_IS_ENABLED for USB_HOST
> - Refactor switch to if statements (Marek Vasut)
> 
>   drivers/usb/dwc3/dwc3-generic.c | 25 +++++++------------------
>   1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 4d5d500aefab..6507219d6fc0 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -226,8 +226,7 @@ U_BOOT_DRIVER(dwc3_generic_peripheral) = {
>   };
>   #endif
>   
> -#if defined(CONFIG_SPL_USB_HOST) || \
> -	!defined(CONFIG_SPL_BUILD) && defined(CONFIG_USB_HOST)
> +#if CONFIG_IS_ENABLED(USB_HOST)
>   static int dwc3_generic_host_probe(struct udevice *dev)
>   {
>   	struct xhci_hcor *hcor;
> @@ -409,7 +408,7 @@ struct dwc3_glue_ops ti_ops = {
>   static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>   {
>   	const char *name = ofnode_get_name(node);
> -	const char *driver = NULL;
> +	const char *driver;
>   	enum usb_dr_mode dr_mode;
>   	struct udevice *dev;
>   	int ret;
> @@ -421,27 +420,17 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>   	if (!dr_mode)
>   		dr_mode = usb_get_dr_mode(node);
>   
> -	switch (dr_mode) {
> -	case USB_DR_MODE_PERIPHERAL:
> -	case USB_DR_MODE_OTG:
> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
> +	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__);
>   		driver = "dwc3-generic-peripheral";
> -#endif
> -		break;
> -#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
> -	case USB_DR_MODE_HOST:
> +	} else if (CONFIG_IS_ENABLED(USB_HOST) && dr_mode == USB_DR_MODE_HOST) {
>   		debug("%s: dr_mode: HOST\n", __func__);
>   		driver = "dwc3-generic-host";
> -		break;
> -#endif
> -	default:
> +	} else {
>   		debug("%s: unsupported dr_mode\n", __func__);

If you are already fixing this, please also print the actual dr_mode 
that is unsupported, i.e. the aforementioned debug() output is useless 
right now, just add dr_mode %s there.

Re: [PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Jonas Karlman]

On 2023-07-16 02:58, Marek Vasut wrote:
> On 7/16/23 01:10, Jonas Karlman wrote:
>> The current error check for device_find_first_child is not working as
>> expected, the documentation for device_find_first_child mention:
>>
>>    @devp: Returns first child device, or NULL if none
>>    Return: 0
>>
>> Change to return early when there is no child node to avoid any possible
>> null pointer dereference.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - Update commit message
>>
>>   drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>> index 35e4b36a695e..4d5d500aefab 100644
>> --- a/drivers/usb/dwc3/dwc3-generic.c
>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>> @@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
>>   			return ret;
>>   	}
>>   
>> -	ret = device_find_first_child(dev, &child);
>> -	if (ret)
>> -		return ret;
>> +	device_find_first_child(dev, &child);
>> +	if (!child)
>> +		return 0;
> 
> What does device_find_first_child() return in your case ? ENODEV or ENOSYS ?

device_find_first_child always returns 0, same as is documented.

See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/core/device.c#L931-941

Regards,
Jonas

Re: [PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Marek Vasut]

On 7/16/23 03:05, Jonas Karlman wrote:
> On 2023-07-16 02:58, Marek Vasut wrote:
>> On 7/16/23 01:10, Jonas Karlman wrote:
>>> The current error check for device_find_first_child is not working as
>>> expected, the documentation for device_find_first_child mention:
>>>
>>>     @devp: Returns first child device, or NULL if none
>>>     Return: 0
>>>
>>> Change to return early when there is no child node to avoid any possible
>>> null pointer dereference.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> v2:
>>> - Update commit message
>>>
>>>    drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>> index 35e4b36a695e..4d5d500aefab 100644
>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>> @@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
>>>    			return ret;
>>>    	}
>>>    
>>> -	ret = device_find_first_child(dev, &child);
>>> -	if (ret)
>>> -		return ret;
>>> +	device_find_first_child(dev, &child);
>>> +	if (!child)
>>> +		return 0;
>>
>> What does device_find_first_child() return in your case ? ENODEV or ENOSYS ?
> 
> device_find_first_child always returns 0, same as is documented.
> 
> See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/core/device.c#L931-941

Ah, I see, thanks for clarifying.

Reviewed-by: Marek Vasut <marex@denx.de>

Could you fix mtu3_plat.c the same way and switch this function to 
return void ? (separate series for these two things please)

Re: [PATCH v2 3/5] usb: dwc3-generic: Relax unsupported dr_mode check

[Jonas Karlman]

On 2023-07-16 02:59, Marek Vasut wrote:
> On 7/16/23 01:10, Jonas Karlman wrote:
>> When dr_mode is peripheral or otg and U-Boot has not been built with
>> DM_USB_GADGET support, booting such device may end up with:
>>
>>    dwc3_glue_bind_common: subnode name: usb@fcc00000
>>    Error binding driver 'dwc3-generic-wrapper': -6
>>    Some drivers failed to bind
>>    initcall sequence 00000000effbca08 failed at call 0000000000a217c8 (err=-6)
>>    ### ERROR ### Please RESET the board ###
>>
>> Instead fail gracfully with ENODEV to allow board continue booting.
>>
>>    dwc3_glue_bind_common: subnode name: usb@fcc00000
>>    dwc3_glue_bind_common: unsupported dr_mode
>>
>> Also use CONFIG_IS_ENABLED(USB_HOST) and change switch to if statements
>> to improve readability of the code.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - Change to use CONFIG_IS_ENABLED for USB_HOST
>> - Refactor switch to if statements (Marek Vasut)
>>
>>   drivers/usb/dwc3/dwc3-generic.c | 25 +++++++------------------
>>   1 file changed, 7 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>> index 4d5d500aefab..6507219d6fc0 100644
>> --- a/drivers/usb/dwc3/dwc3-generic.c
>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>> @@ -226,8 +226,7 @@ U_BOOT_DRIVER(dwc3_generic_peripheral) = {
>>   };
>>   #endif
>>   
>> -#if defined(CONFIG_SPL_USB_HOST) || \
>> -	!defined(CONFIG_SPL_BUILD) && defined(CONFIG_USB_HOST)
>> +#if CONFIG_IS_ENABLED(USB_HOST)
>>   static int dwc3_generic_host_probe(struct udevice *dev)
>>   {
>>   	struct xhci_hcor *hcor;
>> @@ -409,7 +408,7 @@ struct dwc3_glue_ops ti_ops = {
>>   static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>>   {
>>   	const char *name = ofnode_get_name(node);
>> -	const char *driver = NULL;
>> +	const char *driver;
>>   	enum usb_dr_mode dr_mode;
>>   	struct udevice *dev;
>>   	int ret;
>> @@ -421,27 +420,17 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>>   	if (!dr_mode)
>>   		dr_mode = usb_get_dr_mode(node);
>>   
>> -	switch (dr_mode) {
>> -	case USB_DR_MODE_PERIPHERAL:
>> -	case USB_DR_MODE_OTG:
>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>> +	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__);
>>   		driver = "dwc3-generic-peripheral";
>> -#endif
>> -		break;
>> -#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
>> -	case USB_DR_MODE_HOST:
>> +	} else if (CONFIG_IS_ENABLED(USB_HOST) && dr_mode == USB_DR_MODE_HOST) {
>>   		debug("%s: dr_mode: HOST\n", __func__);
>>   		driver = "dwc3-generic-host";
>> -		break;
>> -#endif
>> -	default:
>> +	} else {
>>   		debug("%s: unsupported dr_mode\n", __func__);
> 
> If you are already fixing this, please also print the actual dr_mode 
> that is unsupported, i.e. the aforementioned debug() output is useless 
> right now, just add dr_mode %s there.

Sure, I can add a dr_mode %d to the debug message in a v3, dr_mode is
enum not a string.

Regards,
Jonas

Re: [PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Jonas Karlman]

On 2023-07-16 03:19, Marek Vasut wrote:
> On 7/16/23 03:05, Jonas Karlman wrote:
>> On 2023-07-16 02:58, Marek Vasut wrote:
>>> On 7/16/23 01:10, Jonas Karlman wrote:
>>>> The current error check for device_find_first_child is not working as
>>>> expected, the documentation for device_find_first_child mention:
>>>>
>>>>     @devp: Returns first child device, or NULL if none
>>>>     Return: 0
>>>>
>>>> Change to return early when there is no child node to avoid any possible
>>>> null pointer dereference.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>> v2:
>>>> - Update commit message
>>>>
>>>>    drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>> index 35e4b36a695e..4d5d500aefab 100644
>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>> @@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
>>>>    			return ret;
>>>>    	}
>>>>    
>>>> -	ret = device_find_first_child(dev, &child);
>>>> -	if (ret)
>>>> -		return ret;
>>>> +	device_find_first_child(dev, &child);
>>>> +	if (!child)
>>>> +		return 0;
>>>
>>> What does device_find_first_child() return in your case ? ENODEV or ENOSYS ?
>>
>> device_find_first_child always returns 0, same as is documented.
>>
>> See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/core/device.c#L931-941>> 
> Ah, I see, thanks for clarifying.
> 
> Reviewed-by: Marek Vasut <marex@denx.de>
> 
> Could you fix mtu3_plat.c the same way and switch this function to 
> return void ? (separate series for these two things please)

Sorry, not sure when I would have time for that, currently way too much
ongoing in my pipeline :-)

Regards,
Jonas

Re: [PATCH v2 2/5] usb: dwc3-generic: Return early when there is no child node

[Marek Vasut]

On 7/16/23 09:57, Jonas Karlman wrote:
> On 2023-07-16 03:19, Marek Vasut wrote:
>> On 7/16/23 03:05, Jonas Karlman wrote:
>>> On 2023-07-16 02:58, Marek Vasut wrote:
>>>> On 7/16/23 01:10, Jonas Karlman wrote:
>>>>> The current error check for device_find_first_child is not working as
>>>>> expected, the documentation for device_find_first_child mention:
>>>>>
>>>>>      @devp: Returns first child device, or NULL if none
>>>>>      Return: 0
>>>>>
>>>>> Change to return early when there is no child node to avoid any possible
>>>>> null pointer dereference.
>>>>>
>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>> ---
>>>>> v2:
>>>>> - Update commit message
>>>>>
>>>>>     drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>>>>>     1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>> index 35e4b36a695e..4d5d500aefab 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>> @@ -558,9 +558,9 @@ int dwc3_glue_probe(struct udevice *dev)
>>>>>     			return ret;
>>>>>     	}
>>>>>     
>>>>> -	ret = device_find_first_child(dev, &child);
>>>>> -	if (ret)
>>>>> -		return ret;
>>>>> +	device_find_first_child(dev, &child);
>>>>> +	if (!child)
>>>>> +		return 0;
>>>>
>>>> What does device_find_first_child() return in your case ? ENODEV or ENOSYS ?
>>>
>>> device_find_first_child always returns 0, same as is documented.
>>>
>>> See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/core/device.c#L931-941>>
>> Ah, I see, thanks for clarifying.
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
>>
>> Could you fix mtu3_plat.c the same way and switch this function to
>> return void ? (separate series for these two things please)
> 
> Sorry, not sure when I would have time for that, currently way too much
> ongoing in my pipeline :-)

All right, I sent those two patches, it was trivial. But now I am out of 
time for reviewing today, so this series has to wait until the next 
round, likely next week.