All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@prisktech.co.nz (Tony Prisk)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] gpio: vt8500: memory cleanup missing
Date: Fri, 11 Jan 2013 08:09:46 +1300	[thread overview]
Message-ID: <1357844986-21891-1-git-send-email-linux@prisktech.co.nz> (raw)

This driver is missing a .remove callback, and the fail path on
probe is incomplete.

If an error occurs in vt8500_add_chips, gpio_base is not unmapped.
The driver is also ignoring the return value from this function so
if a chip fails to register it completes as successful.

Replaced pr_err with dev_err in vt8500_add_chips since the device is
available.

There is also no .remove callback defined. To allow removing the
registered chips, I have moved *vtchip to be a static global.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
v2:
Remove global variable and use platform_set_drvdata instead.

 drivers/gpio/gpio-vt8500.c |   51 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-vt8500.c b/drivers/gpio/gpio-vt8500.c
index b53320a..87e59b5 100644
--- a/drivers/gpio/gpio-vt8500.c
+++ b/drivers/gpio/gpio-vt8500.c
@@ -233,10 +233,12 @@ static int vt8500_add_chips(struct platform_device *pdev, void __iomem *base,
 			sizeof(struct vt8500_gpio_chip) * data->num_banks,
 			GFP_KERNEL);
 	if (!vtchip) {
-		pr_err("%s: failed to allocate chip memory\n", __func__);
+		dev_err(&pdev->dev, "failed to allocate chip memory\n");
 		return -ENOMEM;
 	}
 
+	platform_set_drvdata(pdev, vtchip);
+
 	for (i = 0; i < data->num_banks; i++) {
 		vtchip[i].base = base;
 		vtchip[i].regs = &data->banks[i];
@@ -261,6 +263,7 @@ static int vt8500_add_chips(struct platform_device *pdev, void __iomem *base,
 
 		gpiochip_add(chip);
 	}
+
 	return 0;
 }
 
@@ -273,36 +276,64 @@ static struct of_device_id vt8500_gpio_dt_ids[] = {
 
 static int vt8500_gpio_probe(struct platform_device *pdev)
 {
+	int ret;
 	void __iomem *gpio_base;
-	struct device_node *np;
+	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 				of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
 
-	if (!of_id) {
-		dev_err(&pdev->dev, "Failed to find gpio controller\n");
+	if (!np) {
+		dev_err(&pdev->dev, "GPIO node missing in devicetree\n");
 		return -ENODEV;
 	}
 
-	np = pdev->dev.of_node;
-	if (!np) {
-		dev_err(&pdev->dev, "Missing GPIO description in devicetree\n");
-		return -EFAULT;
+	if (!of_id) {
+		dev_err(&pdev->dev, "No matching driver data\n");
+		return -ENODEV;
 	}
 
 	gpio_base = of_iomap(np, 0);
 	if (!gpio_base) {
 		dev_err(&pdev->dev, "Unable to map GPIO registers\n");
-		of_node_put(np);
 		return -ENOMEM;
 	}
 
-	vt8500_add_chips(pdev, gpio_base, of_id->data);
+	ret = vt8500_add_chips(pdev, gpio_base, of_id->data);
+	if (ret) {
+		iounmap(gpio_base);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int vt8500_gpio_remove(struct platform_device *pdev)
+{
+	int i;
+	int ret;
+	const struct vt8500_gpio_data *data;
+	struct vt8500_gpio_chip *vtchip = platform_get_drvdata(pdev);
+	void __iomem *gpio_base = vtchip[0].base;
+	const struct of_device_id *of_id =
+				of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
+
+	data = of_id->data;
+
+	for (i = 0; i < data->num_banks; i++) {
+		ret = gpiochip_remove(&vtchip[i].chip);
+		if (ret)
+			dev_warn(&pdev->dev, "gpiochip_remove returned %d\n",
+				 ret);
+	}
+
+	iounmap(gpio_base);
 
 	return 0;
 }
 
 static struct platform_driver vt8500_gpio_driver = {
 	.probe		= vt8500_gpio_probe,
+	.remove		= vt8500_gpio_remove,
 	.driver		= {
 		.name	= "vt8500-gpio",
 		.owner	= THIS_MODULE,
-- 
1.7.9.5

WARNING: multiple messages have this Message-ID (diff)
From: Tony Prisk <linux@prisktech.co.nz>
To: Linux Walleij <linus.walleij@linaro.org>
Cc: vt8500-wm8505-linux-kernel@googlegroups.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux@arm.linux.org.uk,
	Tony Prisk <linux@prisktech.co.nz>
Subject: [PATCH v2] gpio: vt8500: memory cleanup missing
Date: Fri, 11 Jan 2013 08:09:46 +1300	[thread overview]
Message-ID: <1357844986-21891-1-git-send-email-linux@prisktech.co.nz> (raw)

This driver is missing a .remove callback, and the fail path on
probe is incomplete.

If an error occurs in vt8500_add_chips, gpio_base is not unmapped.
The driver is also ignoring the return value from this function so
if a chip fails to register it completes as successful.

Replaced pr_err with dev_err in vt8500_add_chips since the device is
available.

There is also no .remove callback defined. To allow removing the
registered chips, I have moved *vtchip to be a static global.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
v2:
Remove global variable and use platform_set_drvdata instead.

 drivers/gpio/gpio-vt8500.c |   51 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-vt8500.c b/drivers/gpio/gpio-vt8500.c
index b53320a..87e59b5 100644
--- a/drivers/gpio/gpio-vt8500.c
+++ b/drivers/gpio/gpio-vt8500.c
@@ -233,10 +233,12 @@ static int vt8500_add_chips(struct platform_device *pdev, void __iomem *base,
 			sizeof(struct vt8500_gpio_chip) * data->num_banks,
 			GFP_KERNEL);
 	if (!vtchip) {
-		pr_err("%s: failed to allocate chip memory\n", __func__);
+		dev_err(&pdev->dev, "failed to allocate chip memory\n");
 		return -ENOMEM;
 	}
 
+	platform_set_drvdata(pdev, vtchip);
+
 	for (i = 0; i < data->num_banks; i++) {
 		vtchip[i].base = base;
 		vtchip[i].regs = &data->banks[i];
@@ -261,6 +263,7 @@ static int vt8500_add_chips(struct platform_device *pdev, void __iomem *base,
 
 		gpiochip_add(chip);
 	}
+
 	return 0;
 }
 
@@ -273,36 +276,64 @@ static struct of_device_id vt8500_gpio_dt_ids[] = {
 
 static int vt8500_gpio_probe(struct platform_device *pdev)
 {
+	int ret;
 	void __iomem *gpio_base;
-	struct device_node *np;
+	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 				of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
 
-	if (!of_id) {
-		dev_err(&pdev->dev, "Failed to find gpio controller\n");
+	if (!np) {
+		dev_err(&pdev->dev, "GPIO node missing in devicetree\n");
 		return -ENODEV;
 	}
 
-	np = pdev->dev.of_node;
-	if (!np) {
-		dev_err(&pdev->dev, "Missing GPIO description in devicetree\n");
-		return -EFAULT;
+	if (!of_id) {
+		dev_err(&pdev->dev, "No matching driver data\n");
+		return -ENODEV;
 	}
 
 	gpio_base = of_iomap(np, 0);
 	if (!gpio_base) {
 		dev_err(&pdev->dev, "Unable to map GPIO registers\n");
-		of_node_put(np);
 		return -ENOMEM;
 	}
 
-	vt8500_add_chips(pdev, gpio_base, of_id->data);
+	ret = vt8500_add_chips(pdev, gpio_base, of_id->data);
+	if (ret) {
+		iounmap(gpio_base);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int vt8500_gpio_remove(struct platform_device *pdev)
+{
+	int i;
+	int ret;
+	const struct vt8500_gpio_data *data;
+	struct vt8500_gpio_chip *vtchip = platform_get_drvdata(pdev);
+	void __iomem *gpio_base = vtchip[0].base;
+	const struct of_device_id *of_id =
+				of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
+
+	data = of_id->data;
+
+	for (i = 0; i < data->num_banks; i++) {
+		ret = gpiochip_remove(&vtchip[i].chip);
+		if (ret)
+			dev_warn(&pdev->dev, "gpiochip_remove returned %d\n",
+				 ret);
+	}
+
+	iounmap(gpio_base);
 
 	return 0;
 }
 
 static struct platform_driver vt8500_gpio_driver = {
 	.probe		= vt8500_gpio_probe,
+	.remove		= vt8500_gpio_remove,
 	.driver		= {
 		.name	= "vt8500-gpio",
 		.owner	= THIS_MODULE,
-- 
1.7.9.5


             reply	other threads:[~2013-01-10 19:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-10 19:09 Tony Prisk [this message]
2013-01-10 19:09 ` [PATCH v2] gpio: vt8500: memory cleanup missing Tony Prisk
2013-01-14 14:14 ` Grant Likely
2013-01-14 14:14   ` Grant Likely
  -- strict thread matches above, loose matches on Subject: below --
2013-01-14 18:37 Tony Prisk
2013-01-14 18:37 ` Tony Prisk
2013-01-15  5:20 ` Tony Prisk
2013-01-15  5:20   ` Tony Prisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1357844986-21891-1-git-send-email-linux@prisktech.co.nz \
    --to=linux@prisktech.co.nz \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.