[PATCH 0/8] Use of_mm_gpiochip_remove

[PATCH 0/8] Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
counterpart for of_mm_gpiochip_add.

This set of patches implements the removing of all the drivers that
made use of of_mm_gpiochip_add.

Also some minor cleanups are done on those drivers.

Suggested-by: Linus Walleij <linus.walleij@linaro.org>

Ricardo Ribalda Delgado (8):
  gpio/mpc5200: Use of_mm_gpiochip_remove
  gpio/-mm-lantiq: Use devm_kzalloc
  gpio/mm-lantiq: Do not replicate code
  gpio-mm-lantiq: Use of_propery_read_u16
  gpio/gpio-mm-lantiq: Use of_mm_gpiochip_remove
  gpio/zevio: Use of_mm_gpiochip_remove
  gpio/mpc8xxx: Convert to platform device interface.
  gpio/mpc8xxx: Use of_mm_gpiochip_remove

 drivers/gpio/gpio-mm-lantiq.c | 41 ++++++++++++++-----------
 drivers/gpio/gpio-mpc5200.c   | 23 ++++++++++----
 drivers/gpio/gpio-mpc8xxx.c   | 70 ++++++++++++++++++++++++++-----------------
 drivers/gpio/gpio-zevio.c     | 12 ++++++++
 4 files changed, 95 insertions(+), 51 deletions(-)

-- 
2.1.4

[PATCH 1/8] gpio/mpc5200: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
counterpart for of_mm_gpiochip_add.

This patch implements the remove function of the driver making use of
it.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mpc5200.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-mpc5200.c b/drivers/gpio/gpio-mpc5200.c
index 8ce6c95..4c54215 100644
--- a/drivers/gpio/gpio-mpc5200.c
+++ b/drivers/gpio/gpio-mpc5200.c
@@ -155,10 +155,12 @@ static int mpc52xx_wkup_gpiochip_probe(struct platform_device *ofdev)
 	struct gpio_chip *gc;
 	int ret;
 
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc(&ofdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
+	platform_set_drvdata(ofdev, chip);
+
 	gc = &chip->mmchip.gc;
 
 	gc->ngpio            = 8;
@@ -181,7 +183,11 @@ static int mpc52xx_wkup_gpiochip_probe(struct platform_device *ofdev)
 
 static int mpc52xx_gpiochip_remove(struct platform_device *ofdev)
 {
-	return -EBUSY;
+	struct mpc52xx_gpiochip *chip = platform_get_drvdata(ofdev);
+
+	of_mm_gpiochip_remove(&chip->mmchip);
+
+	return 0;
 }
 
 static const struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
@@ -314,10 +320,12 @@ static int mpc52xx_simple_gpiochip_probe(struct platform_device *ofdev)
 	struct mpc52xx_gpio __iomem *regs;
 	int ret;
 
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc(&ofdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
+	platform_set_drvdata(ofdev, chip);
+
 	gc = &chip->mmchip.gc;
 
 	gc->ngpio            = 32;
@@ -363,11 +371,16 @@ static int __init mpc52xx_gpio_init(void)
 	return 0;
 }
 
-
 /* Make sure we get initialised before anyone else tries to use us */
 subsys_initcall(mpc52xx_gpio_init);
 
-/* No exit call at the moment as we cannot unregister of gpio chips */
+static void __exit mpc52xx_gpio_exit(void)
+{
+	platform_driver_unregister(&mpc52xx_wkup_gpiochip_driver);
+
+	platform_driver_unregister(&mpc52xx_simple_gpiochip_driver);
+}
+module_exit(mpc52xx_gpio_exit);
 
 MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
-- 
2.1.4

[PATCH 2/8] gpio/-mm-lantiq: Use devm_kzalloc

[Ricardo Ribalda Delgado]

Replace kzalloc with the device managed devm_kzalloc

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: John Crispin <blogic@openwrt.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mm-lantiq.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index f228b1c..7890163 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -107,14 +107,13 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct ltq_mm *chip;
 	const __be32 *shadow;
-	int ret = 0;
 
 	if (!res) {
 		dev_err(&pdev->dev, "failed to get memory resource\n");
 		return -ENOENT;
 	}
 
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
@@ -129,10 +128,7 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	if (shadow)
 		chip->shadow = be32_to_cpu(*shadow);
 
-	ret = of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
-	if (ret)
-		kfree(chip);
-	return ret;
+	return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
 }
 
 static const struct of_device_id ltq_mm_match[] = {
-- 
2.1.4

[PATCH 3/8] gpio/mm-lantiq: Do not replicate code

[Ricardo Ribalda Delgado]

Do not replicate code from of_mm_gpiochip_add.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: John Crispin <blogic@openwrt.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mm-lantiq.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index 7890163..7d55161 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -104,21 +104,14 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
 
 static int ltq_mm_probe(struct platform_device *pdev)
 {
-	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct ltq_mm *chip;
 	const __be32 *shadow;
 
-	if (!res) {
-		dev_err(&pdev->dev, "failed to get memory resource\n");
-		return -ENOENT;
-	}
-
 	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
 	chip->mmchip.gc.ngpio = 16;
-	chip->mmchip.gc.label = "gpio-mm-ltq";
 	chip->mmchip.gc.direction_output = ltq_mm_dir_out;
 	chip->mmchip.gc.set = ltq_mm_set;
 	chip->mmchip.save_regs = ltq_mm_save_regs;
-- 
2.1.4

[PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16

[Ricardo Ribalda Delgado]

Instead of parsing manually the shadow content, use the much simpler
helper of_property_read_u16.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Crispin <blogic@openwrt.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mm-lantiq.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index 7d55161..b511a6b 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
 static int ltq_mm_probe(struct platform_device *pdev)
 {
 	struct ltq_mm *chip;
-	const __be32 *shadow;
 
 	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
@@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	chip->mmchip.save_regs = ltq_mm_save_regs;
 
 	/* store the shadow value if one was passed by the devicetree */
-	shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
-	if (shadow)
-		chip->shadow = be32_to_cpu(*shadow);
+	of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
+			     &chip->shadow);
 
 	return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
 }
-- 
2.1.4

[PATCH 5/8] gpio/gpio-mm-lantiq: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
counterpart for of_mm_gpiochip_add.

This patch implements the remove function of the driver making use of
it.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: John Crispin <blogic@openwrt.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mm-lantiq.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index b511a6b..082c1d5 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -110,6 +110,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	if (!chip)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, chip);
+
 	chip->mmchip.gc.ngpio = 16;
 	chip->mmchip.gc.direction_output = ltq_mm_dir_out;
 	chip->mmchip.gc.set = ltq_mm_set;
@@ -122,6 +124,15 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
 }
 
+static int ltq_mm_remove(struct platform_device *pdev)
+{
+	struct ltq_mm *chip = platform_get_drvdata(pdev);
+
+	of_mm_gpiochip_remove(&chip->mmchip);
+
+	return 0;
+}
+
 static const struct of_device_id ltq_mm_match[] = {
 	{ .compatible = "lantiq,gpio-mm" },
 	{},
@@ -130,6 +141,7 @@ MODULE_DEVICE_TABLE(of, ltq_mm_match);
 
 static struct platform_driver ltq_mm_driver = {
 	.probe = ltq_mm_probe,
+	.remove = ltq_mm_remove,
 	.driver = {
 		.name = "gpio-mm-ltq",
 		.of_match_table = ltq_mm_match,
@@ -142,3 +154,9 @@ static int __init ltq_mm_init(void)
 }
 
 subsys_initcall(ltq_mm_init);
+
+static void __exit ltq_mm_exit(void)
+{
+	platform_driver_unregister(&ltq_mm_driver);
+}
+module_exit(ltq_mm_exit);
-- 
2.1.4

[PATCH 6/8] gpio/zevio: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
counterpart for of_mm_gpiochip_add.

This patch implements the remove function of the driver making use of
it.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Fabian Vogt <fabian@ritter-vogt.de>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-zevio.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpio/gpio-zevio.c b/drivers/gpio/gpio-zevio.c
index f769cd5..6f02d7c 100644
--- a/drivers/gpio/gpio-zevio.c
+++ b/drivers/gpio/gpio-zevio.c
@@ -181,6 +181,8 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	if (!controller)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, controller);
+
 	/* Copy our reference */
 	controller->chip.gc = zevio_gpio_chip;
 	controller->chip.gc.dev = &pdev->dev;
@@ -202,6 +204,15 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int zevio_gpio_remove(struct platform_device *pdev)
+{
+	struct zevio_gpio *controller = platform_get_drvdata(pdev);
+
+	of_mm_gpiochip_remove(&controller->chip);
+
+	return 0;
+}
+
 static const struct of_device_id zevio_gpio_of_match[] = {
 	{ .compatible = "lsi,zevio-gpio", },
 	{ },
@@ -215,6 +226,7 @@ static struct platform_driver zevio_gpio_driver = {
 		.of_match_table = zevio_gpio_of_match,
 	},
 	.probe		= zevio_gpio_probe,
+	.remove		= zevio_gpio_remove,
 };
 module_platform_driver(zevio_gpio_driver);
 
-- 
2.1.4

[PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Ricardo Ribalda Delgado]

This way we do not need to transverse the device tree manually.

Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpio/gpio-mpc8xxx.c | 48 +++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index d1ff879..57eb794 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
@@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
 	{}
 };
 
-static void __init mpc8xxx_add_controller(struct device_node *np)
+static int mpc8xxx_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
@@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	unsigned hwirq;
 	int ret;
 
-	mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
-	if (!mpc8xxx_gc) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
+	if (!mpc8xxx_gc)
+		return -ENOMEM;
 
 	spin_lock_init(&mpc8xxx_gc->lock);
 
@@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 
 	ret = of_mm_gpiochip_add(np, mm_gc);
 	if (ret)
-		goto err;
+		return ret;
 
 	hwirq = irq_of_parse_and_map(np, 0);
 	if (hwirq == NO_IRQ)
-		goto skip_irq;
+		return 0;
 
 	mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
 					&mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
 	if (!mpc8xxx_gc->irq)
-		goto skip_irq;
+		return 0;
 
 	id = of_match_node(mpc8xxx_gpio_ids, np);
 	if (id)
@@ -397,24 +397,20 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	irq_set_handler_data(hwirq, mpc8xxx_gc);
 	irq_set_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
 
-skip_irq:
-	return;
-
-err:
-	pr_err("%s: registration failed with status %d\n",
-	       np->full_name, ret);
-	kfree(mpc8xxx_gc);
-
-	return;
+	return 0;
 }
 
-static int __init mpc8xxx_add_gpiochips(void)
-{
-	struct device_node *np;
-
-	for_each_matching_node(np, mpc8xxx_gpio_ids)
-		mpc8xxx_add_controller(np);
+static struct platform_driver mpc8xxx_plat_driver = {
+	.probe		= mpc8xxx_probe,
+	.driver		= {
+		.name = "gpio-mpc8xxx",
+		.of_match_table	= mpc8xxx_gpio_ids,
+	},
+};
 
-	return 0;
+static int __init mpc8xxx_init(void)
+{
+	return platform_driver_register(&mpc8xxx_plat_driver);
 }
-arch_initcall(mpc8xxx_add_gpiochips);
+
+arch_initcall(mpc8xxx_init);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Ricardo Ribalda Delgado]

This way we do not need to transverse the device tree manually.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Peter Korsgaard <jacmet@sunsite.dk>
Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mpc8xxx.c | 48 +++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index d1ff879..57eb794 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
@@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
 	{}
 };
 
-static void __init mpc8xxx_add_controller(struct device_node *np)
+static int mpc8xxx_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
@@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	unsigned hwirq;
 	int ret;
 
-	mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
-	if (!mpc8xxx_gc) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
+	if (!mpc8xxx_gc)
+		return -ENOMEM;
 
 	spin_lock_init(&mpc8xxx_gc->lock);
 
@@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 
 	ret = of_mm_gpiochip_add(np, mm_gc);
 	if (ret)
-		goto err;
+		return ret;
 
 	hwirq = irq_of_parse_and_map(np, 0);
 	if (hwirq == NO_IRQ)
-		goto skip_irq;
+		return 0;
 
 	mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
 					&mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
 	if (!mpc8xxx_gc->irq)
-		goto skip_irq;
+		return 0;
 
 	id = of_match_node(mpc8xxx_gpio_ids, np);
 	if (id)
@@ -397,24 +397,20 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	irq_set_handler_data(hwirq, mpc8xxx_gc);
 	irq_set_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
 
-skip_irq:
-	return;
-
-err:
-	pr_err("%s: registration failed with status %d\n",
-	       np->full_name, ret);
-	kfree(mpc8xxx_gc);
-
-	return;
+	return 0;
 }
 
-static int __init mpc8xxx_add_gpiochips(void)
-{
-	struct device_node *np;
-
-	for_each_matching_node(np, mpc8xxx_gpio_ids)
-		mpc8xxx_add_controller(np);
+static struct platform_driver mpc8xxx_plat_driver = {
+	.probe		= mpc8xxx_probe,
+	.driver		= {
+		.name = "gpio-mpc8xxx",
+		.of_match_table	= mpc8xxx_gpio_ids,
+	},
+};
 
-	return 0;
+static int __init mpc8xxx_init(void)
+{
+	return platform_driver_register(&mpc8xxx_plat_driver);
 }
-arch_initcall(mpc8xxx_add_gpiochips);
+
+arch_initcall(mpc8xxx_init);
-- 
2.1.4

[PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
counterpart for of_mm_gpiochip_add.

This patch implements the remove function of the driver making use of
it.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-mpc8xxx.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index 57eb794..a6952ba3 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -40,6 +40,7 @@ struct mpc8xxx_gpio_chip {
 	 */
 	u32 data;
 	struct irq_domain *irq;
+	unsigned int irqn;
 	const void *of_dev_id_data;
 };
 
@@ -350,13 +351,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
 	const struct of_device_id *id;
-	unsigned hwirq;
 	int ret;
 
 	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
 	if (!mpc8xxx_gc)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, mpc8xxx_gc);
+
 	spin_lock_init(&mpc8xxx_gc->lock);
 
 	mm_gc = &mpc8xxx_gc->mm_gc;
@@ -377,8 +379,8 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	hwirq = irq_of_parse_and_map(np, 0);
-	if (hwirq == NO_IRQ)
+	mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
+	if (mpc8xxx_gc->irqn == NO_IRQ)
 		return 0;
 
 	mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
@@ -394,14 +396,30 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 	out_be32(mm_gc->regs + GPIO_IER, 0xffffffff);
 	out_be32(mm_gc->regs + GPIO_IMR, 0);
 
-	irq_set_handler_data(hwirq, mpc8xxx_gc);
-	irq_set_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
+	irq_set_handler_data(mpc8xxx_gc->irqn, mpc8xxx_gc);
+	irq_set_chained_handler(mpc8xxx_gc->irqn, mpc8xxx_gpio_irq_cascade);
+
+	return 0;
+}
+
+static int mpc8xxx_remove(struct platform_device *pdev)
+{
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = platform_get_drvdata(pdev);
+
+	if (mpc8xxx_gc->irq) {
+		irq_set_handler_data(mpc8xxx_gc->irqn, NULL);
+		irq_set_chained_handler(mpc8xxx_gc->irqn, NULL);
+		irq_domain_remove(mpc8xxx_gc->irq);
+	}
+
+	of_mm_gpiochip_remove(&mpc8xxx_gc->mm_gc);
 
 	return 0;
 }
 
 static struct platform_driver mpc8xxx_plat_driver = {
 	.probe		= mpc8xxx_probe,
+	.remove		= mpc8xxx_remove,
 	.driver		= {
 		.name = "gpio-mpc8xxx",
 		.of_match_table	= mpc8xxx_gpio_ids,
-- 
2.1.4

Re: [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16

[Mark Rutland]

On Sun, Jan 18, 2015 at 11:39:29AM +0000, Ricardo Ribalda Delgado wrote:
> Instead of parsing manually the shadow content, use the much simpler
> helper of_property_read_u16.
> 
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpio/gpio-mm-lantiq.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
> index 7d55161..b511a6b 100644
> --- a/drivers/gpio/gpio-mm-lantiq.c
> +++ b/drivers/gpio/gpio-mm-lantiq.c
> @@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
>  static int ltq_mm_probe(struct platform_device *pdev)
>  {
>  	struct ltq_mm *chip;
> -	const __be32 *shadow;
>  
>  	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>  	if (!chip)
> @@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
>  	chip->mmchip.save_regs = ltq_mm_save_regs;
>  
>  	/* store the shadow value if one was passed by the devicetree */
> -	shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
> -	if (shadow)
> -		chip->shadow = be32_to_cpu(*shadow);
> +	of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
> +			     &chip->shadow);

The old code and new code aren't equivalent:

The binding doesn't specify the use of /bits/ 16 <...>, nor does the
only in-tree dts use it:

arch/mips/boot/dts/easy50712.dts:               lantiq,shadow = <0xfff>;

So in the DTB, lantiq-shadow will be stored as a be32 value. The byte
stream will look like: [00 00 0f ff].

The existing code reads all 4 bytes, and converts them as required,
reading the expected value of 0xfff. If you use of_property_read_u16,
this will only take the first 2 bytes, and will incorrectly read 0.

You can instead use of_property_read_u32, which will function
equivalently to the existing code.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16

[Mark Rutland]

On Sun, Jan 18, 2015 at 11:39:29AM +0000, Ricardo Ribalda Delgado wrote:
> Instead of parsing manually the shadow content, use the much simpler
> helper of_property_read_u16.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Crispin <blogic@openwrt.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/gpio/gpio-mm-lantiq.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
> index 7d55161..b511a6b 100644
> --- a/drivers/gpio/gpio-mm-lantiq.c
> +++ b/drivers/gpio/gpio-mm-lantiq.c
> @@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
>  static int ltq_mm_probe(struct platform_device *pdev)
>  {
>  	struct ltq_mm *chip;
> -	const __be32 *shadow;
>  
>  	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>  	if (!chip)
> @@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
>  	chip->mmchip.save_regs = ltq_mm_save_regs;
>  
>  	/* store the shadow value if one was passed by the devicetree */
> -	shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
> -	if (shadow)
> -		chip->shadow = be32_to_cpu(*shadow);
> +	of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
> +			     &chip->shadow);

The old code and new code aren't equivalent:

The binding doesn't specify the use of /bits/ 16 <...>, nor does the
only in-tree dts use it:

arch/mips/boot/dts/easy50712.dts:               lantiq,shadow = <0xfff>;

So in the DTB, lantiq-shadow will be stored as a be32 value. The byte
stream will look like: [00 00 0f ff].

The existing code reads all 4 bytes, and converts them as required,
reading the expected value of 0xfff. If you use of_property_read_u16,
this will only take the first 2 bytes, and will incorrectly read 0.

You can instead use of_property_read_u32, which will function
equivalently to the existing code.

Thanks,
Mark.

[PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32

[Ricardo Ribalda Delgado]

Instead of parsing manually the shadow content, use the much simpler
helper of_property_read_u32.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Crispin <blogic@openwrt.org>
Cc: devicetree@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---

v2: Reported by Mark Rutland <mark.rutland@arm.com>
 -use of_property_read_u32 instead of u16

 drivers/gpio/gpio-mm-lantiq.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index 7d55161..95ff180 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -105,7 +105,7 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
 static int ltq_mm_probe(struct platform_device *pdev)
 {
 	struct ltq_mm *chip;
-	const __be32 *shadow;
+	u32 shadow;
 
 	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
@@ -117,9 +117,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
 	chip->mmchip.save_regs = ltq_mm_save_regs;
 
 	/* store the shadow value if one was passed by the devicetree */
-	shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
-	if (shadow)
-		chip->shadow = be32_to_cpu(*shadow);
+	if (!of_property_read_u32(pdev->dev.of_node, "lantiq,shadow", &shadow))
+		chip->shadow = shadow;
 
 	return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
 }
-- 
2.1.4

Re: [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16

[Ricardo Ribalda Delgado]

Hello Mark

Thanks for noticing. I have just sent the v2 of the patch.

Regards!

On Mon, Jan 19, 2015 at 11:57 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Sun, Jan 18, 2015 at 11:39:29AM +0000, Ricardo Ribalda Delgado wrote:
>> Instead of parsing manually the shadow content, use the much simpler
>> helper of_property_read_u16.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Alexandre Courbot <gnurou@gmail.com>
>> Cc: Grant Likely <grant.likely@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: John Crispin <blogic@openwrt.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/gpio/gpio-mm-lantiq.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
>> index 7d55161..b511a6b 100644
>> --- a/drivers/gpio/gpio-mm-lantiq.c
>> +++ b/drivers/gpio/gpio-mm-lantiq.c
>> @@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
>>  static int ltq_mm_probe(struct platform_device *pdev)
>>  {
>>       struct ltq_mm *chip;
>> -     const __be32 *shadow;
>>
>>       chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>>       if (!chip)
>> @@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
>>       chip->mmchip.save_regs = ltq_mm_save_regs;
>>
>>       /* store the shadow value if one was passed by the devicetree */
>> -     shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
>> -     if (shadow)
>> -             chip->shadow = be32_to_cpu(*shadow);
>> +     of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
>> +                          &chip->shadow);
>
> The old code and new code aren't equivalent:
>
> The binding doesn't specify the use of /bits/ 16 <...>, nor does the
> only in-tree dts use it:
>
> arch/mips/boot/dts/easy50712.dts:               lantiq,shadow = <0xfff>;
>
> So in the DTB, lantiq-shadow will be stored as a be32 value. The byte
> stream will look like: [00 00 0f ff].
>
> The existing code reads all 4 bytes, and converts them as required,
> reading the expected value of 0xfff. If you use of_property_read_u16,
> this will only take the first 2 bytes, and will incorrectly read 0.
>
> You can instead use of_property_read_u32, which will function
> equivalently to the existing code.
>
> Thanks,
> Mark.



-- 
Ricardo Ribalda

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > This way we do not need to transverse the device tree manually.
 > Cc: Linus Walleij <linus.walleij@linaro.org>
 > Cc: Alexandre Courbot <gnurou@gmail.com>
 > Cc: Grant Likely <grant.likely@linaro.org>
 > Cc: Rob Herring <robh+dt@kernel.org>
 > Cc: Peter Korsgaard <jacmet@sunsite.dk>
 > Cc: devicetree@vger.kernel.org
 > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Looks sensible.

Acked-by: Peter Korsgaard <peter@korsgaard.com>

-- 
Bye, Peter Korsgaard

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > This way we do not need to transverse the device tree manually.
 > Cc: Linus Walleij <linus.walleij@linaro.org>
 > Cc: Alexandre Courbot <gnurou@gmail.com>
 > Cc: Grant Likely <grant.likely@linaro.org>
 > Cc: Rob Herring <robh+dt@kernel.org>
 > Cc: Peter Korsgaard <jacmet@sunsite.dk>
 > Cc: devicetree@vger.kernel.org
 > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
 > ---
 >  drivers/gpio/gpio-mpc8xxx.c | 48 +++++++++++++++++++++------------------------
 >  1 file changed, 22 insertions(+), 26 deletions(-)

 > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
 > index d1ff879..57eb794 100644
 > --- a/drivers/gpio/gpio-mpc8xxx.c
 > +++ b/drivers/gpio/gpio-mpc8xxx.c
 > @@ -15,6 +15,7 @@
 >  #include <linux/of.h>
 >  #include <linux/of_gpio.h>
 >  #include <linux/of_irq.h>
 > +#include <linux/of_platform.h>
 >  #include <linux/gpio.h>
 >  #include <linux/slab.h>
 >  #include <linux/irq.h>
 > @@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
 >  	{}
 >  };
 
 > -static void __init mpc8xxx_add_controller(struct device_node *np)
 > +static int mpc8xxx_probe(struct platform_device *pdev)
 >  {
 > +	struct device_node *np = pdev->dev.of_node;
 >  	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 >  	struct of_mm_gpio_chip *mm_gc;
 >  	struct gpio_chip *gc;
 > @@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 >  	unsigned hwirq;
 >  	int ret;
 
 > -	mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
 > -	if (!mpc8xxx_gc) {
 > -		ret = -ENOMEM;
 > -		goto err;
 > -	}
 > +	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
 > +	if (!mpc8xxx_gc)
 > +		return -ENOMEM;
 
 >  	spin_lock_init(&mpc8xxx_gc->lock);
 
 > @@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 
 >  	ret = of_mm_gpiochip_add(np, mm_gc);
 >  	if (ret)
 > -		goto err;
 > +		return ret;
 
 >  	hwirq = irq_of_parse_and_map(np, 0);
 >  	if (hwirq == NO_IRQ)
 > -		goto skip_irq;
 > +		return 0;

Actually, looking closer - Isn't this leaking mm_gc? It was as well
before, but it would be good to get it fixed.


 >       mpc8xxx_gc-> irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
 >  					&mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
 >  	if (!mpc8xxx_gc->irq)
 > -		goto skip_irq;
 > +		return 0;

And here as well?


-- 
Bye, Peter Korsgaard

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
 > counterpart for of_mm_gpiochip_add.

 > This patch implements the remove function of the driver making use of
 > it.

 > Cc: Linus Walleij <linus.walleij@linaro.org>
 > Cc: Alexandre Courbot <gnurou@gmail.com>
 > Cc: Peter Korsgaard <jacmet@sunsite.dk>
 > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
 > ---
 >  drivers/gpio/gpio-mpc8xxx.c | 28 +++++++++++++++++++++++-----
 >  1 file changed, 23 insertions(+), 5 deletions(-)

 > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
 > index 57eb794..a6952ba3 100644
 > --- a/drivers/gpio/gpio-mpc8xxx.c
 > +++ b/drivers/gpio/gpio-mpc8xxx.c
 > @@ -40,6 +40,7 @@ struct mpc8xxx_gpio_chip {
 >  	 */
 >  	u32 data;
 >  	struct irq_domain *irq;
 > +	unsigned int irqn;
 >  	const void *of_dev_id_data;
 >  };
 
 > @@ -350,13 +351,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 >  	struct of_mm_gpio_chip *mm_gc;
 >  	struct gpio_chip *gc;
 >  	const struct of_device_id *id;
 > -	unsigned hwirq;
 >  	int ret;
 
 >  	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
 >  	if (!mpc8xxx_gc)
 >  		return -ENOMEM;
 
 > +	platform_set_drvdata(pdev, mpc8xxx_gc);
 > +
 >  	spin_lock_init(&mpc8xxx_gc->lock);
 
 >  	mm_gc = &mpc8xxx_gc->mm_gc;
 > @@ -377,8 +379,8 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 >  	if (ret)
 >  		return ret;
 
 > -	hwirq = irq_of_parse_and_map(np, 0);
 > -	if (hwirq == NO_IRQ)
 > +	mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
 > +	if (mpc8xxx_gc->irqn == NO_IRQ)
 >  		return 0;


With this return 0 converted to do of_mm_gpiochip_remove():

Acked-by: Peter Korsgaard <peter@korsgaard.com>

-- 
Bye, Peter Korsgaard

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Hello Peter


>
>  > -    hwirq = irq_of_parse_and_map(np, 0);
>  > -    if (hwirq == NO_IRQ)
>  > +    mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
>  > +    if (mpc8xxx_gc->irqn == NO_IRQ)
>  >              return 0;
>
>
> With this return 0 converted to do of_mm_gpiochip_remove():

Are you sure? The driver can still work as a normal gpio without the
irq domain part and the remove function consider this option. The
original code did also continue....

If you still want to abort if no irq I can of course make the change.

Regards!

>
> Acked-by: Peter Korsgaard <peter@korsgaard.com>
>
> --
> Bye, Peter Korsgaard

Regards



-- 
Ricardo Ribalda

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Ricardo Ribalda Delgado]

Hello Peter

I thought the logic behind the original driver was:

1) Create gpiochip so it can be used by other modules/userland
2) if there is also a irq available for it, create a irqdomain.
otherwise continue.

I can change the code so it does not continue, and exit cleanly if the
irqdomain fails.

You are the original developer of the driver. So: do you think it
makes sense to continue only as a gpiochip? :P

If I need to change it I rather add it as a another patch to the
patchset, so if it causes an issue somewhere it can be found easier
via bisect.


Regards!







On Mon, Jan 19, 2015 at 11:48 PM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:
>
>  > This way we do not need to transverse the device tree manually.
>  > Cc: Linus Walleij <linus.walleij@linaro.org>
>  > Cc: Alexandre Courbot <gnurou@gmail.com>
>  > Cc: Grant Likely <grant.likely@linaro.org>
>  > Cc: Rob Herring <robh+dt@kernel.org>
>  > Cc: Peter Korsgaard <jacmet@sunsite.dk>
>  > Cc: devicetree@vger.kernel.org
>  > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>  > ---
>  >  drivers/gpio/gpio-mpc8xxx.c | 48 +++++++++++++++++++++------------------------
>  >  1 file changed, 22 insertions(+), 26 deletions(-)
>
>  > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
>  > index d1ff879..57eb794 100644
>  > --- a/drivers/gpio/gpio-mpc8xxx.c
>  > +++ b/drivers/gpio/gpio-mpc8xxx.c
>  > @@ -15,6 +15,7 @@
>  >  #include <linux/of.h>
>  >  #include <linux/of_gpio.h>
>  >  #include <linux/of_irq.h>
>  > +#include <linux/of_platform.h>
>  >  #include <linux/gpio.h>
>  >  #include <linux/slab.h>
>  >  #include <linux/irq.h>
>  > @@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
>  >      {}
>  >  };
>
>  > -static void __init mpc8xxx_add_controller(struct device_node *np)
>  > +static int mpc8xxx_probe(struct platform_device *pdev)
>  >  {
>  > +    struct device_node *np = pdev->dev.of_node;
>  >      struct mpc8xxx_gpio_chip *mpc8xxx_gc;
>  >      struct of_mm_gpio_chip *mm_gc;
>  >      struct gpio_chip *gc;
>  > @@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
>  >      unsigned hwirq;
>  >      int ret;
>
>  > -    mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
>  > -    if (!mpc8xxx_gc) {
>  > -            ret = -ENOMEM;
>  > -            goto err;
>  > -    }
>  > +    mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
>  > +    if (!mpc8xxx_gc)
>  > +            return -ENOMEM;
>
>  >      spin_lock_init(&mpc8xxx_gc->lock);
>
>  > @@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
>
>  >      ret = of_mm_gpiochip_add(np, mm_gc);
>  >      if (ret)
>  > -            goto err;
>  > +            return ret;
>
>  >      hwirq = irq_of_parse_and_map(np, 0);
>  >      if (hwirq == NO_IRQ)
>  > -            goto skip_irq;
>  > +            return 0;
>
> Actually, looking closer - Isn't this leaking mm_gc? It was as well
> before, but it would be good to get it fixed.
>
>
>  >       mpc8xxx_gc-> irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
>  >                                      &mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
>  >      if (!mpc8xxx_gc->irq)
>  > -            goto skip_irq;
>  > +            return 0;
>
> And here as well?
>
>
> --
> Bye, Peter Korsgaard



-- 
Ricardo Ribalda

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Alexandre Courbot]

On Sun, Jan 18, 2015 at 8:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> This way we do not need to transverse the device tree manually.

... and this makes things much more legible.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH 1/8] gpio/mpc5200: Use of_mm_gpiochip_remove

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
> counterpart for of_mm_gpiochip_add.
>
> This patch implements the remove function of the driver making use of
> it.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

OK nice stuff, patch applied.

Yours,
Linus Walleij

Re: [PATCH 2/8] gpio/-mm-lantiq: Use devm_kzalloc

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Replace kzalloc with the device managed devm_kzalloc
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: John Crispin <blogic@openwrt.org>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH 3/8] gpio/mm-lantiq: Do not replicate code

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Do not replicate code from of_mm_gpiochip_add.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: John Crispin <blogic@openwrt.org>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32

[Linus Walleij]

On Mon, Jan 19, 2015 at 12:27 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Instead of parsing manually the shadow content, use the much simpler
> helper of_property_read_u32.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Crispin <blogic@openwrt.org>
> Cc: devicetree@vger.kernel.org
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>
> v2: Reported by Mark Rutland <mark.rutland@arm.com>
>  -use of_property_read_u32 instead of u16

This v2 version applied.

Yours,
Linus Walleij

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> This way we do not need to transverse the device tree manually.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied with the ACKs, hope nothing explodes.

Yours,
Linus Walleij

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
> counterpart for of_mm_gpiochip_add.
>
> This patch implements the remove function of the driver making use of
> it.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Waiting for a v2 addressing the issues pointed out by Peter on this
patch.

Yours,
Linus Walleij

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Hello Linus

I am waiting from the confirmation of Peter if that is what he really needs.

It seems to me that the code should continue even if there is no
irqdomain created.

If we need to abort if there is no irqdomain I would rather create a
new patch with this change,  since it it is a change of the current
behaviour, so if something explodes the bisect will be easier.

I will attach to this mail a patch.

Regards

On Tue, Jan 20, 2015 at 11:17 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>
>> Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
>> counterpart for of_mm_gpiochip_add.
>>
>> This patch implements the remove function of the driver making use of
>> it.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Alexandre Courbot <gnurou@gmail.com>
>> Cc: Peter Korsgaard <jacmet@sunsite.dk>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>
> Waiting for a v2 addressing the issues pointed out by Peter on this
> patch.
>
> Yours,
> Linus Walleij



-- 
Ricardo Ribalda

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > Hello Peter
 >> 
 >> > -    hwirq = irq_of_parse_and_map(np, 0);
 >> > -    if (hwirq == NO_IRQ)
 >> > +    mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
 >> > +    if (mpc8xxx_gc->irqn == NO_IRQ)
 >> >              return 0;
 >> 
 >> 
 >> With this return 0 converted to do of_mm_gpiochip_remove():

 > Are you sure? The driver can still work as a normal gpio without the
 > irq domain part and the remove function consider this option. The
 > original code did also continue....

Ahh yes, you are right. Sorry, it's been a while since I wrote that code
(2008).

 > If you still want to abort if no irq I can of course make the change.

No, it is fine.

-- 
Bye, Peter Korsgaard

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > Hello Peter
 >> 
 >> > -    hwirq = irq_of_parse_and_map(np, 0);
 >> > -    if (hwirq == NO_IRQ)
 >> > +    mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
 >> > +    if (mpc8xxx_gc->irqn == NO_IRQ)
 >> >              return 0;
 >> 
 >> 
 >> With this return 0 converted to do of_mm_gpiochip_remove():

 > Are you sure? The driver can still work as a normal gpio without the
 > irq domain part and the remove function consider this option. The
 > original code did also continue....

Ahh yes, you are right. Sorry, it's been a while since I wrote that code
(2008).

 > If you still want to abort if no irq I can of course make the change.

No, it is fine.

-- 
Bye, Peter Korsgaard

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > Hello Peter
 > I thought the logic behind the original driver was:

 > 1) Create gpiochip so it can be used by other modules/userland
 > 2) if there is also a irq available for it, create a irqdomain.
 > otherwise continue.

Indeed - Sorry, I must have been confused yesterday.

-- 
Bye, Peter Korsgaard

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:

 > Hello Peter
 > I thought the logic behind the original driver was:

 > 1) Create gpiochip so it can be used by other modules/userland
 > 2) if there is also a irq available for it, create a irqdomain.
 > otherwise continue.

Indeed - Sorry, I must have been confused yesterday.

-- 
Bye, Peter Korsgaard

Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.

[Ricardo Ribalda Delgado]

No worries :)

On Tue, Jan 20, 2015 at 11:41 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:
>
>  > Hello Peter
>  > I thought the logic behind the original driver was:
>
>  > 1) Create gpiochip so it can be used by other modules/userland
>  > 2) if there is also a irq available for it, create a irqdomain.
>  > otherwise continue.
>
> Indeed - Sorry, I must have been confused yesterday.
>
> --
> Bye, Peter Korsgaard



-- 
Ricardo Ribalda

Re: [PATCH 8/8] gpio/mpc8xxx: Use of_mm_gpiochip_remove

[Linus Walleij]

On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a
> counterpart for of_mm_gpiochip_add.
>
> This patch implements the remove function of the driver making use of
> it.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij