[PATCH v2 2/2] usb: musb-new: sunxi: make compatible with UDC/DM gadget model

[Andre Przywara]

On Thu,  8 Jun 2023 13:56:31 -0600
Sam Edwards <cfsworks at gmail.com> wrote:

Hi,

John asked me have a look at this.

> Since many sunxi boards do not implement a `board_usb_init`, it's

I am confused, what has this have to do with gadget support? *No* sunxi
board build provides board_usb_init(), but apparently this works fine for
now.
I am all for this converting to DM part, but the rationale seems a bit
off.

Also can you give some reason for this patch? What does this fix or
improve? "it's better" is a bit thin, "complying with DM" would already be
sufficient, but maybe there is more?

> better if we just make the sunxi USB driver compatible with the
> DM gadget model, as many other musb-new variants already are.
> 
> This change has been verified working on a T113s.
> 
> Signed-off-by: Sam Edwards <CFSworks at gmail.com>
> ---
>  drivers/usb/musb-new/sunxi.c | 50 +++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
> index 510b254f7d..6658cd995d 100644
> --- a/drivers/usb/musb-new/sunxi.c
> +++ b/drivers/usb/musb-new/sunxi.c
> @@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = {
>  	.ram_bits	= SUNXI_MUSB_RAM_BITS,
>  };
>  
> +#if CONFIG_IS_ENABLED(DM_USB_GADGET)

Please no more #ifdef's. Is there any reason to *not* force
DM_USB_GADGET now, for all sunxi boards, in Kconfig?
Either by "select"ing it in the USB Kconfig, or in arch/arm/Kconfig, like
other platforms do.
Then you don't need to care about the !DM_USB_GADGET definition of this
function and can drop the #ifdef.

> +int dm_usb_gadget_handle_interrupts(struct udevice *dev) {

coding style

> +	struct sunxi_glue *glue = dev_get_priv(dev);
> +	struct musb_host_data *host = &glue->mdata;
> +
> +	host->host->isr(0, host->host);
> +	return 0;
> +}
> +#endif
> +
>  static int musb_usb_probe(struct udevice *dev)
>  {
>  	struct sunxi_glue *glue = dev_get_priv(dev);
> @@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev)
>  	void *base = dev_read_addr_ptr(dev);
>  	int ret;
>  
> -#ifdef CONFIG_USB_MUSB_HOST
> -	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
> -#endif
> -
>  	if (!base)
>  		return -EINVAL;
>  
> @@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev)
>  	pdata.platform_ops = &sunxi_musb_ops;
>  	pdata.config = glue->cfg->config;
>  
> -#ifdef CONFIG_USB_MUSB_HOST
> -	priv->desc_before_addr = true;
> +	if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) {
> +		struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
> +		priv->desc_before_addr = true;
>  
> -	pdata.mode = MUSB_HOST;
> -	host->host = musb_init_controller(&pdata, &glue->dev, base);
> -	if (!host->host)
> -		return -EIO;
> +		pdata.mode = MUSB_HOST;
> +		host->host = musb_init_controller(&pdata, &glue->dev, base);
> +		if (!host->host)
> +			return -EIO;
>  
> -	return musb_lowlevel_init(host);
> -#else
> -	pdata.mode = MUSB_PERIPHERAL;
> -	host->host = musb_register(&pdata, &glue->dev, base);
> -	if (IS_ERR_OR_NULL(host->host))
> -		return -EIO;
> +		return musb_lowlevel_init(host);
> +	} else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) {
> +		pdata.mode = MUSB_PERIPHERAL;
> +		host->host = musb_init_controller(&pdata, &glue->dev, base);
> +		if (!host->host)
> +			return -EIO;
>  
> -	return 0;
> -#endif
> +		return usb_add_gadget_udc(&glue->dev, &host->host->g);
> +	} else {
> +		pdata.mode = MUSB_PERIPHERAL;
> +		host->host = musb_register(&pdata, &glue->dev, base);
> +		if (IS_ERR_OR_NULL(host->host))
> +			return -EIO;
> +
> +		return 0;
> +	}

That looks like a good cleanup! Just need to test it briefly, but it seems
like the gist of this patch is fine.

Cheers,
Andre


>  }
>  
>  static int musb_usb_remove(struct udevice *dev)