Hi Mattijs,
On Wed, Jul 24, 2024 at 12:12 PM Mattijs Korpershoek
<mkorpersh...@baylibre.com> wrote:
>
> Checkpatch complains here:
>
> checkpatch.pl: 164: WARNING: From:/Signed-off-by: email address mismatch:
> 'From: Zixun LI <ad...@hifiphile.com>' != 'Signed-off-by: Zixun LI
> <z...@ogga.fr>'
>
> Please make sure that the commiter matches the Signed-off-by.
> This can be done with git commit --reset-author or --author="name <email>"
>
Will fix all Signed-off-by in v4.
>
> Here we change both:
> 1. pr_err() to log_err()
> 2. fix paramter -> parameter
>
> 2. should be done in patch 3/7 instead of here.
>
> With the above fixed:
>
Will fix in v4.
> Agreed, 'function' is more appropriate wording.
> Zixun, if you need to respin a v4 please consider using 'function'.
> Otherwise I can fix it up while applying !
Will fix in v4.
> > +static int usba_udc_probe(struct udevice *dev)
> > +{
> > + struct usba_priv_data *priv = dev_get_priv(dev);
> > + struct usba_udc *udc = &priv->udc;
> > + int ret;
> > +
> > + ret = usba_udc_clk_init(dev, &priv->clks);
> > + if (ret)
> > + return ret;
> > +
> > + udc->fifo = (void __iomem *)dev_remap_addr_index(dev, FIFO_IOMEM_ID);
> > + if (!udc->fifo)
>
> I think that at this points &priv->clks are valid and enabled.
> Therefore, we should disable/release them (as being done in the remove)
>
> > + return -EINVAL;
> > +
> > + udc->regs = (void __iomem *)dev_remap_addr_index(dev, CTRL_IOMEM_ID);
> > + if (!udc->regs)
>
> Same here
Good catch, I copied the dwc2 driver who has the same issue, will fix in v4.
Also spotted udc->usba_ep allocated in usba_udc_pdata() is not freed.
I'll change probe() and remove() functions as follow:
static int usba_udc_probe(struct udevice *dev)
{
struct usba_priv_data *priv = dev_get_priv(dev);
struct usba_udc *udc = &priv->udc;
int ret;
udc->fifo = (void __iomem *)dev_remap_addr_index(dev, FIFO_IOMEM_ID);
if (!udc->fifo)
return -EINVAL;
udc->regs = (void __iomem *)dev_remap_addr_index(dev, CTRL_IOMEM_ID);
if (!udc->regs)
return -EINVAL;
ret = usba_udc_clk_init(dev, &priv->clks);
if (ret)
return ret;
udc->usba_ep = usba_udc_pdata(&pdata, udc);
udc->gadget.ops = &usba_udc_ops;
udc->gadget.speed = USB_SPEED_HIGH,
udc->gadget.is_dualspeed = 1,
udc->gadget.name = "atmel_usba_udc",
ret = usb_add_gadget_udc((struct device *)dev, &udc->gadget);
if (ret)
goto err;
return 0;
err:
free(priv->udc.usba_ep);
clk_release_bulk(&priv->clks);
return ret;
}
static int usba_udc_remove(struct udevice *dev)
{
struct usba_priv_data *priv = dev_get_priv(dev);
usb_del_gadget_udc(&priv->udc.gadget);
free(priv->udc.usba_ep);
clk_release_bulk(&priv->clks);
return dm_scan_fdt_dev(dev);
}
