| Message ID | 20220926135922.24541-1-dzm91@hust.edu.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 29afbe5f5afc2f724b8aef2d11fbe6a7ee48997e |
| Headers | show |
| Series | usb: cdns3: remove dead code | expand |
> >From: Dongliang Mu <mudongliangabcd@gmail.com> > >Smatch reports the following error: > >drivers/usb/cdns3/cdns3-plat.c:113 cdns3_plat_probe() warn: >platform_get_irq() does not return zero > >From the document, platform_get_irq_byname_optional only returns >non-zero value, and negative value on failure. > >Fix this by removing the zero value checking. > >Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> >--- > drivers/usb/cdns3/cdns3-plat.c | 2 -- > 1 file changed, 2 deletions(-) > >diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c >index dc068e940ed5..2bc5d094548b 100644 >--- a/drivers/usb/cdns3/cdns3-plat.c >+++ b/drivers/usb/cdns3/cdns3-plat.c >@@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) > cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); > if (cdns->wakeup_irq == -EPROBE_DEFER) > return cdns->wakeup_irq; >- else if (cdns->wakeup_irq == 0) >- return -EINVAL; > I think that here we should have: else if (cdns->wakeup_irq == -ENXIO) return -EINVAL; because of function: platform_get_irq_byname_optional -> __platform_get_irq_byname returns irq number (>0), -EPROBE_DEFFER or -ENXIO thanks Pawel > if (cdns->wakeup_irq < 0) { > dev_dbg(dev, "couldn't get wakeup irq\n"); >-- >2.35.1
Hello Pawel, On 28/09/2022 09:40, Pawel Laszczak wrote: >> >> From: Dongliang Mu <mudongliangabcd@gmail.com> >> >> Smatch reports the following error: >> >> drivers/usb/cdns3/cdns3-plat.c:113 cdns3_plat_probe() warn: >> platform_get_irq() does not return zero >> >>From the document, platform_get_irq_byname_optional only returns >> non-zero value, and negative value on failure. >> >> Fix this by removing the zero value checking. >> >> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> >> --- >> drivers/usb/cdns3/cdns3-plat.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c >> index dc068e940ed5..2bc5d094548b 100644 >> --- a/drivers/usb/cdns3/cdns3-plat.c >> +++ b/drivers/usb/cdns3/cdns3-plat.c >> @@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) >> cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); >> if (cdns->wakeup_irq == -EPROBE_DEFER) >> return cdns->wakeup_irq; >> - else if (cdns->wakeup_irq == 0) >> - return -EINVAL; >> > I think that here we should have: > else if (cdns->wakeup_irq == -ENXIO) > return -EINVAL; > because of function: > platform_get_irq_byname_optional -> __platform_get_irq_byname returns > irq number (>0), -EPROBE_DEFFER or -ENXIO But this is changing functionality and should come as a new patch. The original patch is correct as it doesn't change existing code functionality. > > > thanks > Pawel > >> if (cdns->wakeup_irq < 0) { >> dev_dbg(dev, "couldn't get wakeup irq\n"); >> -- >> 2.35.1 > cheers, -roger
On 26/09/2022 16:59, Dongliang Mu wrote: > From: Dongliang Mu <mudongliangabcd@gmail.com> > > Smatch reports the following error: > > drivers/usb/cdns3/cdns3-plat.c:113 cdns3_plat_probe() warn: > platform_get_irq() does not return zero > > From the document, platform_get_irq_byname_optional only returns > non-zero value, and negative value on failure. > > Fix this by removing the zero value checking. > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> Reviewed-by: Roger Quadros <rogerq@kernel.org> > --- > drivers/usb/cdns3/cdns3-plat.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c > index dc068e940ed5..2bc5d094548b 100644 > --- a/drivers/usb/cdns3/cdns3-plat.c > +++ b/drivers/usb/cdns3/cdns3-plat.c > @@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) > cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); > if (cdns->wakeup_irq == -EPROBE_DEFER) > return cdns->wakeup_irq; > - else if (cdns->wakeup_irq == 0) > - return -EINVAL; > > if (cdns->wakeup_irq < 0) { > dev_dbg(dev, "couldn't get wakeup irq\n"); cheers, -roger
On Wed, Sep 28, 2022 at 2:49 PM Roger Quadros <rogerq@kernel.org> wrote: > > Hello Pawel, > > On 28/09/2022 09:40, Pawel Laszczak wrote: > >> > >> From: Dongliang Mu <mudongliangabcd@gmail.com> > >> > >> Smatch reports the following error: > >> > >> drivers/usb/cdns3/cdns3-plat.c:113 cdns3_plat_probe() warn: > >> platform_get_irq() does not return zero > >> > >>From the document, platform_get_irq_byname_optional only returns > >> non-zero value, and negative value on failure. > >> > >> Fix this by removing the zero value checking. > >> > >> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > >> --- > >> drivers/usb/cdns3/cdns3-plat.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c > >> index dc068e940ed5..2bc5d094548b 100644 > >> --- a/drivers/usb/cdns3/cdns3-plat.c > >> +++ b/drivers/usb/cdns3/cdns3-plat.c > >> @@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) > >> cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); > >> if (cdns->wakeup_irq == -EPROBE_DEFER) > >> return cdns->wakeup_irq; > >> - else if (cdns->wakeup_irq == 0) > >> - return -EINVAL; > >> > > I think that here we should have: > > else if (cdns->wakeup_irq == -ENXIO) > > return -EINVAL; > > because of function: > > platform_get_irq_byname_optional -> __platform_get_irq_byname returns > > irq number (>0), -EPROBE_DEFFER or -ENXIO > > But this is changing functionality and should come as a new patch. I agree. Pawel, you should submit a new patch. This satisfies the rule of kernel patching. > > The original patch is correct as it doesn't change existing code > functionality. > > > > > > > thanks > > Pawel > > > >> if (cdns->wakeup_irq < 0) { > >> dev_dbg(dev, "couldn't get wakeup irq\n"); > >> -- > >> 2.35.1 > > > > cheers, > -roger
On Wed, Sep 28, 2022 at 2:50 PM Roger Quadros <rogerq@kernel.org> wrote: > > > > On 26/09/2022 16:59, Dongliang Mu wrote: > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > Smatch reports the following error: > > > > drivers/usb/cdns3/cdns3-plat.c:113 cdns3_plat_probe() warn: > > platform_get_irq() does not return zero > > > > From the document, platform_get_irq_byname_optional only returns > > non-zero value, and negative value on failure. > > > > Fix this by removing the zero value checking. > > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > Reviewed-by: Roger Quadros <rogerq@kernel.org> Hi Roger, By simply checking the usage of API - platform_get_irq_byname_optional, there are several issues in other code sites. However, some code sites are related to semantics. I will analyze all of them and submit patches later. > > > --- > > drivers/usb/cdns3/cdns3-plat.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c > > index dc068e940ed5..2bc5d094548b 100644 > > --- a/drivers/usb/cdns3/cdns3-plat.c > > +++ b/drivers/usb/cdns3/cdns3-plat.c > > @@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) > > cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); > > if (cdns->wakeup_irq == -EPROBE_DEFER) > > return cdns->wakeup_irq; > > - else if (cdns->wakeup_irq == 0) > > - return -EINVAL; > > > > if (cdns->wakeup_irq < 0) { > > dev_dbg(dev, "couldn't get wakeup irq\n"); > > > cheers, > -roger
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c index dc068e940ed5..2bc5d094548b 100644 --- a/drivers/usb/cdns3/cdns3-plat.c +++ b/drivers/usb/cdns3/cdns3-plat.c @@ -110,8 +110,6 @@ static int cdns3_plat_probe(struct platform_device *pdev) cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup"); if (cdns->wakeup_irq == -EPROBE_DEFER) return cdns->wakeup_irq; - else if (cdns->wakeup_irq == 0) - return -EINVAL; if (cdns->wakeup_irq < 0) { dev_dbg(dev, "couldn't get wakeup irq\n");