On Thu, 8 Jun 2023 13:56:31 -0600 Sam Edwards <cfswo...@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 <cfswo...@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)