| Message ID | 20210112023619.5713-1-dong.menglong@zte.com.cn |
|---|---|
| State | New |
| Headers | show |
| Series | ata: remove redundant error print in rb532_pata_driver_probe | expand |
Hello! On 1/12/21 5:36 AM, menglong8.dong@gmail.com wrote: > From: Menglong Dong <dong.menglong@zte.com.cn> > > Coccinelle reports a redundant error print in rb532_pata_driver_probe. > As 'platform_get_irq' already prints the error message, error print > here is redundant and can be removed. > > Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn> > --- > drivers/ata/pata_rb532_cf.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c > index 479c4b29b856..dcde84f571c4 100644 > --- a/drivers/ata/pata_rb532_cf.c > +++ b/drivers/ata/pata_rb532_cf.c > @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct platform_device *pdev) > } > > irq = platform_get_irq(pdev, 0); > - if (irq <= 0) { > - dev_err(&pdev->dev, "no IRQ resource found\n"); > + if (irq <= 0) > return -ENOENT; This still beaks the probe deferral. :-( But that's another problem... [...] MBR, Sergei
Hello, Sergei On Tue, Jan 12, 2021 at 7:15 PM Sergei Shtylyov <sergei.shtylyov@gmail.com> wrote: > > Hello! > > On 1/12/21 5:36 AM, menglong8.dong@gmail.com wrote: > [....] > > irq = platform_get_irq(pdev, 0); > > - if (irq <= 0) { > > - dev_err(&pdev->dev, "no IRQ resource found\n"); > > + if (irq <= 0) > > return -ENOENT; > > This still beaks the probe deferral. :-( > But that's another problem... > > [...] > > MBR, Sergei What does this 'MBR' mean? I am a novice~~~ So, is it better to replace 'platform_get_irq' with 'platform_get_irq_optional' here? -- Best Regards Menglong Dong
Hello! On 13.01.2021 17:04, Menglong Dong wrote: [...] >>> irq = platform_get_irq(pdev, 0); >>> - if (irq <= 0) { >>> - dev_err(&pdev->dev, "no IRQ resource found\n"); >>> + if (irq <= 0) >>> return -ENOENT; >> >> This still beaks the probe deferral. :-( >> But that's another problem... >> >> [...] >> >> MBR, Sergei > > What does this 'MBR' mean? I am a novice~~~ Generally speaking, Master Boot Record. But I also use it to send you My Best Regards. :-) > So, is it better to replace 'platform_get_irq' with > 'platform_get_irq_optional' here? No. You should stop overriding the result to -ENOENT and pass the result up the call chain instead. In order to do it, you should only check for (irq < 0). > -- > Best Regards > Menglong Dong MBR, Sergei
On Thu, Jan 14, 2021 at 4:30 PM Sergei Shtylyov <sergei.shtylyov@gmail.com> wrote: [...] > > > > What does this 'MBR' mean? I am a novice~~~ > > Generally speaking, Master Boot Record. But I also use it to send you My > Best Regards. :-) Haha~, > > So, is it better to replace 'platform_get_irq' with > > 'platform_get_irq_optional' here? > > No. You should stop overriding the result to -ENOENT and pass the result > up the call chain instead. In order to do it, you should only check for (irq < 0). Well, I didn't even notice this. It does seem to be another problem... --- Best Regards Menglong Dong
On 1/12/21 2:15 PM, Sergei Shtylyov wrote: [...] >> From: Menglong Dong <dong.menglong@zte.com.cn> >> >> Coccinelle reports a redundant error print in rb532_pata_driver_probe. >> As 'platform_get_irq' already prints the error message, error print >> here is redundant and can be removed. >> >> Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn> >> --- >> drivers/ata/pata_rb532_cf.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c >> index 479c4b29b856..dcde84f571c4 100644 >> --- a/drivers/ata/pata_rb532_cf.c >> +++ b/drivers/ata/pata_rb532_cf.c >> @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct platform_device *pdev) >> } >> >> irq = platform_get_irq(pdev, 0); >> - if (irq <= 0) { >> - dev_err(&pdev->dev, "no IRQ resource found\n"); >> + if (irq <= 0) >> return -ENOENT; > > This still beaks the probe deferral. :-( > But that's another problem... BTW, your patch summary looks quite wrong -- we need the prefix to specify the patch locus, like this: pata_rb532_cf: remove redundant error printing in the probe() method I.e. "ata: " may even be omitted (your choise though). > [...] MBR, Sergei
diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c index 479c4b29b856..dcde84f571c4 100644 --- a/drivers/ata/pata_rb532_cf.c +++ b/drivers/ata/pata_rb532_cf.c @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct platform_device *pdev) } irq = platform_get_irq(pdev, 0); - if (irq <= 0) { - dev_err(&pdev->dev, "no IRQ resource found\n"); + if (irq <= 0) return -ENOENT; - } gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_IN); if (IS_ERR(gpiod)) {