Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1389915 > unrolled thread

[RFC v4 3/8] gpio: generic: add DT support for generic memory-mapped GPIOs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC v4 3/8] gpio: generic: add DT support for generic memory-mapped GPIOs Christian Lamparter <chunkeey@googlemail.com> - 2016-04-28 11:10 +0200
    Re: [RFC v4 3/8] gpio: generic: add DT support for generic  memory-mapped GPIOs Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-04-28 12:00 +0200
      Re: [RFC v4 3/8] gpio: generic: add DT support for generic memory-mapped GPIOs Christian Lamparter <chunkeey@googlemail.com> - 2016-04-29 01:50 +0200

#1389915 — [RFC v4 3/8] gpio: generic: add DT support for generic memory-mapped GPIOs

From: Álvaro Fernández Rojas <noltari@gmail.com>

This patch adds support for defining memory-mapped GPIOs
which provide a compatible interface for the existing
generic-gpio driver.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/Kconfig            |  3 ++
 drivers/gpio/gpio-mmio-compat.h | 17 ++++++++
 drivers/gpio/gpio-mmio.c        | 95 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 113 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpio/gpio-mmio-compat.h

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f73f26b..d1f124e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -212,6 +212,9 @@ config GPIO_GENERIC_PLATFORM
 	help
 	  Say yes here to support basic platform_device memory-mapped GPIO controllers.
 
+if GPIO_GENERIC_PLATFORM
+endif
+
 config GPIO_GRGPIO
 	tristate "Aeroflex Gaisler GRGPIO support"
 	depends on OF
diff --git a/drivers/gpio/gpio-mmio-compat.h b/drivers/gpio/gpio-mmio-compat.h
new file mode 100644
index 0000000..73c48bc
--- /dev/null
+++ b/drivers/gpio/gpio-mmio-compat.h
@@ -0,0 +1,17 @@
+#ifndef GPIO_MMIO_COMPAT_H
+#define GPIO_MMIO_COMPAT_H
+
+#include <linux/ioport.h>
+
+#define ADD(_name, _func) { .compatible = _name, .data = _func }
+
+#undef ADD
+
+static inline void set_resource_address(struct resource *res,
+					resource_size_t start,
+					resource_size_t len)
+{
+	res->start = start;
+	res->end = start + len - 1;
+}
+#endif /* GPIO_MMIO_COMPAT_H */
diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 6c1cb3b..4de9846 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -61,6 +61,9 @@ o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
 #include <linux/bitops.h>
 #include <linux/platform_device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include "gpio-mmio-compat.h"
 
 static void bgpio_write8(void __iomem *reg, unsigned long data)
 {
@@ -569,6 +572,83 @@ static void __iomem *bgpio_map(struct platform_device *pdev,
 	return devm_ioremap_resource(&pdev->dev, r);
 }
 
+#ifdef CONFIG_OF
+static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
+				     struct bgpio_pdata *pdata,
+				     unsigned long *flags)
+{
+	struct device *dev = &pdev->dev;
+	int err;
+
+	err = of_property_read_u32(dev->of_node, "ngpio", &pdata->ngpio);
+	if (err && err != -EINVAL)
+		return err;
+
+	if (of_device_is_big_endian(dev->of_node))
+		*flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+
+	if (of_property_read_bool(dev->of_node, "unreadable-reg-set"))
+		*flags |= BGPIOF_UNREADABLE_REG_SET;
+
+	if (of_property_read_bool(dev->of_node, "unreadable-reg-dir"))
+		*flags |= BGPIOF_UNREADABLE_REG_DIR;
+
+	if (of_property_read_bool(dev->of_node, "big-endian-byte-order"))
+		*flags |= BGPIOF_BIG_ENDIAN;
+
+	if (of_property_read_bool(dev->of_node, "read-output-reg-set"))
+		*flags |= BGPIOF_READ_OUTPUT_REG_SET;
+
+	if (of_property_read_bool(dev->of_node, "no-output"))
+		*flags |= BGPIOF_NO_OUTPUT;
+	return 0;
+}
+
+#define ADD(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD("linux,gpio-mmio", bgpio_basic_mmio_parse_dt),
+
+	{ }
+};
+#undef ADD
+MODULE_DEVICE_TABLE(of, bgpio_of_match);
+
+static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
+					  unsigned long *flags)
+{
+	int (*parse_dt)(struct platform_device *,
+			struct bgpio_pdata *, unsigned long *);
+	const struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *of_id;
+	struct bgpio_pdata *pdata;
+	int err = -ENODEV;
+
+	of_id = of_match_node(bgpio_of_match, node);
+	if (!of_id)
+		return NULL;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	parse_dt = of_id->data;
+	if (parse_dt)
+		err = parse_dt(pdev, pdata, flags);
+	if (err)
+		return ERR_PTR(err);
+
+	return pdata;
+}
+#else
+static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
+					  unsigned long *flags)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int bgpio_pdev_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -579,10 +659,18 @@ static int bgpio_pdev_probe(struct platform_device *pdev)
 	void __iomem *dirout;
 	void __iomem *dirin;
 	unsigned long sz;
-	unsigned long flags = pdev->id_entry->driver_data;
+	unsigned long flags = 0;
 	int err;
 	struct gpio_chip *gc;
-	struct bgpio_pdata *pdata = dev_get_platdata(dev);
+	struct bgpio_pdata *pdata;
+
+	pdata = bgpio_parse_dt(pdev, &flags);
+	if (IS_ERR(pdata)) {
+		return PTR_ERR(pdata);
+	} else if (!pdata) {
+		pdata = dev_get_platdata(dev);
+		flags = pdev->id_entry->driver_data;
+	}
 
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
 	if (!r)
@@ -646,6 +734,9 @@ MODULE_DEVICE_TABLE(platform, bgpio_id_table);
 static struct platform_driver bgpio_driver = {
 	.driver = {
 		.name = "basic-mmio-gpio",
+#ifdef CONFIG_OF
+		.of_match_table = of_match_ptr(bgpio_of_match),
+#endif
 	},
 	.id_table = bgpio_id_table,
 	.probe = bgpio_pdev_probe,
-- 
2.8.1

[toc] | [next] | [standalone]

#1389950 — Re: [RFC v4 3/8] gpio: generic: add DT support for generic memory-mapped GPIOs

On Thu, Apr 28, 2016 at 12:05 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> From: Álvaro Fernández Rojas <noltari@gmail.com>
>
> This patch adds support for defining memory-mapped GPIOs
> which provide a compatible interface for the existing
> generic-gpio driver.

> +static inline void set_resource_address(struct resource *res,
> +                                       resource_size_t start,
> +                                       resource_size_t len)
> +{
> +       res->start = start;
> +       res->end = start + len - 1;
> +}

It might make sense to put this in the generic (resource related, e.g.
ioport.h) header. There are plenty users of such already and who knows
how many will come.

>  #include <linux/bitops.h>
>  #include <linux/platform_device.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>

Perhaps + empty line?

> +#include "gpio-mmio-compat.h"

[]

> +#ifdef CONFIG_OF
> +static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
> +                                    struct bgpio_pdata *pdata,
> +                                    unsigned long *flags)
> +{
> +       struct device *dev = &pdev->dev;
> +       int err;
> +
> +       err = of_property_read_u32(dev->of_node, "ngpio", &pdata->ngpio);
> +       if (err && err != -EINVAL)
> +               return err;

I think check for -EINVAL needs explanation.

> +}

[]

> +       pdata = bgpio_parse_dt(pdev, &flags);
> +       if (IS_ERR(pdata)) {
> +               return PTR_ERR(pdata);

> +       } else if (!pdata) {

Redundant else

> +               pdata = dev_get_platdata(dev);
> +               flags = pdev->id_entry->driver_data;
> +       }
>
>         r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
>         if (!r)
> @@ -646,6 +734,9 @@ MODULE_DEVICE_TABLE(platform, bgpio_id_table);
>  static struct platform_driver bgpio_driver = {
>         .driver = {
>                 .name = "basic-mmio-gpio",

> +#ifdef CONFIG_OF

Doesn't of_match_ptr do this for you?


> +               .of_match_table = of_match_ptr(bgpio_of_match),
> +#endif

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1390648

On Thursday, April 28, 2016 12:58:03 PM Andy Shevchenko wrote:
> On Thu, Apr 28, 2016 at 12:05 PM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> > From: Álvaro Fernández Rojas <noltari@gmail.com>
> >
> > This patch adds support for defining memory-mapped GPIOs
> > which provide a compatible interface for the existing
> > generic-gpio driver.

Thanks for your comments! I've prepared a new series which
I'm going to post tomorrow. I've incorporated most of the
remarks, but there's something I have to say about: 

> > +static inline void set_resource_address(struct resource *res,
> > +                                       resource_size_t start,
> > +                                       resource_size_t len)
> > +{
> > +       res->start = start;
> > +       res->end = start + len - 1;
> > +}
> 
> It might make sense to put this in the generic (resource related, e.g.
> ioport.h) header. There are plenty users of such already and who knows
> how many will come.

I looked around and found plenty of code in drivers/
alone doing the same song and dance around it:

acpi/pci_root.c acpi/resource.c
bus/mvebu-mbus.c
i2c/busses/i2c-i801.c
irqchip/irq-mips-gic.c
memory/omap-gpmc.c
mfd/janz-cmodio.c mfd/lpc_ich.c mfd/sm501.c mtd/devices/ms02-nv.c
nvdimm/namespace_devs.c
of/address.c
parisc/ccio-dma.c parisc/dino.c parisc/lba_pci.c
pci/hotplug/ibmphp_res.c pci/bus.c pci/iov.c pci/setup-res.c
pci/setup-bus.c pci/hotplug/ibmphp_res.c
pcmcia/rsrc_mgr.c pcmcia/pcmcia_resource.c
pnp/manager.c
platform/x86/intel_pmc_ipc.c
pinctrl/sh-pfc/core.c
etc...

I think arch/ will have a few more. If anything this will require
help from coccinelle and more stuff. For now I'll convert the code
to do the same thing as everybody else. And after the "linux,gpio-mmio"
has been successfully mainlined I can worry about how to write the
perfect set_resource_address or set_resource_range (needs to handle
over- and underflows, etc...) and setup the automatic tools to convert
the whole tree. So, unless someone else beats me to this, this would
be my plan.

Regards,
Christian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web