Re: [PATCH v2] usb: dwc3: host: remove dead code in dwc3_host_get_irq()

[Oliver Neukum]

On 23.03.23 12:13, Dan Carpenter wrote:

> > > > v1->v2: remove redundant goto
> > > >    drivers/usb/dwc3/host.c | 4 ----
> > > >    1 file changed, 4 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> > > > index f6f13e7f1ba1..ca1e8294e835 100644
> > > > --- a/drivers/usb/dwc3/host.c
> > > > +++ b/drivers/usb/dwc3/host.c
> > > > @@ -54,12 +54,8 @@ static int dwc3_host_get_irq(struct dwc3 *dwc)
> > > >    	irq = platform_get_irq(dwc3_pdev, 0);
> > > >    	if (irq > 0) {
> > > >    		dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
> > > > -		goto out;
> > > >    	}
> > >
> > >      Now drop {} please. :-)
> >
> > Well, no, please drop the whole patch.
> > If platform_get_irq() returns -EPROBE_DEFER you now give that
> > as a return value.
> >
> > This tiny bit of optimization is not worth changing semantics.
>
> The v2 patch doesn't change the semantics.  Mine did though...

Now I may be dense, but let's look at the current code:

        irq = platform_get_irq(dwc3_pdev, 0);

assuming irq = -EPROBE_DEFER

        if (irq > 0) {

not taken
                dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
                goto out;
        }

        if (!irq)

irq != 0
                irq = -EINVAL;

out:
        return irq;

returning -EINVAL

Patched version:

        irq = platform_get_irq(dwc3_pdev, 0);

assuming irq = -EPROBE_DEFER

        if (irq > 0) {
                dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
        }

out:
        return irq;

returning -EPROBE_DEFER

Your version:

 	irq = platform_get_irq(dwc3_pdev, 0);

assuming irq = -EPROBE_DEFER

 	if (irq > 0) {

not taken
 		dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
+		return irq;
 	}
 
+	return -ENODEV;

Yet another error return.
Now, I admit I am by now sufficiently confused to know which version is
correct, but they are all three different in what they return.

	Regards
		Oliver