RE: [PATCH] usb: gadget: udc: renesas_usb3: Fix RZ/V2M {modprobe,bind} error

From: Biju Das
Date: Mon May 29 2023 - 04:43:21 EST


Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH] usb: gadget: udc: renesas_usb3: Fix RZ/V2M
> {modprobe,bind} error
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Fri, May 26, 2023 at 03:36:15PM +0100, Biju Das wrote:
> > Currently {modprobe, bind} after {rmmod, unbind} results in probe
> failure.
> >
> > genirq: Flags mismatch irq 22. 00000004 (85070400.usb3drd) vs.
> > 00000004 (85070400.usb3drd)
> > renesas_usb3: probe of 85070000.usb3peri failed with error -16
> >
> > Fix this issue by replacing "parent dev"->"dev" as the irq resource is
> > managed by this driver.
>
> If the dev pointer passed to devm_request_irq() is not the correct one,
> how does it work the first time the driver is loaded ?

+ Marc/ Kernel.org to give some feedback on this issue

I believe there may be a bug in the genirq (kernel/irq) driver.
first time it works ok. Maybe this driver is caching on unload
with null value and comparing with actual one (irq 22) during reload??

Maybe genirq expert can comment what went wrong here??

>
> > Fixes: 9cad72dfc556 ("usb: gadget: Add support for RZ/V2M USB3DRD
> driver"
>
> There's a missing ')' at the end of the line.

Oops missed it.

>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/gadget/udc/renesas_usb3.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index aac8bc185afa..4a37b2e4b9b3 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -2877,7 +2877,7 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> > struct rzv2m_usb3drd *ddata = dev_get_drvdata(pdev-
> >dev.parent);
> >
> > usb3->drd_reg = ddata->reg;
> > - ret = devm_request_irq(ddata->dev, ddata->drd_irq,
> > + ret = devm_request_irq(&pdev->dev, ddata->drd_irq,
> > renesas_usb3_otg_irq, 0,
> > dev_name(ddata->dev), usb3);
>
> Shouldn't you use dev_name(&pdev->dev) too ?

This irq resource belongs to usb3drd driver and is managed by renesas_usb3 driver.
It is just representation of irqname and cat /proc/interrupts shows the correct
irq resource name. with dev_name(ddata->dev), it displays correct resource name
associated with the handler.

root@rzv2m:~# cat /proc/interrupts | grep usb
22: 0 GICv2 274 Level 85070400.usb3drd
23: 353 GICv2 277 Level xhci-hcd:usb1
28: 0 GICv2 278 Level 85070000.usb3peri

Cheers,
Biju

>
> > if (ret < 0)
>
> --
> Regards,
>
> Laurent Pinchart