[PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Bernd Edlinger]

This is an internal 32-bit input and 32-bit output port to the FPGA logic.

Instantiate this in the device tree as:

   gpio3: gpio@ff706010 {
    #address-cells = <1>;
    #size-cells = <0>;
    compatible = "altr,fpgamgr-gpio";
    reg = <0xff706010 0x8>;
    status = "okay";

    portd: gpio-controller@0 {
     compatible = "altr,fpgamgr-gpio-output";
     gpio-controller;
     #gpio-cells = <2>;
     reg = <0>;
    };

    porte: gpio-controller@1 {
     compatible = "altr,fpgamgr-gpio-input";
     gpio-controller;
     #gpio-cells = <2>;
     reg = <1>;
    };
   };

Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
  drivers/gpio/Kconfig        |   6 ++
  drivers/gpio/Makefile       |   1 +
  drivers/gpio/gpio-fpgamgr.c | 256 
++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 263 insertions(+)
  create mode 100644 drivers/gpio/gpio-fpgamgr.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3388d54..e0c5a35 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -192,6 +192,12 @@ config GPIO_EXAR
  	  Selecting this option will enable handling of GPIO pins present
  	  on Exar XR17V352/354/358 chips.

+config GPIO_FPGAMGR
+	tristate "Altera FPGAMGR GPIO"
+	depends on OF_GPIO
+	help
+	  Say yes here to support the Altera FPGAMGR GPIO device.
+
  config GPIO_GE_FPGA
  	bool "GE FPGA based GPIO"
  	depends on GE_FPGA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index aeb70e9d..88a32ab 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_GPIO_ETRAXFS)	+= gpio-etraxfs.o
  obj-$(CONFIG_GPIO_EXAR)		+= gpio-exar.o
  obj-$(CONFIG_GPIO_F7188X)	+= gpio-f7188x.o
  obj-$(CONFIG_GPIO_FTGPIO010)	+= gpio-ftgpio010.o
+obj-$(CONFIG_GPIO_FPGAMGR)	+= gpio-fpgamgr.o
  obj-$(CONFIG_GPIO_GE_FPGA)	+= gpio-ge.o
  obj-$(CONFIG_GPIO_GPIO_MM)	+= gpio-gpio-mm.o
  obj-$(CONFIG_GPIO_GRGPIO)	+= gpio-grgpio.o
diff --git a/drivers/gpio/gpio-fpgamgr.c b/drivers/gpio/gpio-fpgamgr.c
new file mode 100644
index 0000000..8aa86e7
--- /dev/null
+++ b/drivers/gpio/gpio-fpgamgr.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (c) 2015 Softing Industrial Automation GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include <linux/gpio/driver.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+
+struct fpgamgr_port_property {
+	struct device_node		*node;
+	const char			*name;
+	unsigned int			idx;
+};
+
+struct fpgamgr_platform_data {
+	struct fpgamgr_port_property	*properties;
+	unsigned int			nports;
+};
+
+struct fpgamgr_gpio_port {
+	struct gpio_chip		bgc;
+	bool				is_registered;
+	struct fpgamgr_gpio		*gpio;
+	unsigned int			idx;
+};
+
+struct fpgamgr_gpio {
+	struct	device			*dev;
+	void __iomem			*regs;
+	struct fpgamgr_gpio_port	*ports;
+	unsigned int			nr_ports;
+};
+
+static int fpgamgr_gpio_add_port(struct fpgamgr_gpio *gpio,
+				 struct fpgamgr_port_property *pp,
+				 unsigned int offs)
+{
+	struct fpgamgr_gpio_port *port;
+	void __iomem *dat;
+	int err;
+
+	port = &gpio->ports[offs];
+	port->gpio = gpio;
+	port->idx = pp->idx;
+
+	dat = gpio->regs + (pp->idx * 4);
+
+	err = bgpio_init(&port->bgc, gpio->dev, 4, dat, NULL, NULL,
+			 NULL, NULL, 0);
+	if (err) {
+		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
+			pp->name);
+		return err;
+	}
+
+#ifdef CONFIG_OF_GPIO
+	port->bgc.of_node = pp->node;
+#endif
+
+	err = gpiochip_add(&port->bgc);
+	if (err)
+		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
+			pp->name);
+	else
+		port->is_registered = true;
+
+	return err;
+}
+
+static void fpgamgr_gpio_unregister(struct fpgamgr_gpio *gpio)
+{
+	unsigned int m;
+
+	for (m = 0; m < gpio->nr_ports; ++m)
+		if (gpio->ports[m].is_registered)
+			gpiochip_remove(&gpio->ports[m].bgc);
+}
+
+static struct fpgamgr_platform_data *
+fpgamgr_gpio_get_pdata_of(struct device *dev)
+{
+	struct device_node *node, *port_np;
+	struct fpgamgr_platform_data *pdata;
+	struct fpgamgr_port_property *pp;
+	int nports;
+	int i;
+
+	node = dev->of_node;
+	if (!IS_ENABLED(CONFIG_OF_GPIO) || !node)
+		return ERR_PTR(-ENODEV);
+
+	nports = of_get_child_count(node);
+	if (nports == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->properties = kcalloc(nports, sizeof(*pp), GFP_KERNEL);
+	if (!pdata->properties) {
+		kfree(pdata);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pdata->nports = nports;
+
+	i = 0;
+	for_each_child_of_node(node, port_np) {
+		pp = &pdata->properties[i++];
+		pp->node = port_np;
+
+		if (of_property_read_u32(port_np, "reg", &pp->idx) ||
+		    pp->idx > 1) {
+			dev_err(dev, "missing/invalid port index for %s\n",
+				port_np->full_name);
+			kfree(pdata->properties);
+			kfree(pdata);
+			return ERR_PTR(-EINVAL);
+		}
+
+		pp->name = port_np->full_name;
+	}
+
+	return pdata;
+}
+
+static inline void fpgamgr_free_pdata_of(struct fpgamgr_platform_data 
*pdata)
+{
+	if (!IS_ENABLED(CONFIG_OF_GPIO) || !pdata)
+		return;
+
+	kfree(pdata->properties);
+	kfree(pdata);
+}
+
+static int fpgamgr_gpio_probe(struct platform_device *pdev)
+{
+	unsigned int i;
+	struct resource *res;
+	struct fpgamgr_gpio *gpio;
+	int err;
+	struct device *dev = &pdev->dev;
+	struct fpgamgr_platform_data *pdata = dev_get_platdata(dev);
+	bool is_pdata_alloc = !pdata;
+
+	if (is_pdata_alloc) {
+		pdata = fpgamgr_gpio_get_pdata_of(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	if (!pdata->nports) {
+		err = -ENODEV;
+		goto out_err;
+	}
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+	gpio->dev = &pdev->dev;
+	gpio->nr_ports = pdata->nports;
+
+	gpio->ports = devm_kcalloc(&pdev->dev, gpio->nr_ports,
+				   sizeof(*gpio->ports), GFP_KERNEL);
+	if (!gpio->ports) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	gpio->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(gpio->regs)) {
+		err = PTR_ERR(gpio->regs);
+		goto out_err;
+	}
+
+	for (i = 0; i < gpio->nr_ports; i++) {
+		err = fpgamgr_gpio_add_port(gpio, &pdata->properties[i], i);
+		if (err)
+			goto out_unregister;
+	}
+	platform_set_drvdata(pdev, gpio);
+
+	goto out_err;
+
+out_unregister:
+	fpgamgr_gpio_unregister(gpio);
+
+out_err:
+	if (is_pdata_alloc)
+		fpgamgr_free_pdata_of(pdata);
+
+	return err;
+}
+
+static int fpgamgr_gpio_remove(struct platform_device *pdev)
+{
+	struct fpgamgr_gpio *gpio = platform_get_drvdata(pdev);
+
+	fpgamgr_gpio_unregister(gpio);
+
+	return 0;
+}
+
+static const struct of_device_id fpgamgr_of_match[] = {
+	{ .compatible = "altr,fpgamgr-gpio" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fpgamgr_of_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int fpgamgr_gpio_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int fpgamgr_gpio_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(fpgamgr_gpio_pm_ops, fpgamgr_gpio_suspend,
+			 fpgamgr_gpio_resume);
+
+static struct platform_driver fpgamgr_gpio_driver = {
+	.driver		= {
+		.name	= "gpio-fpgamgr",
+		.owner	= THIS_MODULE,
+		.pm	= &fpgamgr_gpio_pm_ops,
+		.of_match_table = of_match_ptr(fpgamgr_of_match),
+	},
+	.probe		= fpgamgr_gpio_probe,
+	.remove		= fpgamgr_gpio_remove,
+};
+
+module_platform_driver(fpgamgr_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bernd Edlinger");
+MODULE_DESCRIPTION("Altera fpgamgr GPIO driver");
-- 
2.7.4

Re: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Linus Walleij]

On Fri, Sep 22, 2017 at 8:41 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:

> This is an internal 32-bit input and 32-bit output port to the FPGA logic.

What does "internal" mean in this context? On the chip?

I hope it still qualifies to be called "general purpose input/output".

Otherwise it's not GPIO...

> Instantiate this in the device tree as:
>
>    gpio3: gpio@ff706010 {
>     #address-cells = <1>;
>     #size-cells = <0>;
>     compatible = "altr,fpgamgr-gpio";
>     reg = <0xff706010 0x8>;
>     status = "okay";
>
>     portd: gpio-controller@0 {
>      compatible = "altr,fpgamgr-gpio-output";
>      gpio-controller;
>      #gpio-cells = <2>;
>      reg = <0>;
>     };
>
>     porte: gpio-controller@1 {
>      compatible = "altr,fpgamgr-gpio-input";
>      gpio-controller;
>      #gpio-cells = <2>;
>      reg = <1>;
>     };

So one port is output-only and one is input-only?

Fair enough.

> +config GPIO_FPGAMGR

Call it GPIO_ALTERA_FPGAMGR or something.

"FPGAMGR" is too generic. There must be a plethora
of FPGA managers in the world.

> +obj-$(CONFIG_GPIO_FPGAMGR)     += gpio-fpgamgr.o

So call that file gpio-altera-fpgamgr.o as well.

> +/*

Add a blurb here at the top describing the driver and what it is for.

> + * Copyright (c) 2015 Softing Industrial Automation GmbH

> +static int fpgamgr_gpio_add_port(struct fpgamgr_gpio *gpio,
> +                                struct fpgamgr_port_property *pp,
> +                                unsigned int offs)
> +{
> +       struct fpgamgr_gpio_port *port;
> +       void __iomem *dat;
> +       int err;
> +
> +       port = &gpio->ports[offs];
> +       port->gpio = gpio;
> +       port->idx = pp->idx;
> +
> +       dat = gpio->regs + (pp->idx * 4);
> +
> +       err = bgpio_init(&port->bgc, gpio->dev, 4, dat, NULL, NULL,
> +                        NULL, NULL, 0);

Nice reuse of MMIO GPIO.

> +#ifdef CONFIG_OF_GPIO
> +       port->bgc.of_node = pp->node;
> +#endif

Drop the #ifdef.

The Kconfig already depends on OF_GPIO so this is always true.

The concept of "port" is the same as "bank".

Please read Thierry's proposed changes that I will merge as soon as
he respins them, he creates a "bank" abstraction for gpiolib:
https://marc.info/?l=linux-gpio&m=150429237121695&w=2

I strongly feel you should use this infrastructure, even if Thierry's work
is much centered around being able to have per-bank IRQs.

> +static void fpgamgr_gpio_unregister(struct fpgamgr_gpio *gpio)
> +{
> +       unsigned int m;
> +
> +       for (m = 0; m < gpio->nr_ports; ++m)
> +               if (gpio->ports[m].is_registered)
> +                       gpiochip_remove(&gpio->ports[m].bgc);
> +}

Why can't you simply use devm_gpiochip_add_data() and get rid
of the unregister business? (data can be NULL)

> +static struct fpgamgr_platform_data *
> +fpgamgr_gpio_get_pdata_of(struct device *dev)
> +{
> +       struct device_node *node, *port_np;

Rename "node" to just "np" (node pointer) this is the convention
we usually employ.

Just declare it like this:

struct device_node *np = dev->of_node;

So you don't need to assign it later.

> +       struct fpgamgr_platform_data *pdata;
> +       struct fpgamgr_port_property *pp;
> +       int nports;
> +       int i;
> +
> +       node = dev->of_node;
> +       if (!IS_ENABLED(CONFIG_OF_GPIO) || !node)
> +               return ERR_PTR(-ENODEV);

Drop this, as stated above it is always enabled when compiling
and probing this code.

> +       nports = of_get_child_count(node);
> +       if (nports == 0)
> +               return ERR_PTR(-ENODEV);
> +
> +       pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +       if (!pdata)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pdata->properties = kcalloc(nports, sizeof(*pp), GFP_KERNEL);
> +       if (!pdata->properties) {
> +               kfree(pdata);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       pdata->nports = nports;

Again important to use Thierry's infrastructure for ports/banks.

> +static inline void fpgamgr_free_pdata_of(struct fpgamgr_platform_data
> *pdata)
> +{
> +       if (!IS_ENABLED(CONFIG_OF_GPIO) || !pdata)
> +               return;

Drop that.

> +static int fpgamgr_gpio_probe(struct platform_device *pdev)
> +{

Since you use the struct device * pointer a lot here, introduce a local
variable like this:


struct device *dev = &pdev->dev;

Then substitute &pdev->dev for just dev below. Make it so much
easier to read.

> +       unsigned int i;
> +       struct resource *res;
> +       struct fpgamgr_gpio *gpio;

gpio is a bit ambigous here don't you think?

At least call it "fgpio" or something.

> +static const struct of_device_id fpgamgr_of_match[] = {
> +       { .compatible = "altr,fpgamgr-gpio" },
> +       { /* Sentinel */ }
> +};

Are these device tree bindings already merged? Else they need
a separate patch with Cc to devicetree@vger.kernel.org.

> +#ifdef CONFIG_PM_SLEEP
> +static int fpgamgr_gpio_suspend(struct device *dev)
> +{
> +       return 0;
> +}
> +
> +static int fpgamgr_gpio_resume(struct device *dev)
> +{
> +       return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(fpgamgr_gpio_pm_ops, fpgamgr_gpio_suspend,
> +                        fpgamgr_gpio_resume);

Either implement the suspend resume for real or drop this entire thing.
Just skeleton functions are not going to help anyone.

> +static struct platform_driver fpgamgr_gpio_driver = {
> +       .driver         = {
> +               .name   = "gpio-fpgamgr",

gpio-altera-fpgamgr

> +               .owner  = THIS_MODULE,
> +               .pm     = &fpgamgr_gpio_pm_ops,

Drop that too unless you implement it.

Yours,
Linus Walleij

Re: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Bernd Edlinger]

On 09/27/17 02:20, Linus Walleij wrote:
> On Fri, Sep 22, 2017 at 8:41 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
> 
>> This is an internal 32-bit input and 32-bit output port to the FPGA logic.
> 
> What does "internal" mean in this context? On the chip?
> 

Yes, these are simple interconnects between the two major parts on the
chip.  One is called HPS and it consists of two ARM cores.
And the other is a large programmable logic area called FPGA Fabric.

> I hope it still qualifies to be called "general purpose input/output".
> 
> Otherwise it's not GPIO...
> 

Absolutely it looks and feels like a general-purpose I/O facility.
It can be used to control arbitrary logic functions.

>> Instantiate this in the device tree as:
>>
>>     gpio3: gpio@ff706010 {
>>      #address-cells = <1>;
>>      #size-cells = <0>;
>>      compatible = "altr,fpgamgr-gpio";
>>      reg = <0xff706010 0x8>;
>>      status = "okay";
>>
>>      portd: gpio-controller@0 {
>>       compatible = "altr,fpgamgr-gpio-output";
>>       gpio-controller;
>>       #gpio-cells = <2>;
>>       reg = <0>;
>>      };
>>
>>      porte: gpio-controller@1 {
>>       compatible = "altr,fpgamgr-gpio-input";
>>       gpio-controller;
>>       #gpio-cells = <2>;
>>       reg = <1>;
>>      };
> 
> So one port is output-only and one is input-only?
> 
> Fair enough.
> 
>> +config GPIO_FPGAMGR
> 
> Call it GPIO_ALTERA_FPGAMGR or something.
> 

Done.

> "FPGAMGR" is too generic. There must be a plethora
> of FPGA managers in the world.
> 
>> +obj-$(CONFIG_GPIO_FPGAMGR)     += gpio-fpgamgr.o
> 
> So call that file gpio-altera-fpgamgr.o as well.
> 

Done.

>> +/*
> 
> Add a blurb here at the top describing the driver and what it is for.
> 

Done.

>> + * Copyright (c) 2015 Softing Industrial Automation GmbH
> 
>> +static int fpgamgr_gpio_add_port(struct fpgamgr_gpio *gpio,
>> +                                struct fpgamgr_port_property *pp,
>> +                                unsigned int offs)
>> +{
>> +       struct fpgamgr_gpio_port *port;
>> +       void __iomem *dat;
>> +       int err;
>> +
>> +       port = &gpio->ports[offs];
>> +       port->gpio = gpio;
>> +       port->idx = pp->idx;
>> +
>> +       dat = gpio->regs + (pp->idx * 4);
>> +
>> +       err = bgpio_init(&port->bgc, gpio->dev, 4, dat, NULL, NULL,
>> +                        NULL, NULL, 0);
> 
> Nice reuse of MMIO GPIO.
> 

Thanks.
Though I must admit, that I have copied most of this code verbatim
from gpio-dwapb.c ;)

>> +#ifdef CONFIG_OF_GPIO
>> +       port->bgc.of_node = pp->node;
>> +#endif
> 
> Drop the #ifdef.
> 

Done.

> The Kconfig already depends on OF_GPIO so this is always true.
> 
> The concept of "port" is the same as "bank".
> 
> Please read Thierry's proposed changes that I will merge as soon as
> he respins them, he creates a "bank" abstraction for gpiolib:
> https://marc.info/?l=linux-gpio&m=150429237121695&w=2
> 
> I strongly feel you should use this infrastructure, even if Thierry's work
> is much centered around being able to have per-bank IRQs.
> 

I hope that does not mean both ports have to share the same
interrupt spin lock.  While it is necessary to synchronize write
accesses to the same port, this driver has no need to synchronize
with the other port.

Interesting point is that this patch does not touch gpio-dwapb.c
at all.  Although gpio-dwapb.c has even irq support.

>> +static void fpgamgr_gpio_unregister(struct fpgamgr_gpio *gpio)
>> +{
>> +       unsigned int m;
>> +
>> +       for (m = 0; m < gpio->nr_ports; ++m)
>> +               if (gpio->ports[m].is_registered)
>> +                       gpiochip_remove(&gpio->ports[m].bgc);
>> +}
> 
> Why can't you simply use devm_gpiochip_add_data() and get rid
> of the unregister business? (data can be NULL)
> 

Done.  Note, that funny unregister code is originally from gpio-dwapb.c,
I have probably never tested the unregister code path at all.

>> +static struct fpgamgr_platform_data *
>> +fpgamgr_gpio_get_pdata_of(struct device *dev)
>> +{
>> +       struct device_node *node, *port_np;
> 
> Rename "node" to just "np" (node pointer) this is the convention
> we usually employ.
> 
> Just declare it like this:
> 
> struct device_node *np = dev->of_node;
> 
> So you don't need to assign it later.
> 

Done.

>> +       struct fpgamgr_platform_data *pdata;
>> +       struct fpgamgr_port_property *pp;
>> +       int nports;
>> +       int i;
>> +
>> +       node = dev->of_node;
>> +       if (!IS_ENABLED(CONFIG_OF_GPIO) || !node)
>> +               return ERR_PTR(-ENODEV);
> 
> Drop this, as stated above it is always enabled when compiling
> and probing this code.
> 

Done.

>> +       nports = of_get_child_count(node);
>> +       if (nports == 0)
>> +               return ERR_PTR(-ENODEV);
>> +
>> +       pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
>> +       if (!pdata)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pdata->properties = kcalloc(nports, sizeof(*pp), GFP_KERNEL);
>> +       if (!pdata->properties) {
>> +               kfree(pdata);
>> +               return ERR_PTR(-ENOMEM);
>> +       }
>> +
>> +       pdata->nports = nports;
> 
> Again important to use Thierry's infrastructure for ports/banks.
> 

I do not yet fully understood what that means for this driver.
However I would like to do that as a separate patch, in order to allow
back-ports of this driver.

>> +static inline void fpgamgr_free_pdata_of(struct fpgamgr_platform_data
>> *pdata)
>> +{
>> +       if (!IS_ENABLED(CONFIG_OF_GPIO) || !pdata)
>> +               return;
> 
> Drop that.
> 

Done.

>> +static int fpgamgr_gpio_probe(struct platform_device *pdev)
>> +{
> 
> Since you use the struct device * pointer a lot here, introduce a local
> variable like this:
> 
> 
> struct device *dev = &pdev->dev;
> 
> Then substitute &pdev->dev for just dev below. Make it so much
> easier to read.
> 

Done.

>> +       unsigned int i;
>> +       struct resource *res;
>> +       struct fpgamgr_gpio *gpio;
> 
> gpio is a bit ambigous here don't you think?
> 
> At least call it "fgpio" or something.
> 

Done.

>> +static const struct of_device_id fpgamgr_of_match[] = {
>> +       { .compatible = "altr,fpgamgr-gpio" },
>> +       { /* Sentinel */ }
>> +};
> 
> Are these device tree bindings already merged? Else they need
> a separate patch with Cc to devicetree@vger.kernel.org.
> 

You mean a text file under Documentation/devicetree/bindings/gpio
with a few words how the device tree has to look like?

I have not yet done that, but I will prepare a separate patch.

>> +#ifdef CONFIG_PM_SLEEP
>> +static int fpgamgr_gpio_suspend(struct device *dev)
>> +{
>> +       return 0;
>> +}
>> +
>> +static int fpgamgr_gpio_resume(struct device *dev)
>> +{
>> +       return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(fpgamgr_gpio_pm_ops, fpgamgr_gpio_suspend,
>> +                        fpgamgr_gpio_resume);
> 
> Either implement the suspend resume for real or drop this entire thing.
> Just skeleton functions are not going to help anyone.
> 

Dropped.

>> +static struct platform_driver fpgamgr_gpio_driver = {
>> +       .driver         = {
>> +               .name   = "gpio-fpgamgr",
> 
> gpio-altera-fpgamgr
> 

Done.

>> +               .owner  = THIS_MODULE,
>> +               .pm     = &fpgamgr_gpio_pm_ops,
> 
> Drop that too unless you implement it.
> 

Done.

Thanks
Bernd.

>From 223be945bf0a7d7d4bc979eccd77075b0a447ece Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Thu, 21 Sep 2017 15:50:36 +0200
Subject: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O.
 This is an internal 32-bit input and 32-bit output port to the FPGA logic.

Instantiate this in the device tree as:

  gpio3: gpio@ff706010 {
   #address-cells = <1>;
   #size-cells = <0>;
   compatible = "altr,fpgamgr-gpio";
   reg = <0xff706010 0x8>;
   status = "okay";

   portd: gpio-controller@0 {
    compatible = "altr,fpgamgr-gpio-output";
    gpio-controller;
    #gpio-cells = <2>;
    reg = <0>;
   };

   porte: gpio-controller@1 {
    compatible = "altr,fpgamgr-gpio-input";
    gpio-controller;
    #gpio-cells = <2>;
    reg = <1>;
   };
  };

Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
 drivers/gpio/Kconfig               |   6 +
 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpio-altera-fpgamgr.c | 219 +++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)
 create mode 100644 drivers/gpio/gpio-altera-fpgamgr.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3388d54..0bec903 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -97,6 +97,12 @@ config GPIO_ALTERA
 
 	  If driver is built as a module it will be called gpio-altera.
 
+config GPIO_ALTERA_FPGAMGR
+	tristate "Altera FPGAMGR GPIO"
+	depends on OF_GPIO
+	help
+	  Say yes here to support the Altera FPGAMGR GPIO device.
+
 config GPIO_AMDPT
 	tristate "AMD Promontory GPIO support"
 	depends on ACPI
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index aeb70e9d..3eb73d4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
 obj-$(CONFIG_GPIO_ALTERA_A10SR)	+= gpio-altera-a10sr.o
+obj-$(CONFIG_GPIO_ALTERA_FPGAMGR)	+= gpio-altera-fpgamgr.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMDPT)	+= gpio-amdpt.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
diff --git a/drivers/gpio/gpio-altera-fpgamgr.c b/drivers/gpio/gpio-altera-fpgamgr.c
new file mode 100644
index 0000000..76eff81
--- /dev/null
+++ b/drivers/gpio/gpio-altera-fpgamgr.c
@@ -0,0 +1,219 @@
+/*
+ * This is a GPIO driver for the internal FPGA Manager I/O ports
+ * connecting the HPS to the FPGA logic on certain Altera parts.
+ *
+ * Copyright (c) 2015 Softing Industrial Automation GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include <linux/gpio/driver.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+
+struct fpgamgr_port_property {
+	struct device_node		*node;
+	const char			*name;
+	unsigned int			idx;
+};
+
+struct fpgamgr_platform_data {
+	struct fpgamgr_port_property	*properties;
+	unsigned int			nports;
+};
+
+struct fpgamgr_gpio_port {
+	struct gpio_chip		bgc;
+	struct fpgamgr_gpio		*gpio;
+	unsigned int			idx;
+};
+
+struct fpgamgr_gpio {
+	struct	device			*dev;
+	void __iomem			*regs;
+	struct fpgamgr_gpio_port	*ports;
+	unsigned int			nr_ports;
+};
+
+static int fpgamgr_gpio_add_port(struct fpgamgr_gpio *gpio,
+				 struct fpgamgr_port_property *pp,
+				 unsigned int offs)
+{
+	struct fpgamgr_gpio_port *port;
+	void __iomem *dat;
+	int err;
+
+	port = &gpio->ports[offs];
+	port->gpio = gpio;
+	port->idx = pp->idx;
+
+	dat = gpio->regs + (pp->idx * 4);
+
+	err = bgpio_init(&port->bgc, gpio->dev, 4, dat, NULL, NULL,
+			 NULL, NULL, 0);
+	if (err) {
+		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
+			pp->name);
+		return err;
+	}
+
+	port->bgc.of_node = pp->node;
+
+	err = devm_gpiochip_add_data(gpio->dev, &port->bgc, NULL);
+	if (err)
+		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
+			pp->name);
+
+	return err;
+}
+
+static struct fpgamgr_platform_data *
+fpgamgr_gpio_get_pdata_of(struct device *dev)
+{
+	struct device_node *np = dev->of_node, *port_np;
+	struct fpgamgr_platform_data *pdata;
+	struct fpgamgr_port_property *pp;
+	int nports;
+	int i;
+
+	if (!np)
+		return ERR_PTR(-ENODEV);
+
+	nports = of_get_child_count(np);
+	if (nports == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->properties = kcalloc(nports, sizeof(*pp), GFP_KERNEL);
+	if (!pdata->properties) {
+		kfree(pdata);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pdata->nports = nports;
+
+	i = 0;
+	for_each_child_of_node(np, port_np) {
+		pp = &pdata->properties[i++];
+		pp->node = port_np;
+
+		if (of_property_read_u32(port_np, "reg", &pp->idx) ||
+		    pp->idx > 1) {
+			dev_err(dev, "missing/invalid port index for %s\n",
+				port_np->full_name);
+			kfree(pdata->properties);
+			kfree(pdata);
+			return ERR_PTR(-EINVAL);
+		}
+
+		pp->name = port_np->full_name;
+	}
+
+	return pdata;
+}
+
+static inline void fpgamgr_free_pdata_of(struct fpgamgr_platform_data *pdata)
+{
+	if (!pdata)
+		return;
+
+	kfree(pdata->properties);
+	kfree(pdata);
+}
+
+static int fpgamgr_gpio_probe(struct platform_device *pdev)
+{
+	unsigned int i;
+	struct resource *res;
+	struct fpgamgr_gpio *fgpio;
+	int err;
+	struct device *dev = &pdev->dev;
+	struct fpgamgr_platform_data *pdata = dev_get_platdata(dev);
+	bool is_pdata_alloc = !pdata;
+
+	if (is_pdata_alloc) {
+		pdata = fpgamgr_gpio_get_pdata_of(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	if (!pdata->nports) {
+		err = -ENODEV;
+		goto out_err;
+	}
+
+	fgpio = devm_kzalloc(dev, sizeof(*fgpio), GFP_KERNEL);
+	if (!fgpio) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+	fgpio->dev = dev;
+	fgpio->nr_ports = pdata->nports;
+
+	fgpio->ports = devm_kcalloc(dev, fgpio->nr_ports,
+				   sizeof(*fgpio->ports), GFP_KERNEL);
+	if (!fgpio->ports) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	fgpio->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(fgpio->regs)) {
+		err = PTR_ERR(fgpio->regs);
+		goto out_err;
+	}
+
+	for (i = 0; i < fgpio->nr_ports; i++) {
+		err = fpgamgr_gpio_add_port(fgpio, &pdata->properties[i], i);
+		if (err)
+			goto out_unregister;
+	}
+	platform_set_drvdata(pdev, fgpio);
+
+	goto out_err;
+
+out_unregister:
+	while (i > 0)
+		devm_gpiochip_remove(dev, &fgpio->ports[--i].bgc);
+
+out_err:
+	if (is_pdata_alloc)
+		fpgamgr_free_pdata_of(pdata);
+
+	return err;
+}
+
+static const struct of_device_id fpgamgr_of_match[] = {
+	{ .compatible = "altr,fpgamgr-gpio" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fpgamgr_of_match);
+
+static struct platform_driver fpgamgr_gpio_driver = {
+	.driver		= {
+		.name	= "gpio-altera-fpgamgr",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(fpgamgr_of_match),
+	},
+	.probe		= fpgamgr_gpio_probe,
+};
+
+module_platform_driver(fpgamgr_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bernd Edlinger");
+MODULE_DESCRIPTION("Altera fpgamgr GPIO driver");
-- 
2.7.4

Re: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Christian Lamparter]

Hello,

On Saturday, September 30, 2017 1:15:38 PM CEST Bernd Edlinger wrote:
> Absolutely it looks and feels like a general-purpose I/O facility.
> It can be used to control arbitrary logic functions.
Hm, Is it possible to differenciate it from a "syscon" device?

"System controller node represents a register region containing a set
of miscellaneous registers. The registers are not cohesive enough to
represent as any specific type of device. The typical use-case is for
some other node's driver, or platform-specific code, to acquire a
reference to the syscon node (e.g. by phandle, node path, or search
using a specific compatible value), interrogate the node (or associated
OS driver) to determine the location of the registers, and access the
registers directly."

<http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mfd/syscon.txt>

> >> Instantiate this in the device tree as:
> >>
> >>     gpio3: gpio@ff706010 {
> >>      #address-cells = <1>;
> >>      #size-cells = <0>;
> >>      compatible = "altr,fpgamgr-gpio";
> >>      reg = <0xff706010 0x8>;
> >>      status = "okay";
> >>
> >>      portd: gpio-controller@0 {
> >>       compatible = "altr,fpgamgr-gpio-output";
> >>       gpio-controller;
> >>       #gpio-cells = <2>;
> >>       reg = <0>;
> >>      };
> >>
> >>      porte: gpio-controller@1 {
> >>       compatible = "altr,fpgamgr-gpio-input";
> >>       gpio-controller;
> >>       #gpio-cells = <2>;
> >>       reg = <1>;
> >>      };
> > 
> > So one port is output-only and one is input-only?
> > 
> > Fair enough.

The driver seemd fairly simple. So, You could actually get
away with just adding the "altr,fpgamgr-gpio" compatible string
to gpio-mmio.c's bgpio_of_match struct at [0] and change the dt
to something like this:

	portd: gpio@ff706010 {
		compatible = "altr,fpgamgr-gpio";
		reg = <0xff706010 0x4>;
		reg-names = "dat";
		gpio-controller;
		#gpio-cells = <2>;
   }

   	porte: gpio@ff706014 {
		compatible = "altr,fpgamgr-gpio";
		reg = <0xff706014 0x4>;
		reg-names = "dat";
		gpio-controller;
		#gpio-cells = <2>;
		no-output;
   }

I added "no-output" property to the "porte" gpio definition.
This property will force the direction of all of porte gpios
to "in". (Which based on the "input", is what you want?)

Note: If you need any insperation for the missing Device-tree
binding document, you can look at:

wd,mbl-gpio.txt [1] and ni,169445-nand-gpio.txt [2].

Regards,
Christian

[0] <https://github.com/torvalds/linux/blob/master/drivers/gpio/gpio-mmio.c#L575>
[1] <https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/gpio/wd%2Cmbl-gpio.txt
[2] <https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/gpio/ni%2C169445-nand-gpio.txt>

Re: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Linus Walleij]

On Sat, Sep 30, 2017 at 3:15 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 09/27/17 02:20, Linus Walleij wrote:
>> On Fri, Sep 22, 2017 at 8:41 PM, Bernd Edlinger

> Though I must admit, that I have copied most of this code verbatim
> from gpio-dwapb.c ;)

All right, so then we need to keep this in mind in the future, we are going
to see more of these kind of layouts.

>> Please read Thierry's proposed changes that I will merge as soon as
>> he respins them, he creates a "bank" abstraction for gpiolib:
>> https://marc.info/?l=linux-gpio&m=150429237121695&w=2
>>
>> I strongly feel you should use this infrastructure, even if Thierry's work
>> is much centered around being able to have per-bank IRQs.
>>
>
> I hope that does not mean both ports have to share the same
> interrupt spin lock.  While it is necessary to synchronize write
> accesses to the same port, this driver has no need to synchronize
> with the other port.
>
> Interesting point is that this patch does not touch gpio-dwapb.c
> at all.  Although gpio-dwapb.c has even irq support.

It came up, and it is pretty evident that when we add a port/bank abstraction
it should be possible to reuse with dwapb as well, we need to cover a
few basic cases like that.

> I have probably never tested the unregister code path at all.

You can do this in sysfs using the "bind" and "unbind"
files. Echo the bus ID into these to unbind/bind the driver.

> From 223be945bf0a7d7d4bc979eccd77075b0a447ece Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Thu, 21 Sep 2017 15:50:36 +0200
> Subject: [PATCH] Add a GPIO driver for Altera FPGA Manager Fabric I/O.

Just resend the patch with a v2 suffix after PATCH.

Else I get confused!

Yours,
Linus Walleij

[PATCHv2] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Bernd Edlinger]

On 10/07/17 12:39, Linus Walleij wrote:
> Just resend the patch with a v2 suffix after PATCH.
> 
> Else I get confused!

Sure, resent patch follows.
I will send the device tree bindings patch in a separate mail.


 From 223be945bf0a7d7d4bc979eccd77075b0a447ece Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Thu, 21 Sep 2017 15:50:36 +0200
Subject: [PATCHv2] Add a GPIO driver for Altera FPGA Manager Fabric I/O.

This is an internal 32-bit input and 32-bit output port to the FPGA logic.

Instantiate this in the device tree as:

   gpio3: gpio@ff706010 {
    #address-cells = <1>;
    #size-cells = <0>;
    compatible = "altr,fpgamgr-gpio";
    reg = <0xff706010 0x8>;
    status = "okay";

    portd: gpio-controller@0 {
     compatible = "altr,fpgamgr-gpio-output";
     gpio-controller;
     #gpio-cells = <2>;
     reg = <0>;
    };

    porte: gpio-controller@1 {
     compatible = "altr,fpgamgr-gpio-input";
     gpio-controller;
     #gpio-cells = <2>;
     reg = <1>;
    };
   };

Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
  drivers/gpio/Kconfig               |   6 +
  drivers/gpio/Makefile              |   1 +
  drivers/gpio/gpio-altera-fpgamgr.c | 219 
+++++++++++++++++++++++++++++++++++++
  3 files changed, 226 insertions(+)
  create mode 100644 drivers/gpio/gpio-altera-fpgamgr.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3388d54..0bec903 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -97,6 +97,12 @@ config GPIO_ALTERA

  	  If driver is built as a module it will be called gpio-altera.

+config GPIO_ALTERA_FPGAMGR
+	tristate "Altera FPGAMGR GPIO"
+	depends on OF_GPIO
+	help
+	  Say yes here to support the Altera FPGAMGR GPIO device.
+
  config GPIO_AMDPT
  	tristate "AMD Promontory GPIO support"
  	depends on ACPI
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index aeb70e9d..3eb73d4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
  obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
  obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
  obj-$(CONFIG_GPIO_ALTERA_A10SR)	+= gpio-altera-a10sr.o
+obj-$(CONFIG_GPIO_ALTERA_FPGAMGR)	+= gpio-altera-fpgamgr.o
  obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
  obj-$(CONFIG_GPIO_AMDPT)	+= gpio-amdpt.o
  obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
diff --git a/drivers/gpio/gpio-altera-fpgamgr.c 
b/drivers/gpio/gpio-altera-fpgamgr.c
new file mode 100644
index 0000000..76eff81
--- /dev/null
+++ b/drivers/gpio/gpio-altera-fpgamgr.c
@@ -0,0 +1,219 @@
+/*
+ * This is a GPIO driver for the internal FPGA Manager I/O ports
+ * connecting the HPS to the FPGA logic on certain Altera parts.
+ *
+ * Copyright (c) 2015 Softing Industrial Automation GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include <linux/gpio/driver.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+
+struct fpgamgr_port_property {
+	struct device_node		*node;
+	const char			*name;
+	unsigned int			idx;
+};
+
+struct fpgamgr_platform_data {
+	struct fpgamgr_port_property	*properties;
+	unsigned int			nports;
+};
+
+struct fpgamgr_gpio_port {
+	struct gpio_chip		bgc;
+	struct fpgamgr_gpio		*gpio;
+	unsigned int			idx;
+};
+
+struct fpgamgr_gpio {
+	struct	device			*dev;
+	void __iomem			*regs;
+	struct fpgamgr_gpio_port	*ports;
+	unsigned int			nr_ports;
+};
+
+static int fpgamgr_gpio_add_port(struct fpgamgr_gpio *gpio,
+				 struct fpgamgr_port_property *pp,
+				 unsigned int offs)
+{
+	struct fpgamgr_gpio_port *port;
+	void __iomem *dat;
+	int err;
+
+	port = &gpio->ports[offs];
+	port->gpio = gpio;
+	port->idx = pp->idx;
+
+	dat = gpio->regs + (pp->idx * 4);
+
+	err = bgpio_init(&port->bgc, gpio->dev, 4, dat, NULL, NULL,
+			 NULL, NULL, 0);
+	if (err) {
+		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
+			pp->name);
+		return err;
+	}
+
+	port->bgc.of_node = pp->node;
+
+	err = devm_gpiochip_add_data(gpio->dev, &port->bgc, NULL);
+	if (err)
+		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
+			pp->name);
+
+	return err;
+}
+
+static struct fpgamgr_platform_data *
+fpgamgr_gpio_get_pdata_of(struct device *dev)
+{
+	struct device_node *np = dev->of_node, *port_np;
+	struct fpgamgr_platform_data *pdata;
+	struct fpgamgr_port_property *pp;
+	int nports;
+	int i;
+
+	if (!np)
+		return ERR_PTR(-ENODEV);
+
+	nports = of_get_child_count(np);
+	if (nports == 0)
+		return ERR_PTR(-ENODEV);
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->properties = kcalloc(nports, sizeof(*pp), GFP_KERNEL);
+	if (!pdata->properties) {
+		kfree(pdata);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pdata->nports = nports;
+
+	i = 0;
+	for_each_child_of_node(np, port_np) {
+		pp = &pdata->properties[i++];
+		pp->node = port_np;
+
+		if (of_property_read_u32(port_np, "reg", &pp->idx) ||
+		    pp->idx > 1) {
+			dev_err(dev, "missing/invalid port index for %s\n",
+				port_np->full_name);
+			kfree(pdata->properties);
+			kfree(pdata);
+			return ERR_PTR(-EINVAL);
+		}
+
+		pp->name = port_np->full_name;
+	}
+
+	return pdata;
+}
+
+static inline void fpgamgr_free_pdata_of(struct fpgamgr_platform_data 
*pdata)
+{
+	if (!pdata)
+		return;
+
+	kfree(pdata->properties);
+	kfree(pdata);
+}
+
+static int fpgamgr_gpio_probe(struct platform_device *pdev)
+{
+	unsigned int i;
+	struct resource *res;
+	struct fpgamgr_gpio *fgpio;
+	int err;
+	struct device *dev = &pdev->dev;
+	struct fpgamgr_platform_data *pdata = dev_get_platdata(dev);
+	bool is_pdata_alloc = !pdata;
+
+	if (is_pdata_alloc) {
+		pdata = fpgamgr_gpio_get_pdata_of(dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
+	if (!pdata->nports) {
+		err = -ENODEV;
+		goto out_err;
+	}
+
+	fgpio = devm_kzalloc(dev, sizeof(*fgpio), GFP_KERNEL);
+	if (!fgpio) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+	fgpio->dev = dev;
+	fgpio->nr_ports = pdata->nports;
+
+	fgpio->ports = devm_kcalloc(dev, fgpio->nr_ports,
+				   sizeof(*fgpio->ports), GFP_KERNEL);
+	if (!fgpio->ports) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	fgpio->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(fgpio->regs)) {
+		err = PTR_ERR(fgpio->regs);
+		goto out_err;
+	}
+
+	for (i = 0; i < fgpio->nr_ports; i++) {
+		err = fpgamgr_gpio_add_port(fgpio, &pdata->properties[i], i);
+		if (err)
+			goto out_unregister;
+	}
+	platform_set_drvdata(pdev, fgpio);
+
+	goto out_err;
+
+out_unregister:
+	while (i > 0)
+		devm_gpiochip_remove(dev, &fgpio->ports[--i].bgc);
+
+out_err:
+	if (is_pdata_alloc)
+		fpgamgr_free_pdata_of(pdata);
+
+	return err;
+}
+
+static const struct of_device_id fpgamgr_of_match[] = {
+	{ .compatible = "altr,fpgamgr-gpio" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fpgamgr_of_match);
+
+static struct platform_driver fpgamgr_gpio_driver = {
+	.driver		= {
+		.name	= "gpio-altera-fpgamgr",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(fpgamgr_of_match),
+	},
+	.probe		= fpgamgr_gpio_probe,
+};
+
+module_platform_driver(fpgamgr_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bernd Edlinger");
+MODULE_DESCRIPTION("Altera fpgamgr GPIO driver");
-- 
2.7.4

Re: [PATCHv2] Add a GPIO driver for Altera FPGA Manager Fabric I/O

[Linus Walleij]

On Sat, Oct 7, 2017 at 7:39 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:

> On 10/07/17 12:39, Linus Walleij wrote:
>> Just resend the patch with a v2 suffix after PATCH.
>>
>> Else I get confused!
>
> Sure, resent patch follows.
> I will send the device tree bindings patch in a separate mail.

The bindings must be reviewed first before I can apply them
and then the driver.

Please keep the patches together in a series 1/2, 2/2.

Please send patches with git send-email as separate mails,
not inline.

Yours,
Linus Walleij