Skip to contentSkip to conversation

gpio: em: Add Device Tree support

PATCH In review Magnus Damm 4 participants 5 messages Feb 26, 2013 – Mar 2, 2013

Subsystems (2)
GPIO SuperH
lore
Share & tools
lore

Magnus Damm

author
#0 lore raw

PATCH 1/1 gpio: em: Add Device Tree support

From: Magnus Damm <[email protected]>

Update the Emma Mobile GPIO driver to add DT support.

The patch simply adds a two-cell xlate function and updates the probe code to allow configuration via DT using the "ngpios" property plus OF id in the same style as gpio-mvebu.c. The code is also adjusted to use postcore_initcall() to force early setup.

Signed-off-by Magnus Damm <[email protected]>

---

Written on top of:
[PATCH] gpio: em: Use irq_domain_add_simple() to fix runtime error

 drivers/gpio/gpio-em.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)
drivers/gpio/gpio-em.c +42 -3
@@ -231,10 +231,12 @@ static int em_gio_irq_domain_map(struct
 
 static struct irq_domain_ops em_gio_irq_domain_ops = {
 	.map	= em_gio_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
 };
 
 static int em_gio_probe(struct platform_device *pdev)
 {
+	struct gpio_em_config pdata_dt;
 	struct gpio_em_config *pdata = pdev->dev.platform_data;
 	struct em_gio_priv *p;
 	struct resource *io[2], *irq[2];

@@ -259,8 +261,8 @@ static int em_gio_probe(struct platform_
 	irq[0] = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	irq[1] = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
 
-	if (!io[0] || !io[1] || !irq[0] || !irq[1] || !pdata) {
-		dev_err(&pdev->dev, "missing IRQ, IOMEM or configuration\n");
+	if (!io[0] || !io[1] || !irq[0] || !irq[1]) {
+		dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
 		ret = -EINVAL;
 		goto err1;
 	}

@@ -279,6 +281,25 @@ static int em_gio_probe(struct platform_
 		goto err2;
 	}
 
+	if (!pdata) {
+		memset(&pdata_dt, 0, sizeof(pdata_dt));
+		pdata = &pdata_dt;
+
+		if (of_property_read_u32(pdev->dev.of_node, "ngpios",
+					 &pdata->number_of_pins)) {
+			dev_err(&pdev->dev, "Missing ngpios OF property\n");
+			ret = -EINVAL;
+			goto err3;
+		}
+
+		ret = of_alias_get_id(pdev->dev.of_node, "gpio");
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Couldn't get OF id\n");
+			goto err3;
+		}
+		pdata->gpio_base = ret * 32; /* 32 GPIOs per instance */
+	}
+
 	gpio_chip = &p->gpio_chip;
 	gpio_chip->direction_input = em_gio_direction_input;
 	gpio_chip->get = em_gio_get;

@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
 	return 0;
 }
 
+static const struct of_device_id em_gio_dt_ids[] = {
+	{ .compatible = "renesas,em-gio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+
 static struct platform_driver em_gio_device_driver = {
 	.probe		= em_gio_probe,
 	.remove		= em_gio_remove,
 	.driver		= {
 		.name	= "em_gio",
+		.of_match_table = em_gio_dt_ids,
+		.owner		= THIS_MODULE,
 	}
 };
 
-module_platform_driver(em_gio_device_driver);
+static int __init em_gio_init(void)
+{
+	return platform_driver_register(&em_gio_device_driver);
+}
+postcore_initcall(em_gio_init);
+
+static void __exit em_gio_exit(void)
+{
+	platform_driver_unregister(&em_gio_device_driver);
+}
+module_exit(em_gio_exit);
 
 MODULE_AUTHOR("Magnus Damm");
 MODULE_DESCRIPTION("Renesas Emma Mobile GIO Driver");
#1 lore raw

Hi Magnus,

Quoted 4 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the Emma Mobile GPIO driver to add DT support.

...

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 389

Quoted 4 lines
drivers/gpio/gpio-em.c +0 -0
@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
 	return 0;
 }

#ifdef CONFIG_OF here? No need to have extra aliases in modules if OF support is not enabled (or is entire ARM arch now enables device tree?).

Quoted 16 lines
drivers/gpio/gpio-em.c +8 -0
+static const struct of_device_id em_gio_dt_ids[] = {
+	{ .compatible = "renesas,em-gio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+
 static struct platform_driver em_gio_device_driver = {
 	.probe		= em_gio_probe,
 	.remove		= em_gio_remove,
 	.driver		= {
 		.name	= "em_gio",
+		.of_match_table = em_gio_dt_ids,
+		.owner		= THIS_MODULE,
 	}
 };
Signature · 1 line
Dmitry
author
#2 lore raw

Hi Dmitry,

Thanks for your feedback!

Quoted 18 lines · Dmitry Torokhov

Hi Magnus,

Quoted 4 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the Emma Mobile GPIO driver to add DT support.

...

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 389

Quoted 4 lines
drivers/gpio/gpio-em.c +0 -0
@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
      return 0;
 }

#ifdef CONFIG_OF here? No need to have extra aliases in modules if OF support is not enabled (or is entire ARM arch now enables device tree?).

Quoted 6 lines

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 395

Quoted 6 lines
drivers/gpio/gpio-em.c +6 -0
+static const struct of_device_id em_gio_dt_ids[] = {
+     { .compatible = "renesas,em-gio", },
+     {},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+

I suppose we could sprinkle a couple of #ifdefs across the code, but I have to say that I'm not that fond of #ifdefs in general. So if it was up to me only then I would aim at having exactly zero #ifdefs in my drivers at the expense of slightly larger binaries in some cases.

Are you interested in hacking on EMEV2 in general? I assume you're busy, but I could try to locate a developer board for you if you'd like.

Cheers,

/ magnus

#3 lore raw
Quoted 24 lines · Magnus Damm

Hi Dmitry,

Thanks for your feedback!

Quoted 18 lines · Dmitry Torokhov

Hi Magnus,

Quoted 4 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the Emma Mobile GPIO driver to add DT support.

...

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 389

Quoted 4 lines
drivers/gpio/gpio-em.c +0 -0
@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
      return 0;
 }

#ifdef CONFIG_OF here? No need to have extra aliases in modules if OF support is not enabled (or is entire ARM arch now enables device tree?).

Quoted 11 lines
Quoted 6 lines

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 395

Quoted 6 lines
drivers/gpio/gpio-em.c +6 -0
+static const struct of_device_id em_gio_dt_ids[] = {
+     { .compatible = "renesas,em-gio", },
+     {},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+

I suppose we could sprinkle a couple of #ifdefs across the code, but I have to say that I'm not that fond of #ifdefs in general. So if it was up to me only then I would aim at having exactly zero #ifdefs in my drivers at the expense of slightly larger binaries in some cases.

My take on this is as follows (if anyone cares):

* The driver in question is currently only useful in conjunction with the Emev2 SoC and the kzm9g board which uses that SoC. The current (and to date only) practice when booting that board and thus SoC with merged upstream code is to do so using DT.

* The inclusion of the code above does not appear to cause build-time breakage even if CONFIG_OF is not set.

So it seems to me that it is reasonable to leave the code as is without being guarded by an #ifdef.

#4 lore raw
Quoted 44 lines · Simon Horman
Quoted 24 lines · Magnus Damm

Hi Dmitry,

Thanks for your feedback!

Quoted 18 lines · Dmitry Torokhov

Hi Magnus,

Quoted 4 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the Emma Mobile GPIO driver to add DT support.

...

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 389

Quoted 4 lines
drivers/gpio/gpio-em.c +0 -0
@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
      return 0;
 }

#ifdef CONFIG_OF here? No need to have extra aliases in modules if OF support is not enabled (or is entire ARM arch now enables device tree?).

Quoted 11 lines
Quoted 6 lines

drivers/gpio/gpio-em.c static int em_gio_remove(struct platform · line 395

Quoted 6 lines
drivers/gpio/gpio-em.c +6 -0
+static const struct of_device_id em_gio_dt_ids[] = {
+     { .compatible = "renesas,em-gio", },
+     {},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+

I suppose we could sprinkle a couple of #ifdefs across the code, but I have to say that I'm not that fond of #ifdefs in general. So if it was up to me only then I would aim at having exactly zero #ifdefs in my drivers at the expense of slightly larger binaries in some cases.

My take on this is as follows (if anyone cares):

* The driver in question is currently only useful in conjunction with the Emev2 SoC and the kzm9g board which uses that SoC. The current (and to date only) practice when booting that board and thus SoC with merged upstream code is to do so using DT.

Quoted 5 lines

* The inclusion of the code above does not appear to cause build-time breakage even if CONFIG_OF is not set.

So it seems to me that it is reasonable to leave the code as is without being guarded by an #ifdef.

I completely agree. Applied for v3.10

g.

Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.