lkml.org 
[lkml]   [2022]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] arch: powerpc: platforms: 85xx: Add missing of_node_put in sgy_cts1000.c
From
Le 16/06/2022 à 17:19, Liang He a écrit :
> In gpio_halt_probe(), of_find_matching_node() will return a node pointer with
> refcount incremented. We should use of_node_put() in each fail path or when it
> is not used anymore.
>
> Signed-off-by: Liang He <windhl@126.com>
> ---
> changelog:
>
> v2: use goto-label patch style advised by Christophe.
> v1: add of_node_put() before each exit.
>
> arch/powerpc/platforms/85xx/sgy_cts1000.c | 27 +++++++++++++++--------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c
> index 98ae64075193..e280f963d88c 100644
> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
> @@ -73,6 +73,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
> struct device_node *node = pdev->dev.of_node;
> int gpio, err, irq;
> int trigger;
> + int ret;
>
> if (!node)
> return -ENODEV;
> @@ -84,20 +85,24 @@ static int gpio_halt_probe(struct platform_device *pdev)
>
> /* Technically we could just read the first one, but punish
> * DT writers for invalid form. */
> - if (of_gpio_count(halt_node) != 1)
> - return -EINVAL;
> + if (of_gpio_count(halt_node) != 1) {
> + ret = -EINVAL;
> + goto err_put;
> + }
>
> /* Get the gpio number relative to the dynamic base. */
> gpio = of_get_gpio_flags(halt_node, 0, &flags);
> - if (!gpio_is_valid(gpio))
> - return -EINVAL;
> + if (!gpio_is_valid(gpio)) {
> + ret = -EINVAL;
> + gotot err_put;
> + }
>
> err = gpio_request(gpio, "gpio-halt");
> if (err) {
> printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n",
> gpio);
> - halt_node = NULL;
> - return err;
> + ret = err;
> + goto err_put;
> }
>
> trigger = (flags == OF_GPIO_ACTIVE_LOW);
> @@ -112,8 +117,8 @@ static int gpio_halt_probe(struct platform_device *pdev)
> printk(KERN_ERR "gpio-halt: error requesting IRQ %d for "
> "GPIO %d.\n", irq, gpio);
> gpio_free(gpio);
> - halt_node = NULL;
> - return err;
> + ret = err;
> + goto err_put;
> }
>
> /* Register our halt function */
> @@ -122,8 +127,12 @@ static int gpio_halt_probe(struct platform_device *pdev)
>
> printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d"
> " irq).\n", gpio, trigger, irq);
> + ret = 0;
>
> - return 0;
> +err_put:
> + of_node_put(halt_node);
> + halt_node = NULL;

Hi,
so now we set 'halt_node' to NULL even in the normal case.
This is really spurious.

Look at gpio_halt_cb(), but I think that this is just wrong and badly
breaks this driver.

CJ


> + return ret;
> }
>
> static int gpio_halt_remove(struct platform_device *pdev)

\
 
 \ /
  Last update: 2022-06-16 20:55    [W:0.053 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site