diff mbox

[v7,2/3] gpio: mmio: add DT support for memory-mapped GPIOs

Message ID 50f7f506364f84effd5224677b21726fd2e511a4.1462372360.git.chunkeey@googlemail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christian Lamparter May 6, 2016, 11:10 a.m. UTC
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".

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/gpio-mmio.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko May 6, 2016, 11:44 a.m. UTC | #1
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 */
Mark Rutland May 6, 2016, 11:50 a.m. UTC | #2
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.
Christian Lamparter May 6, 2016, 12:22 p.m. UTC | #3
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 mbox

Patch

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,