kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw_random : platform_get_irq() already prints an error
@ 2020-10-18  5:49 Nigel Christian
  2020-10-18 10:53 ` Julia Lawall
  2020-10-27 11:22 ` Dan Carpenter
  0 siblings, 2 replies; 3+ messages in thread
From: Nigel Christian @ 2020-10-18  5:49 UTC (permalink / raw)
  To: kernel-janitors

coccicheck
drivers/char//hw_random/imx-rngc.c:256:2-9: line 256 is redundant because platform_get_irq() already prints an error

Signed-off-by: Nigel Christian <nigel.l.christian@gmail.com>
---
 drivers/char/hw_random/imx-rngc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
index 61c844baf26e..69f13ff1bbec 100644
--- a/drivers/char/hw_random/imx-rngc.c
+++ b/drivers/char/hw_random/imx-rngc.c
@@ -253,7 +253,6 @@ static int imx_rngc_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq <= 0) {
-		dev_err(&pdev->dev, "Couldn't get irq %d\n", irq);
 		return irq;
 	}
 
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] hw_random : platform_get_irq() already prints an error
  2020-10-18  5:49 [PATCH] hw_random : platform_get_irq() already prints an error Nigel Christian
@ 2020-10-18 10:53 ` Julia Lawall
  2020-10-27 11:22 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Julia Lawall @ 2020-10-18 10:53 UTC (permalink / raw)
  To: kernel-janitors

This should also go to the maintainers of the file, which you can find
using the script scripts/get_maintainer.pl.

hw_random is not the right way to start the subject line for this file.
Use git log --oneline to see what others have done.

On Sun, 18 Oct 2020, Nigel Christian wrote:

> coccicheck
> drivers/char//hw_random/imx-rngc.c:256:2-9: line 256 is redundant because platform_get_irq() already prints an error

The log message should be limited to at most 75 columns, because when it
ends up in git it will be slightly indented.

The log message should explain what you are doing and why, not just repeat
the output of a tool.  In particular, the file and line number should not
be included in the log message, because that information is already
available in the changes below.

julia

> Signed-off-by: Nigel Christian <nigel.l.christian@gmail.com>
> ---
>  drivers/char/hw_random/imx-rngc.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
> index 61c844baf26e..69f13ff1bbec 100644
> --- a/drivers/char/hw_random/imx-rngc.c
> +++ b/drivers/char/hw_random/imx-rngc.c
> @@ -253,7 +253,6 @@ static int imx_rngc_probe(struct platform_device *pdev)
>
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq <= 0) {
> -		dev_err(&pdev->dev, "Couldn't get irq %d\n", irq);
>  		return irq;
>  	}
>
> --
> 2.28.0
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] hw_random : platform_get_irq() already prints an error
  2020-10-18  5:49 [PATCH] hw_random : platform_get_irq() already prints an error Nigel Christian
  2020-10-18 10:53 ` Julia Lawall
@ 2020-10-27 11:22 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-10-27 11:22 UTC (permalink / raw)
  To: kernel-janitors

On Sun, Oct 18, 2020 at 01:49:12AM -0400, Nigel Christian wrote:
> coccicheck
> drivers/char//hw_random/imx-rngc.c:256:2-9: line 256 is redundant because platform_get_irq() already prints an error
> 
> Signed-off-by: Nigel Christian <nigel.l.christian@gmail.com>
> ---
>  drivers/char/hw_random/imx-rngc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
> index 61c844baf26e..69f13ff1bbec 100644
> --- a/drivers/char/hw_random/imx-rngc.c
> +++ b/drivers/char/hw_random/imx-rngc.c
> @@ -253,7 +253,6 @@ static int imx_rngc_probe(struct platform_device *pdev)
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq <= 0) {
            ^^^^^^^^
This check is wrong.  It should be:

	if (irq < 0) {

The platform_get_irq() function will never return zero.  But say it
did return a zero then that means that "return irq;" is returning
success which would be a bug.

> -		dev_err(&pdev->dev, "Couldn't get irq %d\n", irq);
>  		return irq;
>  	}

The patch introduces a new checkpatch warning because now there is only
one statement in the if statment block.  (Patches shouldn't introduce
checkpatch warnings).  It should be:

	irq = platform_get_irq(pdev, 0);
	if (irq < 0)
		return irq;

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-27 11:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-18  5:49 [PATCH] hw_random : platform_get_irq() already prints an error Nigel Christian
2020-10-18 10:53 ` Julia Lawall
2020-10-27 11:22 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).