| Message ID | a5d6bb7bb73ff9396c9e8053a8e8e0975b857934.1461806071.git.chunkeey@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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,