[PATCH gpio 0/4] cleanup for gpio drivers

[PATCH gpio 0/4] cleanup for gpio drivers

[Varka Bhadram]

Varka Bhadram (4):
  drivers: gpio: use devm_kzalloc
  gpio-max7300: remove 'ret' variable
  gpio-arizona: drop owner assignment from platform_drivers
  drivers: gpio: use (!foo) instead of (foo == NULL)

 drivers/gpio/gpio-adp5588.c  |    7 +++----
 drivers/gpio/gpio-arizona.c  |    3 +--
 drivers/gpio/gpio-da9052.c   |    2 +-
 drivers/gpio/gpio-da9055.c   |    2 +-
 drivers/gpio/gpio-kempld.c   |    2 +-
 drivers/gpio/gpio-max7300.c  |    4 +---
 drivers/gpio/gpio-mc33880.c  |    2 +-
 drivers/gpio/gpio-mcp23s08.c |    7 ++++---
 8 files changed, 13 insertions(+), 16 deletions(-)

-- 
1.7.9.5

[PATCH gpio 1/4] drivers: gpio: use devm_kzalloc

[Varka Bhadram]

We can use devres API for allocating memory. No need of using kfree.

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 drivers/gpio/gpio-adp5588.c  |    3 +--
 drivers/gpio/gpio-mcp23s08.c |    7 ++++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 3beed6e..0de8c70 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -378,7 +378,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
 	if (dev == NULL)
 		return -ENOMEM;
 
@@ -446,7 +446,6 @@ static int adp5588_gpio_probe(struct i2c_client *client,
 err_irq:
 	adp5588_irq_teardown(dev);
 err:
-	kfree(dev);
 	return ret;
 }
 
diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index eea5d7e..a17b81f 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -949,10 +949,12 @@ static int mcp23s08_probe(struct spi_device *spi)
 	if (!chips)
 		return -ENODEV;
 
-	data = kzalloc(sizeof(*data) + chips * sizeof(struct mcp23s08),
-			GFP_KERNEL);
+	data = devm_kzalloc(&spi->dev,
+			    sizeof(*data) + chips * sizeof(struct mcp23s08),
+			    GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
+
 	spi_set_drvdata(spi, data);
 
 	spi->irq = irq_of_parse_and_map(spi->dev.of_node, 0);
@@ -989,7 +991,6 @@ fail:
 			continue;
 		gpiochip_remove(&data->mcp[addr]->chip);
 	}
-	kfree(data);
 	return status;
 }
 
-- 
1.7.9.5

[PATCH gpio 2/4] gpio-max7300: remove 'ret' variable

[Varka Bhadram]

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 drivers/gpio/gpio-max7300.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-max7300.c b/drivers/gpio/gpio-max7300.c
index 40ab6df..0cc2c27 100644
--- a/drivers/gpio/gpio-max7300.c
+++ b/drivers/gpio/gpio-max7300.c
@@ -35,7 +35,6 @@ static int max7300_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct max7301 *ts;
-	int ret;
 
 	if (!i2c_check_functionality(client->adapter,
 			I2C_FUNC_SMBUS_BYTE_DATA))
@@ -49,8 +48,7 @@ static int max7300_probe(struct i2c_client *client,
 	ts->write = max7300_i2c_write;
 	ts->dev = &client->dev;
 
-	ret = __max730x_probe(ts);
-	return ret;
+	return __max730x_probe(ts);
 }
 
 static int max7300_remove(struct i2c_client *client)
-- 
1.7.9.5

[PATCH gpio 3/4] gpio-arizona: drop owner assignment from platform_drivers

[Varka Bhadram]

This driver no need to set the owner field, it will be populated
by driver core.

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 drivers/gpio/gpio-arizona.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c
index fe369f5..ab35038 100644
--- a/drivers/gpio/gpio-arizona.c
+++ b/drivers/gpio/gpio-arizona.c
@@ -155,7 +155,6 @@ static int arizona_gpio_remove(struct platform_device *pdev)
 
 static struct platform_driver arizona_gpio_driver = {
 	.driver.name	= "arizona-gpio",
-	.driver.owner	= THIS_MODULE,
 	.probe		= arizona_gpio_probe,
 	.remove		= arizona_gpio_remove,
 };
-- 
1.7.9.5

[PATCH gpio 4/4] drivers: gpio: use (!foo) instead of (foo == NULL)

[Varka Bhadram]

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 drivers/gpio/gpio-adp5588.c |    4 ++--
 drivers/gpio/gpio-arizona.c |    2 +-
 drivers/gpio/gpio-da9052.c  |    2 +-
 drivers/gpio/gpio-da9055.c  |    2 +-
 drivers/gpio/gpio-kempld.c  |    2 +-
 drivers/gpio/gpio-mc33880.c |    2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 0de8c70..c90273d 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -367,7 +367,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
 	struct gpio_chip *gc;
 	int ret, i, revid;
 
-	if (pdata == NULL) {
+	if (!pdata) {
 		dev_err(&client->dev, "missing platform data\n");
 		return -ENODEV;
 	}
@@ -379,7 +379,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
 	}
 
 	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
-	if (dev == NULL)
+	if (!dev)
 		return -ENOMEM;
 
 	dev->client = client;
diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c
index ab35038..eb5a0ce 100644
--- a/drivers/gpio/gpio-arizona.c
+++ b/drivers/gpio/gpio-arizona.c
@@ -103,7 +103,7 @@ static int arizona_gpio_probe(struct platform_device *pdev)
 
 	arizona_gpio = devm_kzalloc(&pdev->dev, sizeof(*arizona_gpio),
 				    GFP_KERNEL);
-	if (arizona_gpio == NULL)
+	if (!arizona_gpio)
 		return -ENOMEM;
 
 	arizona_gpio->arizona = arizona;
diff --git a/drivers/gpio/gpio-da9052.c b/drivers/gpio/gpio-da9052.c
index 389a4d2..2e9578e 100644
--- a/drivers/gpio/gpio-da9052.c
+++ b/drivers/gpio/gpio-da9052.c
@@ -212,7 +212,7 @@ static int da9052_gpio_probe(struct platform_device *pdev)
 	int ret;
 
 	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
-	if (gpio == NULL)
+	if (!gpio)
 		return -ENOMEM;
 
 	gpio->da9052 = dev_get_drvdata(pdev->dev.parent);
diff --git a/drivers/gpio/gpio-da9055.c b/drivers/gpio/gpio-da9055.c
index b8d7570..7227e6e 100644
--- a/drivers/gpio/gpio-da9055.c
+++ b/drivers/gpio/gpio-da9055.c
@@ -146,7 +146,7 @@ static int da9055_gpio_probe(struct platform_device *pdev)
 	int ret;
 
 	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
-	if (gpio == NULL)
+	if (!gpio)
 		return -ENOMEM;
 
 	gpio->da9055 = dev_get_drvdata(pdev->dev.parent);
diff --git a/drivers/gpio/gpio-kempld.c b/drivers/gpio/gpio-kempld.c
index 443518f..6b8115f 100644
--- a/drivers/gpio/gpio-kempld.c
+++ b/drivers/gpio/gpio-kempld.c
@@ -156,7 +156,7 @@ static int kempld_gpio_probe(struct platform_device *pdev)
 	}
 
 	gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
-	if (gpio == NULL)
+	if (!gpio)
 		return -ENOMEM;
 
 	gpio->pld = pld;
diff --git a/drivers/gpio/gpio-mc33880.c b/drivers/gpio/gpio-mc33880.c
index 4e3e160..a431604 100644
--- a/drivers/gpio/gpio-mc33880.c
+++ b/drivers/gpio/gpio-mc33880.c
@@ -151,7 +151,7 @@ static int mc33880_remove(struct spi_device *spi)
 	struct mc33880 *mc;
 
 	mc = spi_get_drvdata(spi);
-	if (mc == NULL)
+	if (!mc)
 		return -ENODEV;
 
 	gpiochip_remove(&mc->chip);
-- 
1.7.9.5

Re: [PATCH gpio 1/4] drivers: gpio: use devm_kzalloc

[Linus Walleij]

On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:

> We can use devres API for allocating memory. No need of using kfree.
>
> Signed-off-by: Varka Bhadram <varkab@cdac.in>

Straight forward clean-up. Patch applied.

Yours,
Linus Walleij

Re: [PATCH gpio 2/4] gpio-max7300: remove 'ret' variable

[Linus Walleij]

On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:

> Signed-off-by: Varka Bhadram <varkab@cdac.in>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH gpio 3/4] gpio-arizona: drop owner assignment from platform_drivers

[Linus Walleij]

On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:

> This driver no need to set the owner field, it will be populated
> by driver core.
>
> Signed-off-by: Varka Bhadram <varkab@cdac.in>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH gpio 4/4] drivers: gpio: use (!foo) instead of (foo == NULL)

[Linus Walleij]

On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:

> Signed-off-by: Varka Bhadram <varkab@cdac.in>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH gpio 1/4] drivers: gpio: use devm_kzalloc

[Johan Hovold]

On Tue, Mar 31, 2015 at 09:49:08AM +0530, Varka Bhadram wrote:
> We can use devres API for allocating memory. No need of using kfree.
> 
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>  drivers/gpio/gpio-adp5588.c  |    3 +--
>  drivers/gpio/gpio-mcp23s08.c |    7 ++++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
> index 3beed6e..0de8c70 100644
> --- a/drivers/gpio/gpio-adp5588.c
> +++ b/drivers/gpio/gpio-adp5588.c
> @@ -378,7 +378,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
>  		return -EIO;
>  	}
>  
> -	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
>  	if (dev == NULL)
>  		return -ENOMEM;
>  
> @@ -446,7 +446,6 @@ static int adp5588_gpio_probe(struct i2c_client *client,
>  err_irq:
>  	adp5588_irq_teardown(dev);
>  err:
> -	kfree(dev);
>  	return ret;
>  }

You cannot just switch the allocation to devm_kzalloc without removing
the kfree from the remove callback.

> diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
> index eea5d7e..a17b81f 100644
> --- a/drivers/gpio/gpio-mcp23s08.c
> +++ b/drivers/gpio/gpio-mcp23s08.c
> @@ -949,10 +949,12 @@ static int mcp23s08_probe(struct spi_device *spi)
>  	if (!chips)
>  		return -ENODEV;
>  
> -	data = kzalloc(sizeof(*data) + chips * sizeof(struct mcp23s08),
> -			GFP_KERNEL);
> +	data = devm_kzalloc(&spi->dev,
> +			    sizeof(*data) + chips * sizeof(struct mcp23s08),
> +			    GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> +
>  	spi_set_drvdata(spi, data);
>  
>  	spi->irq = irq_of_parse_and_map(spi->dev.of_node, 0);
> @@ -989,7 +991,6 @@ fail:
>  			continue;
>  		gpiochip_remove(&data->mcp[addr]->chip);
>  	}
> -	kfree(data);
>  	return status;
>  }

Same bug again.

Johan

Re: [PATCH gpio 1/4] drivers: gpio: use devm_kzalloc

[Johan Hovold]

On Tue, Apr 07, 2015 at 04:19:17PM +0200, Linus Walleij wrote:
> On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:
> 
> > We can use devres API for allocating memory. No need of using kfree.
> >
> > Signed-off-by: Varka Bhadram <varkab@cdac.in>
> 
> Straight forward clean-up. Patch applied.

You should probably drop this one (or remove the kfrees from the remove
callbacks as a follow up).

Johan

Re: [PATCH gpio 4/4] drivers: gpio: use (!foo) instead of (foo == NULL)

[Alexandre Courbot]

I wonder, is this any better, or is there a recommended guideline
somewhere? Testing against NULL at least highlights the fact that the
variable is a pointer, this looks like valuable information to me.

On Tue, Apr 7, 2015 at 11:22 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Mar 31, 2015 at 6:19 AM, Varka Bhadram <varkabhadram@gmail.com> wrote:
>
>> Signed-off-by: Varka Bhadram <varkab@cdac.in>
>
> Patch applied.
>
> Yours,
> Linus Walleij

Re: [PATCH gpio 4/4] drivers: gpio: use (!foo) instead of (foo == NULL)

[Linus Walleij]

On Wed, Apr 8, 2015 at 3:50 AM, Alexandre Courbot <gnurou@gmail.com> wrote:

> I wonder, is this any better, or is there a recommended guideline
> somewhere?

This construct is used in Chapter 7 of
Documentation/CodingStyle

Though the use is not commented on.

But it's fairly common & compact & nice I think.

Yours,
Linus Walleij