| Message ID | 20200324132029.16296-1-tangbin@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] usb: gadget: bcm63xx_udc:remove redundant variable assignment | expand |
Hello! On 03/24/2020 04:20 PM, Tang Bin wrote: > --v1------------------------------------ > In this function, the variable 'rc' is assigned after this place, > so the definition is invalid. > > --v2------------------------------------ > In this function, the variable 'rc' will be assigned by the function > 'usb_add_gadget_udc()',so the assignment here is redundant,we should > remove it. > > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> NAK. > --- > drivers/usb/gadget/udc/bcm63xx_udc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c > index 54501814d..a7afa8c35 100644 > --- a/drivers/usb/gadget/udc/bcm63xx_udc.c > +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c > @@ -2321,8 +2321,6 @@ static int bcm63xx_udc_probe(struct platform_device *pdev) > if (rc) > return rc; > > - rc = -ENXIO; > - > /* IRQ resource #0: control interrupt (VBUS, speed, etc.) */ > irq = platform_get_irq(pdev, 0); > if (irq < 0) This *if* branch goes to the 'out_uninit' label which uses 'rc' (and it should be negative). In principle, if you change 'rc' to 'irq' below, this patch would be sane. It's not as is. MBR, Sergei
On 03/24/2020 05:50 PM, Sergei Shtylyov wrote: >> --v1------------------------------------ >> In this function, the variable 'rc' is assigned after this place, >> so the definition is invalid. >> >> --v2------------------------------------ >> In this function, the variable 'rc' will be assigned by the function >> 'usb_add_gadget_udc()',so the assignment here is redundant,we should >> remove it. >> >> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> > > NAK. > >> --- >> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c >> index 54501814d..a7afa8c35 100644 >> --- a/drivers/usb/gadget/udc/bcm63xx_udc.c >> +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c >> @@ -2321,8 +2321,6 @@ static int bcm63xx_udc_probe(struct platform_device *pdev) >> if (rc) >> return rc; >> >> - rc = -ENXIO; >> - >> /* IRQ resource #0: control interrupt (VBUS, speed, etc.) */ >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) > > This *if* branch goes to the 'out_uninit' label which uses 'rc' (and it should > be negative). > In principle, if you change 'rc' to 'irq' below, this patch would be sane. > It's not as is. Still, the other *goto* out_uninit in te loop below shoild be changed as well. Otherwise, if the result is overriden to -ENXIO, e.g. deferred probing is borked. MBR, Sergei
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c index 54501814d..a7afa8c35 100644 --- a/drivers/usb/gadget/udc/bcm63xx_udc.c +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c @@ -2321,8 +2321,6 @@ static int bcm63xx_udc_probe(struct platform_device *pdev) if (rc) return rc; - rc = -ENXIO; - /* IRQ resource #0: control interrupt (VBUS, speed, etc.) */ irq = platform_get_irq(pdev, 0); if (irq < 0)
--v1------------------------------------ In this function, the variable 'rc' is assigned after this place, so the definition is invalid. --v2------------------------------------ In this function, the variable 'rc' will be assigned by the function 'usb_add_gadget_udc()',so the assignment here is redundant,we should remove it. Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> --- drivers/usb/gadget/udc/bcm63xx_udc.c | 2 -- 1 file changed, 2 deletions(-)