diff mbox series

tty: serial: add missing clk_put()

Message ID 047273ae4e4c25eb7b81fd69d761161e@208suo.com
State New
Headers show
Series tty: serial: add missing clk_put() | expand

Commit Message

sunran001@208suo.com July 18, 2023, 7:55 a.m. UTC
This patch fixes the following Coccinelle error:

./drivers/tty/serial/bcm63xx_uart.c:854:2-8: ERROR: missing clk_put;
clk_get on line 849 and execution via conditional on line 853

Signed-off-by: Ran Sun <sunran001@208suo.com>
---
  drivers/tty/serial/bcm63xx_uart.c | 1 +
  1 file changed, 1 insertion(+)

      port->iotype = UPIO_MEM;

Comments

Dmitry Baryshkov July 18, 2023, 8:40 a.m. UTC | #1
On 18/07/2023 10:55, sunran001@208suo.com wrote:
> This patch fixes the following Coccinelle error:
> 
> ./drivers/tty/serial/bcm63xx_uart.c:854:2-8: ERROR: missing clk_put;
> clk_get on line 849 and execution via conditional on line 853
> 
> Signed-off-by: Ran Sun <sunran001@208suo.com>
> ---
>   drivers/tty/serial/bcm63xx_uart.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/bcm63xx_uart.c 
> b/drivers/tty/serial/bcm63xx_uart.c
> index 55e82d0bf92d..7353b683952d 100644
> --- a/drivers/tty/serial/bcm63xx_uart.c
> +++ b/drivers/tty/serial/bcm63xx_uart.c
> @@ -851,6 +851,7 @@ static int bcm_uart_probe(struct platform_device *pdev)
>           clk = of_clk_get(pdev->dev.of_node, 0);
> 
>       if (IS_ERR(clk))
> +        clk_put(clk);

No!

First, calling clk_put() on the error pointer is incorrect. This will 
throw a warning in __clk_put(), but generally passing an error pointer 
to another function can cause different kinds of breakage.

Second, you have added the line, but this also moved the return 
statement out of the if condition. This way all bcm_uart_probe() calls 
will end up with -ENODEV return value.

Please stop blindly fixing the coccinelle warnings. Take care to read 
and understand the code.

>           return -ENODEV;
> 
>       port->iotype = UPIO_MEM;
Greg Kroah-Hartman July 18, 2023, 1:25 p.m. UTC | #2
On Tue, Jul 18, 2023 at 03:55:23PM +0800, sunran001@208suo.com wrote:
> This patch fixes the following Coccinelle error:
> 
> ./drivers/tty/serial/bcm63xx_uart.c:854:2-8: ERROR: missing clk_put;
> clk_get on line 849 and execution via conditional on line 853
> 
> Signed-off-by: Ran Sun <sunran001@208suo.com>
> ---
>  drivers/tty/serial/bcm63xx_uart.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/bcm63xx_uart.c
> b/drivers/tty/serial/bcm63xx_uart.c
> index 55e82d0bf92d..7353b683952d 100644
> --- a/drivers/tty/serial/bcm63xx_uart.c
> +++ b/drivers/tty/serial/bcm63xx_uart.c
> @@ -851,6 +851,7 @@ static int bcm_uart_probe(struct platform_device *pdev)
>          clk = of_clk_get(pdev->dev.of_node, 0);
> 
>      if (IS_ERR(clk))
> +        clk_put(clk);
>          return -ENODEV;
> 
>      port->iotype = UPIO_MEM;

Ran, as was pointed out before, you obviously didn't even test this
patch, nor any of the other submissions you made.

Please take the time to learn C a bit better, and then start out in a
part of the kernel where basic changes are accepted, like
drivers/staging/ so that you can learn how to properly send patches
(this was incorrectly sent as well.)

Then you can work up to attempting to fix other changes like this, if
they are correct, and you will know how to properly test your changes
instead of just making rote changes like this without understanding the
implications of them.

best of luck!

greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/serial/bcm63xx_uart.c 
b/drivers/tty/serial/bcm63xx_uart.c
index 55e82d0bf92d..7353b683952d 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -851,6 +851,7 @@  static int bcm_uart_probe(struct platform_device 
*pdev)
          clk = of_clk_get(pdev->dev.of_node, 0);

      if (IS_ERR(clk))
+        clk_put(clk);
          return -ENODEV;