linux-kernel - Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0aa79585-c88e-e5b7-c260-6e9ceee6d776@ti.com>
Date:   Tue, 18 Jul 2017 11:50:11 -0500
From:   Suman Anna <s-anna@...com>
To:     Keerthy <j-keerthy@...com>, <linus.walleij@...aro.org>,
        <ssantosh@...nel.org>
CC:     <robh+dt@...nel.org>, <nsekhar@...com>, <fcooper@...com>,
        <linux-gpio@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of
 gpiochip_add_data

Hi Keerthy,

On 07/18/2017 05:57 AM, Keerthy wrote:
> Use the devm version of gpiochip_add_data and pass on the
> return value. Reset the static variables to 0 before returning.
> 
> Signed-off-by: Keerthy <j-keerthy@...com>
> ---
>  drivers/gpio/gpio-davinci.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 65cb359..2c88054 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -166,7 +166,7 @@ static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
>  static int davinci_gpio_probe(struct platform_device *pdev)
>  {
>  	static int ctrl_num, bank_base;
> -	int gpio, bank;
> +	int gpio, bank, ret = 0;
>  	unsigned ngpio, nbank;
>  	struct davinci_gpio_controller *chips;
>  	struct davinci_gpio_platform_data *pdata;
> @@ -232,7 +232,13 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>  	for (gpio = 0, bank = 0; gpio < ngpio; gpio += 32, bank++)
>  		chips->regs[bank] = gpio_base + offset_array[bank];
>  
> -	gpiochip_add_data(&chips->chip, chips);
> +	ret = devm_gpiochip_add_data(dev, &chips->chip, chips);
> +	if (ret) {
> +		ctrl_num = 0;
> +		bank_base = 0;

Hmm, this doesn't look right to me. These variables are defined as
static, and you are resetting them unconditionally. This should be an
issue when you have multiple devices and one of them fails.

regards
Suman

> +		return ret;
> +	}
> +
>  	platform_set_drvdata(pdev, chips);
>  	davinci_gpio_irq_setup(pdev);
>  	return 0;
>