| Message ID | 50f7f506364f84effd5224677b21726fd2e511a4.1462372360.git.chunkeey@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 6, 2016 at 2:10 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 > are compatible with the existing gpio-mmio interface. The generic > library provides support for many memory-mapped GPIO controllers > that are found in various on-board FPGA and ASIC solutions that > are used to control board's switches, LEDs, chip-selects, > Ethernet/USB PHY power, etc. > > For setting GPIO's there are three configurations: > 1. single input/output register resource (named "dat"), > 2. set/clear pair (named "set" and "clr"), > 3. single output register resource and single input resource > ("set" and dat"). > > The configuration is detected by which resources are present. > For the single output register, this drives a 1 by setting a bit > and a zero by clearing a bit. For the set clr pair, this drives > a 1 by setting a bit in the set register and clears it by setting > a bit in the clear register. The configuration is detected by > which resources are present. > > For setting the GPIO direction, there are three configurations: > a. simple bidirectional GPIOs that requires no configuration. > b. an output direction register (named "dirout") > where a 1 bit indicates the GPIO is an output. > c. an input direction register (named "dirin") > where a 1 bit indicates the GPIO is an input. > > The first user for this binding is "wd,mbl-gpio". Some (mostly) style issues below. > @@ -569,6 +571,89 @@ 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; > + > + pdata->base = -1; + empty line > + /* If ngpio property is not specified, of_property_read_u32 > + * will return -EINVAL. In this case the number of GPIOs is > + * automatically determined by the register width. Any > + * other error of of_property_read_u32 is due bad data and > + * needs to be dealt with. Couple style issues: /* * First sentence starts here. func() are going with parens. */ > + */ > + 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_GPIO_OF(_name, _func) { .compatible = _name, .data = _func } > + > +static const struct of_device_id bgpio_of_match[] = { > + ADD_GPIO_OF("wd,mbl-gpio", bgpio_basic_mmio_parse_dt), > + { } > +}; > +#undef ADD_GPIO_OF > +MODULE_DEVICE_TABLE(of, bgpio_of_match); > + > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev, > + unsigned long *flags) > +{ > + const 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; (1) > + > + of_id = of_match_node(bgpio_of_match, node); > + if (!of_id) > + return NULL; (2) Why so? > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata), > + GFP_KERNEL); > + if (!pdata) > + return ERR_PTR(-ENOMEM); > + > + parse_dt = (const void *)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 */
Hi, On Fri, May 06, 2016 at 01:10:20PM +0200, Christian Lamparter wrote: > +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; > + > + pdata->base = -1; > + /* If ngpio property is not specified, of_property_read_u32 > + * will return -EINVAL. In this case the number of GPIOs is > + * automatically determined by the register width. Any > + * other error of of_property_read_u32 is due bad data and > + * needs to be dealt with. > + */ > + 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; > +} Other than no-output, none of these are in the wd,mbl-gpio binding. Please remove them for now. We can add them as/when other users of this binding appear and begin to need them. Thanks, Mark.
On Friday, May 06, 2016 02:44:14 PM Andy Shevchenko wrote: > > + /* If ngpio property is not specified, of_property_read_u32 > > + * will return -EINVAL. In this case the number of GPIOs is > > + * automatically determined by the register width. Any > > + * other error of of_property_read_u32 is due bad data and > > + * needs to be dealt with. > > Couple style issues: > /* > * First sentence starts here. func() are going with parens. > */ Ack, I'm used to drivers/net. I have to remove the ngpio in a future version, so the comment is removed as well. > > + > > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev, > > + unsigned long *flags) > > +{ > > + const 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; > > (1) > > > + > > + of_id = of_match_node(bgpio_of_match, node); > > + if (!of_id) > > + return NULL; > > (2) > > Why so? This is because of existing arch code in /arch/arm/mach-clps711x/board-p720t.c. You remember there's a driver with a device-tree binding "cirrus,clps711x-gpio" for it. But the current kernel arch code still registers a platform_device and it doesn't look like it has any device tree support. So this is necessary for those partially converted archs to co-exist with the driver. I'll update the -ENODEV to -EINVAL. > > + > > + pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata), > > + GFP_KERNEL); > > + if (!pdata) > > + return ERR_PTR(-ENOMEM); > > + > > + parse_dt = (const void *)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 */
diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index 6c1cb3b..1cfb70a 100644 --- a/drivers/gpio/gpio-mmio.c +++ b/drivers/gpio/gpio-mmio.c @@ -61,6 +61,8 @@ 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> static void bgpio_write8(void __iomem *reg, unsigned long data) { @@ -569,6 +571,89 @@ 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; + + pdata->base = -1; + /* If ngpio property is not specified, of_property_read_u32 + * will return -EINVAL. In this case the number of GPIOs is + * automatically determined by the register width. Any + * other error of of_property_read_u32 is due bad data and + * needs to be dealt with. + */ + 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_GPIO_OF(_name, _func) { .compatible = _name, .data = _func } + +static const struct of_device_id bgpio_of_match[] = { + ADD_GPIO_OF("wd,mbl-gpio", bgpio_basic_mmio_parse_dt), + { } +}; +#undef ADD_GPIO_OF +MODULE_DEVICE_TABLE(of, bgpio_of_match); + +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev, + unsigned long *flags) +{ + const 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 = (const void *)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 +664,19 @@ 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); + + 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 +740,7 @@ MODULE_DEVICE_TABLE(platform, bgpio_id_table); static struct platform_driver bgpio_driver = { .driver = { .name = "basic-mmio-gpio", + .of_match_table = of_match_ptr(bgpio_of_match), }, .id_table = bgpio_id_table, .probe = bgpio_pdev_probe,