Re: [PATCH 1/2] usb: dwc3-generic: set "mode" based on caller of dwc3_generic_probe()

[Roger Quadros <rogerq@kernel.org>]

On 26/11/2024 14:03, Siddharth Vadapalli wrote:
> There are only two callers of "dwc3_generic_probe()", namely:
> 1. dwc3_generic_peripheral_probe()
> 2. dwc3_generic_host_probe()
> Currently, the "mode" is set based on the device-tree node of the
> platform device. Also, the DWC3 core doesn't support updating the "mode"
> dynamically at runtime if it is set to "OTG", i.e. "OTG" is treated as a
> separate mode in itself, rather than being treated as a mode which should
> eventually lead to "host"/"peripheral".
> 
> Given that the callers of "dwc3_generic_probe()" clarify the expected
> "mode" of the USB Controller, use that "mode" instead of the one
> specified in the device-tree. This shall allow the USB Controller to
> function both as a "Host" and as a "Peripheral" when the "mode" is "otg"
> in the device-tree, based on the caller of "dwc3_generic_probe()".
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  drivers/usb/dwc3/dwc3-generic.c | 9 +++++----

TI AM62a is not even using this driver so how did you test this?
Did I miss a DT patch?

>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 2ab41cbae45..55e62b35c61 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -51,7 +51,8 @@ struct dwc3_generic_host_priv {
>  };
>  
>  static int dwc3_generic_probe(struct udevice *dev,
> -			      struct dwc3_generic_priv *priv)
> +			      struct dwc3_generic_priv *priv,
> +			      enum usb_dr_mode mode)
>  {
>  	int rc;
>  	struct dwc3_generic_plat *plat = dev_get_plat(dev);
> @@ -62,7 +63,7 @@ static int dwc3_generic_probe(struct udevice *dev,
>  
>  	dwc3->dev = dev;
>  	dwc3->maximum_speed = plat->maximum_speed;
> -	dwc3->dr_mode = plat->dr_mode;
> +	dwc3->dr_mode = mode;

if platform supported mode was "host" only or "peripheral" only
then you shouldn't we prevent from using the other role?

In u-boot shell, if user starts Host driver first and then starts peripheral driver
how does it work?
Did you test this case?

>  #if CONFIG_IS_ENABLED(OF_CONTROL)
>  	dwc3_of_parse(dwc3);
>  
> @@ -197,7 +198,7 @@ static int dwc3_generic_peripheral_probe(struct udevice *dev)
>  {
>  	struct dwc3_generic_priv *priv = dev_get_priv(dev);
>  
> -	return dwc3_generic_probe(dev, priv);
> +	return dwc3_generic_probe(dev, priv, USB_DR_MODE_PERIPHERAL);
>  }
>  
>  static int dwc3_generic_peripheral_remove(struct udevice *dev)
> @@ -241,7 +242,7 @@ static int dwc3_generic_host_probe(struct udevice *dev)
>  	struct dwc3_generic_host_priv *priv = dev_get_priv(dev);
>  	int rc;
>  
> -	rc = dwc3_generic_probe(dev, &priv->gen_priv);
> +	rc = dwc3_generic_probe(dev, &priv->gen_priv, USB_DR_MODE_HOST);
>  	if (rc)
>  		return rc;
>  

-- 
cheers,
-roger