[PATCH v5 0/7]gpio/xilinx: Add support for PCI mapped devices and rmmod

[PATCH v5 0/7]gpio/xilinx: Add support for PCI mapped devices and rmmod

[Ricardo Ribalda Delgado]

The main purpose of this patchset is to use the driver on a PCI mapped fpga.

This patchset also converts the driver to platform device, so it will support
hot plugged devices and rmmod.

Changeset:
v5:
Suggested by Alexandre Courbot <gnurou@gmail.com>
-Create a single gpio chip on dual cores
-Create new function of_mm_gpiochip_remove

v4:
Suggested by Michal Simek <michal.simek@xilinx.com>
-Add missing module_exit

v3
Suggested by Michal Simek <michal.simek@xilinx.com>
-Add patch for fixing kernel-doc
-Leave the driver at subsys_initcall


v2
Suggested by Alexandre Courbot <gnurou@gmail.com>
Merge 2 and 3 (convert to platform device and implement remove)

Ricardo Ribalda Delgado (7):
  gpio/xilinx: Remove offset property
  gpio/xilinx: Convert the driver to platform device interface
  gpio/xilinx: Add support for X86 Arch
  gpio/xilinx: Fix kernel-doc
  gpio/xilinx: Create a single gpio chip on dual cores
  gpio/gpiolib-of: Create of_mm_gpiochip_remove
  gpio/xilinx: Use of_mm_gpiochip_remove

 drivers/gpio/Kconfig       |   4 +-
 drivers/gpio/gpio-xilinx.c | 284 ++++++++++++++++++++++++++-------------------
 drivers/gpio/gpiolib-of.c  |  17 +++
 include/linux/of_gpio.h    |   1 +
 4 files changed, 182 insertions(+), 124 deletions(-)

-- 
2.1.3

[PATCH v5 1/7] gpio/xilinx: Remove offset property

[Ricardo Ribalda Delgado]

Instead of calculating the register offset per call, pre-calculate it on
probe time.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-xilinx.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index ba18b06..e668ec4 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -43,14 +43,12 @@
  * struct of_mm_gpio_chip mmchip: OF GPIO chip for memory mapped banks
  * gpio_state: GPIO state shadow register
  * gpio_dir: GPIO direction shadow register
- * offset: GPIO channel offset
  * gpio_lock: Lock used for synchronization
  */
 struct xgpio_instance {
 	struct of_mm_gpio_chip mmchip;
 	u32 gpio_state;
 	u32 gpio_dir;
-	u32 offset;
 	spinlock_t gpio_lock;
 };
 
@@ -65,12 +63,8 @@ struct xgpio_instance {
 static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
-	struct xgpio_instance *chip =
-	    container_of(mm_gc, struct xgpio_instance, mmchip);
 
-	void __iomem *regs = mm_gc->regs + chip->offset;
-
-	return !!(xgpio_readreg(regs + XGPIO_DATA_OFFSET) & BIT(gpio));
+	return !!(xgpio_readreg(mm_gc->regs + XGPIO_DATA_OFFSET) & BIT(gpio));
 }
 
 /**
@@ -88,7 +82,6 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
-	void __iomem *regs = mm_gc->regs;
 
 	spin_lock_irqsave(&chip->gpio_lock, flags);
 
@@ -98,8 +91,7 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	else
 		chip->gpio_state &= ~BIT(gpio);
 
-	xgpio_writereg(regs + chip->offset + XGPIO_DATA_OFFSET,
-							 chip->gpio_state);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
 
 	spin_unlock_irqrestore(&chip->gpio_lock, flags);
 }
@@ -119,13 +111,12 @@ static int xgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
-	void __iomem *regs = mm_gc->regs;
 
 	spin_lock_irqsave(&chip->gpio_lock, flags);
 
 	/* Set the GPIO bit in shadow register and set direction as input */
 	chip->gpio_dir |= BIT(gpio);
-	xgpio_writereg(regs + chip->offset + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
 
 	spin_unlock_irqrestore(&chip->gpio_lock, flags);
 
@@ -148,7 +139,6 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
-	void __iomem *regs = mm_gc->regs;
 
 	spin_lock_irqsave(&chip->gpio_lock, flags);
 
@@ -157,12 +147,11 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 		chip->gpio_state |= BIT(gpio);
 	else
 		chip->gpio_state &= ~BIT(gpio);
-	xgpio_writereg(regs + chip->offset + XGPIO_DATA_OFFSET,
-		       chip->gpio_state);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
 
 	/* Clear the GPIO bit in shadow register and set direction as output */
 	chip->gpio_dir &= ~BIT(gpio);
-	xgpio_writereg(regs + chip->offset + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
 
 	spin_unlock_irqrestore(&chip->gpio_lock, flags);
 
@@ -178,10 +167,8 @@ static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
 
-	xgpio_writereg(mm_gc->regs + chip->offset + XGPIO_DATA_OFFSET,
-							chip->gpio_state);
-	xgpio_writereg(mm_gc->regs + chip->offset + XGPIO_TRI_OFFSET,
-							 chip->gpio_dir);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET,	chip->gpio_state);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
 }
 
 /**
@@ -247,9 +234,6 @@ static int xgpio_of_probe(struct device_node *np)
 		if (!chip)
 			return -ENOMEM;
 
-		/* Add dual channel offset */
-		chip->offset = XGPIO_CHANNEL_OFFSET;
-
 		/* Update GPIO state shadow register with default value */
 		of_property_read_u32(np, "xlnx,dout-default-2",
 				     &chip->gpio_state);
@@ -285,6 +269,10 @@ static int xgpio_of_probe(struct device_node *np)
 			np->full_name, status);
 			return status;
 		}
+
+		/* Add dual channel offset */
+		chip->mmchip.regs += XGPIO_CHANNEL_OFFSET;
+
 		pr_info("XGpio: %s: dual channel registered, base is %d\n",
 					np->full_name, chip->mmchip.gc.base);
 	}
-- 
2.1.3

Re: [PATCH v5 1/7] gpio/xilinx: Remove offset property

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Instead of calculating the register offset per call, pre-calculate it on
> probe time.
>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied. Sorry for the fat delays on this patch set.

Yours,
Linus Walleij

[PATCH v5 2/7] gpio/xilinx: Convert the driver to platform device interface

[Ricardo Ribalda Delgado]

This way we do not need to transverse the device tree manually and we
support hot plugged devices.

Also Implement remove callback so the driver can be unloaded

Acked-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-xilinx.c | 83 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 66 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index e668ec4..c7ed92b 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -44,12 +44,18 @@
  * gpio_state: GPIO state shadow register
  * gpio_dir: GPIO direction shadow register
  * gpio_lock: Lock used for synchronization
+ * inited: True if the port has been inited
  */
 struct xgpio_instance {
 	struct of_mm_gpio_chip mmchip;
 	u32 gpio_state;
 	u32 gpio_dir;
 	spinlock_t gpio_lock;
+	bool inited;
+};
+
+struct xgpio {
+	struct xgpio_instance port[2];
 };
 
 /**
@@ -172,24 +178,56 @@ static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
 }
 
 /**
+ * xgpio_remove - Remove method for the GPIO device.
+ * @pdev: pointer to the platform device
+ *
+ * This function remove gpiochips and frees all the allocated resources.
+ */
+static int xgpio_remove(struct platform_device *pdev)
+{
+	struct xgpio *xgpio = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		if (!xgpio->port[i].inited)
+			continue;
+		gpiochip_remove(&xgpio->port[i].mmchip.gc);
+
+		if (i == 1)
+			xgpio->port[i].mmchip.regs -= XGPIO_CHANNEL_OFFSET;
+
+		iounmap(xgpio->port[i].mmchip.regs);
+		kfree(xgpio->port[i].mmchip.gc.label);
+	}
+
+	return 0;
+}
+
+/**
  * xgpio_of_probe - Probe method for the GPIO device.
- * @np: pointer to device tree node
+ * @pdev: pointer to the platform device
  *
  * This function probes the GPIO device in the device tree. It initializes the
  * driver data structure. It returns 0, if the driver is bound to the GPIO
  * device, or a negative value if there is an error.
  */
-static int xgpio_of_probe(struct device_node *np)
+static int xgpio_probe(struct platform_device *pdev)
 {
+	struct xgpio *xgpio;
 	struct xgpio_instance *chip;
 	int status = 0;
+	struct device_node *np = pdev->dev.of_node;
 	const u32 *tree_info;
 	u32 ngpio;
 
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-	if (!chip)
+	xgpio = devm_kzalloc(&pdev->dev, sizeof(*xgpio), GFP_KERNEL);
+	if (!xgpio)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, xgpio);
+
+	chip = &xgpio->port[0];
+
 	/* Update GPIO state shadow register with default value */
 	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state);
 
@@ -209,6 +247,7 @@ static int xgpio_of_probe(struct device_node *np)
 
 	spin_lock_init(&chip->gpio_lock);
 
+	chip->mmchip.gc.dev = &pdev->dev;
 	chip->mmchip.gc.direction_input = xgpio_dir_in;
 	chip->mmchip.gc.direction_output = xgpio_dir_out;
 	chip->mmchip.gc.get = xgpio_get;
@@ -219,20 +258,18 @@ static int xgpio_of_probe(struct device_node *np)
 	/* Call the OF gpio helper to setup and register the GPIO device */
 	status = of_mm_gpiochip_add(np, &chip->mmchip);
 	if (status) {
-		kfree(chip);
 		pr_err("%s: error in probe function with status %d\n",
 		       np->full_name, status);
 		return status;
 	}
+	chip->inited = true;
 
 	pr_info("XGpio: %s: registered, base is %d\n", np->full_name,
 							chip->mmchip.gc.base);
 
 	tree_info = of_get_property(np, "xlnx,is-dual", NULL);
 	if (tree_info && be32_to_cpup(tree_info)) {
-		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-		if (!chip)
-			return -ENOMEM;
+		chip = &xgpio->port[1];
 
 		/* Update GPIO state shadow register with default value */
 		of_property_read_u32(np, "xlnx,dout-default-2",
@@ -254,6 +291,7 @@ static int xgpio_of_probe(struct device_node *np)
 
 		spin_lock_init(&chip->gpio_lock);
 
+		chip->mmchip.gc.dev = &pdev->dev;
 		chip->mmchip.gc.direction_input = xgpio_dir_in;
 		chip->mmchip.gc.direction_output = xgpio_dir_out;
 		chip->mmchip.gc.get = xgpio_get;
@@ -264,7 +302,7 @@ static int xgpio_of_probe(struct device_node *np)
 		/* Call the OF gpio helper to setup and register the GPIO dev */
 		status = of_mm_gpiochip_add(np, &chip->mmchip);
 		if (status) {
-			kfree(chip);
+			xgpio_remove(pdev);
 			pr_err("%s: error in probe function with status %d\n",
 			np->full_name, status);
 			return status;
@@ -272,6 +310,7 @@ static int xgpio_of_probe(struct device_node *np)
 
 		/* Add dual channel offset */
 		chip->mmchip.regs += XGPIO_CHANNEL_OFFSET;
+		chip->inited = true;
 
 		pr_info("XGpio: %s: dual channel registered, base is %d\n",
 					np->full_name, chip->mmchip.gc.base);
@@ -285,19 +324,29 @@ static const struct of_device_id xgpio_of_match[] = {
 	{ /* end of list */ },
 };
 
-static int __init xgpio_init(void)
-{
-	struct device_node *np;
+MODULE_DEVICE_TABLE(of, xgpio_of_match);
 
-	for_each_matching_node(np, xgpio_of_match)
-		xgpio_of_probe(np);
+static struct platform_driver xgpio_plat_driver = {
+	.probe		= xgpio_probe,
+	.remove		= xgpio_remove,
+	.driver		= {
+			.name = "gpio-xilinx",
+			.of_match_table	= xgpio_of_match,
+	},
+};
 
-	return 0;
+static int __init xgpio_init(void)
+{
+	return platform_driver_register(&xgpio_plat_driver);
 }
 
-/* Make sure we get initialized before anyone else tries to use us */
 subsys_initcall(xgpio_init);
-/* No exit call at the moment as we cannot unregister of GPIO chips */
+
+static void __exit xgpio_exit(void)
+{
+	platform_driver_unregister(&xgpio_plat_driver);
+}
+module_exit(xgpio_exit);
 
 MODULE_AUTHOR("Xilinx, Inc.");
 MODULE_DESCRIPTION("Xilinx GPIO driver");
-- 
2.1.3

Re: [PATCH v5 2/7] gpio/xilinx: Convert the driver to platform device interface

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> This way we do not need to transverse the device tree manually and we
> support hot plugged devices.
>
> Also Implement remove callback so the driver can be unloaded
>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Ricardo Ribalda Delgado]

Core can be accessed via PCIe on X86 platform.
This patch also allows the driver to be used as module.

Acked-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/Kconfig       | 4 ++--
 drivers/gpio/gpio-xilinx.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 414d055..b17d226 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -366,8 +366,8 @@ config GPIO_XGENE
 	  here to enable the GFC GPIO functionality.
 
 config GPIO_XILINX
-	bool "Xilinx GPIO support"
-	depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
+	tristate "Xilinx GPIO support"
+	depends on OF_GPIO && (PPC_OF || MICROBLAZE || ARCH_ZYNQ || ARCH_X86)
 	help
 	  Say yes here to support the Xilinx FPGA GPIO device
 
diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index c7ed92b..554060a 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -30,7 +30,7 @@
 #define XGPIO_CHANNEL_OFFSET	0x8
 
 /* Read/Write access to the GPIO registers */
-#ifdef CONFIG_ARCH_ZYNQ
+#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86)
 # define xgpio_readreg(offset)		readl(offset)
 # define xgpio_writereg(offset, val)	writel(val, offset)
 #else
-- 
2.1.3

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Core can be accessed via PCIe on X86 platform.
> This patch also allows the driver to be used as module.
>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

PCIe on x86, using device tree? The ACPI people are just gonna love this.

Patch applied, anyway. DT on x86 is not unheard of.

>  config GPIO_XILINX
> -       bool "Xilinx GPIO support"
> -       depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
> +       tristate "Xilinx GPIO support"
> +       depends on OF_GPIO && (PPC_OF || MICROBLAZE || ARCH_ZYNQ || ARCH_X86)

Maybe you want to add COMPILE_TEST too?

Yours,
Linus Walleij

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Ricardo Ribalda Delgado]

Hello

On Mon, Jan 12, 2015 at 10:24 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>
>> Core can be accessed via PCIe on X86 platform.
>> This patch also allows the driver to be used as module.
>>
>> Acked-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>
> PCIe on x86, using device tree? The ACPI people are just gonna love this.

>
> Patch applied, anyway. DT on x86 is not unheard of.
>
>>  config GPIO_XILINX
>> -       bool "Xilinx GPIO support"
>> -       depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
>> +       tristate "Xilinx GPIO support"
>> +       depends on OF_GPIO && (PPC_OF || MICROBLAZE || ARCH_ZYNQ || ARCH_X86)
>
> Maybe you want to add COMPILE_TEST too?

Up to you

>
> Yours,
> Linus Walleij



-- 
Ricardo Ribalda

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Paul Bolle]

On Wed, 2014-12-17 at 16:51 +0100, Ricardo Ribalda Delgado wrote:
> Core can be accessed via PCIe on X86 platform.
> This patch also allows the driver to be used as module.
> 
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/gpio/Kconfig       | 4 ++--
>  drivers/gpio/gpio-xilinx.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

This patch became c586b3075d5b ("gpio/xilinx: Add support for X86 Arch")
in today's linux-next (next-20150113). I noticed because a script I use
to check linux-next spotted a problem in it.

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 414d055..b17d226 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -366,8 +366,8 @@ config GPIO_XGENE
>  	  here to enable the GFC GPIO functionality.
>  
>  config GPIO_XILINX
> -	bool "Xilinx GPIO support"
> -	depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
> +	tristate "Xilinx GPIO support"
> +	depends on OF_GPIO && (PPC_OF || MICROBLAZE || ARCH_ZYNQ || ARCH_X86)

There's no Kconfig symbol "ARCH_X86". Did you actually mean "X86"?

>  	help
>  	  Say yes here to support the Xilinx FPGA GPIO device
>  
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index c7ed92b..554060a 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -30,7 +30,7 @@
>  #define XGPIO_CHANNEL_OFFSET	0x8
>  
>  /* Read/Write access to the GPIO registers */
> -#ifdef CONFIG_ARCH_ZYNQ
> +#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86)

Yes, you probably did want just "X86".

>  # define xgpio_readreg(offset)		readl(offset)
>  # define xgpio_writereg(offset, val)	writel(val, offset)
>  #else


Paul Bolle

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Ricardo Ribalda Delgado]

Hello Paul

Thanks for noticing

Shall I send the patch or you want to do it yourself?


Thanks again!

On Tue, Jan 13, 2015 at 10:02 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Wed, 2014-12-17 at 16:51 +0100, Ricardo Ribalda Delgado wrote:
>> Core can be accessed via PCIe on X86 platform.
>> This patch also allows the driver to be used as module.
>>
>> Acked-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/gpio/Kconfig       | 4 ++--
>>  drivers/gpio/gpio-xilinx.c | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> This patch became c586b3075d5b ("gpio/xilinx: Add support for X86 Arch")
> in today's linux-next (next-20150113). I noticed because a script I use
> to check linux-next spotted a problem in it.
>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 414d055..b17d226 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -366,8 +366,8 @@ config GPIO_XGENE
>>         here to enable the GFC GPIO functionality.
>>
>>  config GPIO_XILINX
>> -     bool "Xilinx GPIO support"
>> -     depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
>> +     tristate "Xilinx GPIO support"
>> +     depends on OF_GPIO && (PPC_OF || MICROBLAZE || ARCH_ZYNQ || ARCH_X86)
>
> There's no Kconfig symbol "ARCH_X86". Did you actually mean "X86"?
>
>>       help
>>         Say yes here to support the Xilinx FPGA GPIO device
>>
>> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
>> index c7ed92b..554060a 100644
>> --- a/drivers/gpio/gpio-xilinx.c
>> +++ b/drivers/gpio/gpio-xilinx.c
>> @@ -30,7 +30,7 @@
>>  #define XGPIO_CHANNEL_OFFSET 0x8
>>
>>  /* Read/Write access to the GPIO registers */
>> -#ifdef CONFIG_ARCH_ZYNQ
>> +#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86)
>
> Yes, you probably did want just "X86".
>
>>  # define xgpio_readreg(offset)               readl(offset)
>>  # define xgpio_writereg(offset, val) writel(val, offset)
>>  #else
>
>
> Paul Bolle
>



-- 
Ricardo Ribalda

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Paul Bolle]

Hi Ricardo,

On Tue, 2015-01-13 at 10:04 +0100, Ricardo Ribalda Delgado wrote:
> Thanks for noticing
> 
> Shall I send the patch or you want to do it yourself?

I rather prefer you do it, even if it looks trivial.

Changing that line to
    [...] || X86)

will apparently enable a bit of code for the first time on x86. Code
that I know nothing about. I don't even know how to build or test it.
See, I've submitted such obviously correct and trivial changes before,
only to receive fan mail from the kbuild test robot the very next day. I
hope to not repeat that experience.


Paul Bolle

Re: [PATCH v5 3/7] gpio/xilinx: Add support for X86 Arch

[Linus Walleij]

On Tue, Jan 13, 2015 at 10:02 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Wed, 2014-12-17 at 16:51 +0100, Ricardo Ribalda Delgado wrote:
>> Core can be accessed via PCIe on X86 platform.
>> This patch also allows the driver to be used as module.
>>
>> Acked-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/gpio/Kconfig       | 4 ++--
>>  drivers/gpio/gpio-xilinx.c | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> This patch became c586b3075d5b ("gpio/xilinx: Add support for X86 Arch")
> in today's linux-next (next-20150113). I noticed because a script I use
> to check linux-next spotted a problem in it.

Yes the topic line was way too long so I just took my axe
and chopped of a string.

Yours,
Linus Walleij

[PATCH v5 4/7] gpio/xilinx: Fix kernel-doc

[Ricardo Ribalda Delgado]

Some documentation were not following the kernel-doc format.
Backporting patch from Xilinx git repository.

Suggested-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-xilinx.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 554060a..9cf4457 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -40,11 +40,11 @@
 
 /**
  * struct xgpio_instance - Stores information about GPIO device
- * struct of_mm_gpio_chip mmchip: OF GPIO chip for memory mapped banks
- * gpio_state: GPIO state shadow register
- * gpio_dir: GPIO direction shadow register
- * gpio_lock: Lock used for synchronization
- * inited: True if the port has been inited
+ * @mmchip: OF GPIO chip for memory mapped banks
+ * @gpio_state: GPIO state shadow register
+ * @gpio_dir: GPIO direction shadow register
+ * @gpio_lock: Lock used for synchronization
+ * @inited: True if the port has been inited
  */
 struct xgpio_instance {
 	struct of_mm_gpio_chip mmchip;
@@ -63,8 +63,11 @@ struct xgpio {
  * @gc:     Pointer to gpio_chip device structure.
  * @gpio:   GPIO signal number.
  *
- * This function reads the specified signal of the GPIO device. It returns 0 if
- * the signal clear, 1 if signal is set or negative value on error.
+ * This function reads the specified signal of the GPIO device.
+ *
+ * Return:
+ * 0 if direction of GPIO signals is set as input otherwise it
+ * returns negative error value.
  */
 static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
 {
@@ -107,9 +110,9 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
  * @gc:     Pointer to gpio_chip device structure.
  * @gpio:   GPIO signal number.
  *
- * This function sets the direction of specified GPIO signal as input.
- * It returns 0 if direction of GPIO signals is set as input otherwise it
- * returns negative error value.
+ * Return:
+ * 0 - if direction of GPIO signals is set as input
+ * otherwise it returns negative error value.
  */
 static int xgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 {
@@ -135,8 +138,10 @@ static int xgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
  * @gpio:   GPIO signal number.
  * @val:    Value to be written to specified signal.
  *
- * This function sets the direction of specified GPIO signal as output. If all
- * GPIO signals of GPIO chip is configured as input then it returns
+ * This function sets the direction of specified GPIO signal as output.
+ *
+ * Return:
+ * If all GPIO signals of GPIO chip is configured as input then it returns
  * error otherwise it returns 0.
  */
 static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
@@ -166,7 +171,7 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 
 /**
  * xgpio_save_regs - Set initial values of GPIO pins
- * @mm_gc: pointer to memory mapped GPIO chip structure
+ * @mm_gc: Pointer to memory mapped GPIO chip structure
  */
 static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
 {
@@ -207,9 +212,9 @@ static int xgpio_remove(struct platform_device *pdev)
  * xgpio_of_probe - Probe method for the GPIO device.
  * @pdev: pointer to the platform device
  *
- * This function probes the GPIO device in the device tree. It initializes the
- * driver data structure. It returns 0, if the driver is bound to the GPIO
- * device, or a negative value if there is an error.
+ * Return:
+ * It returns 0, if the driver is bound to the GPIO device, or
+ * a negative value if there is an error.
  */
 static int xgpio_probe(struct platform_device *pdev)
 {
-- 
2.1.3

Re: [PATCH v5 4/7] gpio/xilinx: Fix kernel-doc

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Some documentation were not following the kernel-doc format.
> Backporting patch from Xilinx git repository.
>
> Suggested-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v5 5/7] gpio/xilinx: Create a single gpio chip on dual cores

[Ricardo Ribalda Delgado]

Currently, we had two gpio chips on cores configured as dual.

This lead to mapping the same memory region twice and duplicating the
init and remove code.

This patch creates a single gpiochip for single and dual cores.

Suggested-by: Alexandre Courbot <gnurou@gmail.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-xilinx.c | 217 +++++++++++++++++++++++----------------------
 1 file changed, 109 insertions(+), 108 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 9cf4457..e89fb42 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -48,15 +48,35 @@
  */
 struct xgpio_instance {
 	struct of_mm_gpio_chip mmchip;
-	u32 gpio_state;
-	u32 gpio_dir;
-	spinlock_t gpio_lock;
-	bool inited;
+	unsigned int gpio_width[2];
+	u32 gpio_state[2];
+	u32 gpio_dir[2];
+	spinlock_t gpio_lock[2];
 };
 
-struct xgpio {
-	struct xgpio_instance port[2];
-};
+static inline int xgpio_index(struct xgpio_instance *chip, int gpio)
+{
+	if (gpio >= chip->gpio_width[0])
+		return 1;
+
+	return 0;
+}
+
+static inline int xgpio_regoffset(struct xgpio_instance *chip, int gpio)
+{
+	if (xgpio_index(chip, gpio))
+		return XGPIO_CHANNEL_OFFSET;
+
+	return 0;
+}
+
+static inline int xgpio_offset(struct xgpio_instance *chip, int gpio)
+{
+	if (xgpio_index(chip, gpio))
+		return gpio - chip->gpio_width[0];
+
+	return gpio;
+}
 
 /**
  * xgpio_get - Read the specified signal of the GPIO device.
@@ -72,8 +92,14 @@ struct xgpio {
 static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct xgpio_instance *chip =
+	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	u32 val;
 
-	return !!(xgpio_readreg(mm_gc->regs + XGPIO_DATA_OFFSET) & BIT(gpio));
+	val = xgpio_readreg(mm_gc->regs + XGPIO_DATA_OFFSET +
+			    xgpio_regoffset(chip, gpio));
+
+	return !!(val & BIT(xgpio_offset(chip, gpio)));
 }
 
 /**
@@ -91,18 +117,21 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	int index =  xgpio_index(chip, gpio);
+	int offset =  xgpio_offset(chip, gpio);
 
-	spin_lock_irqsave(&chip->gpio_lock, flags);
+	spin_lock_irqsave(&chip->gpio_lock[index], flags);
 
 	/* Write to GPIO signal and set its direction to output */
 	if (val)
-		chip->gpio_state |= BIT(gpio);
+		chip->gpio_state[index] |= BIT(offset);
 	else
-		chip->gpio_state &= ~BIT(gpio);
+		chip->gpio_state[index] &= ~BIT(offset);
 
-	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET +
+		       xgpio_regoffset(chip, gpio), chip->gpio_state[index]);
 
-	spin_unlock_irqrestore(&chip->gpio_lock, flags);
+	spin_unlock_irqrestore(&chip->gpio_lock[index], flags);
 }
 
 /**
@@ -120,14 +149,17 @@ static int xgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	int index =  xgpio_index(chip, gpio);
+	int offset =  xgpio_offset(chip, gpio);
 
-	spin_lock_irqsave(&chip->gpio_lock, flags);
+	spin_lock_irqsave(&chip->gpio_lock[index], flags);
 
 	/* Set the GPIO bit in shadow register and set direction as input */
-	chip->gpio_dir |= BIT(gpio);
-	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	chip->gpio_dir[index] |= BIT(offset);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET +
+		       xgpio_regoffset(chip, gpio), chip->gpio_dir[index]);
 
-	spin_unlock_irqrestore(&chip->gpio_lock, flags);
+	spin_unlock_irqrestore(&chip->gpio_lock[index], flags);
 
 	return 0;
 }
@@ -150,21 +182,25 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	int index =  xgpio_index(chip, gpio);
+	int offset =  xgpio_offset(chip, gpio);
 
-	spin_lock_irqsave(&chip->gpio_lock, flags);
+	spin_lock_irqsave(&chip->gpio_lock[index], flags);
 
 	/* Write state of GPIO signal */
 	if (val)
-		chip->gpio_state |= BIT(gpio);
+		chip->gpio_state[index] |= BIT(offset);
 	else
-		chip->gpio_state &= ~BIT(gpio);
-	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
+		chip->gpio_state[index] &= ~BIT(offset);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET +
+			xgpio_regoffset(chip, gpio), chip->gpio_state[index]);
 
 	/* Clear the GPIO bit in shadow register and set direction as output */
-	chip->gpio_dir &= ~BIT(gpio);
-	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	chip->gpio_dir[index] &= ~BIT(offset);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET +
+			xgpio_regoffset(chip, gpio), chip->gpio_dir[index]);
 
-	spin_unlock_irqrestore(&chip->gpio_lock, flags);
+	spin_unlock_irqrestore(&chip->gpio_lock[index], flags);
 
 	return 0;
 }
@@ -178,8 +214,16 @@ static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
 
-	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET,	chip->gpio_state);
-	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET,	chip->gpio_state[0]);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir[0]);
+
+	if (!chip->gpio_width[1])
+		return;
+
+	xgpio_writereg(mm_gc->regs + XGPIO_DATA_OFFSET + XGPIO_TRI_OFFSET,
+		       chip->gpio_state[1]);
+	xgpio_writereg(mm_gc->regs + XGPIO_TRI_OFFSET + XGPIO_TRI_OFFSET,
+		       chip->gpio_dir[1]);
 }
 
 /**
@@ -190,20 +234,12 @@ static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
  */
 static int xgpio_remove(struct platform_device *pdev)
 {
-	struct xgpio *xgpio = platform_get_drvdata(pdev);
-	int i;
+	struct xgpio_instance *chip = platform_get_drvdata(pdev);
 
-	for (i = 0; i < 2; i++) {
-		if (!xgpio->port[i].inited)
-			continue;
-		gpiochip_remove(&xgpio->port[i].mmchip.gc);
+	gpiochip_remove(&chip->mmchip.gc);
 
-		if (i == 1)
-			xgpio->port[i].mmchip.regs -= XGPIO_CHANNEL_OFFSET;
-
-		iounmap(xgpio->port[i].mmchip.regs);
-		kfree(xgpio->port[i].mmchip.gc.label);
-	}
+	iounmap(chip->mmchip.regs);
+	kfree(chip->mmchip.gc.label);
 
 	return 0;
 }
@@ -218,40 +254,58 @@ static int xgpio_remove(struct platform_device *pdev)
  */
 static int xgpio_probe(struct platform_device *pdev)
 {
-	struct xgpio *xgpio;
 	struct xgpio_instance *chip;
 	int status = 0;
 	struct device_node *np = pdev->dev.of_node;
-	const u32 *tree_info;
-	u32 ngpio;
+	u32 is_dual;
 
-	xgpio = devm_kzalloc(&pdev->dev, sizeof(*xgpio), GFP_KERNEL);
-	if (!xgpio)
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, xgpio);
-
-	chip = &xgpio->port[0];
+	platform_set_drvdata(pdev, chip);
 
 	/* Update GPIO state shadow register with default value */
-	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state);
-
-	/* By default, all pins are inputs */
-	chip->gpio_dir = 0xFFFFFFFF;
+	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state[0]);
 
 	/* Update GPIO direction shadow register with default value */
-	of_property_read_u32(np, "xlnx,tri-default", &chip->gpio_dir);
+	if (of_property_read_u32(np, "xlnx,tri-default", &chip->gpio_dir[0]))
+		chip->gpio_dir[0] = 0xFFFFFFFF;
 
 	/*
 	 * Check device node and parent device node for device width
 	 * and assume default width of 32
 	 */
-	if (of_property_read_u32(np, "xlnx,gpio-width", &ngpio))
-		ngpio = 32;
-	chip->mmchip.gc.ngpio = (u16)ngpio;
+	if (of_property_read_u32(np, "xlnx,gpio-width", &chip->gpio_width[0]))
+		chip->gpio_width[0] = 32;
+
+	spin_lock_init(&chip->gpio_lock[0]);
+
+	if (of_property_read_u32(np, "xlnx,is-dual", &is_dual))
+		is_dual = 0;
 
-	spin_lock_init(&chip->gpio_lock);
+	if (is_dual) {
+		/* Update GPIO state shadow register with default value */
+		of_property_read_u32(np, "xlnx,dout-default-2",
+				     &chip->gpio_state[1]);
+
+		/* Update GPIO direction shadow register with default value */
+		if (of_property_read_u32(np, "xlnx,tri-default-2",
+					 &chip->gpio_dir[1]))
+			chip->gpio_dir[1] = 0xFFFFFFFF;
 
+		/*
+		 * Check device node and parent device node for device width
+		 * and assume default width of 32
+		 */
+		if (of_property_read_u32(np, "xlnx,gpio2-width",
+					 &chip->gpio_width[1]))
+			chip->gpio_width[1] = 32;
+
+		spin_lock_init(&chip->gpio_lock[1]);
+	}
+
+	chip->mmchip.gc.ngpio = chip->gpio_width[0] + chip->gpio_width[1];
 	chip->mmchip.gc.dev = &pdev->dev;
 	chip->mmchip.gc.direction_input = xgpio_dir_in;
 	chip->mmchip.gc.direction_output = xgpio_dir_out;
@@ -267,59 +321,6 @@ static int xgpio_probe(struct platform_device *pdev)
 		       np->full_name, status);
 		return status;
 	}
-	chip->inited = true;
-
-	pr_info("XGpio: %s: registered, base is %d\n", np->full_name,
-							chip->mmchip.gc.base);
-
-	tree_info = of_get_property(np, "xlnx,is-dual", NULL);
-	if (tree_info && be32_to_cpup(tree_info)) {
-		chip = &xgpio->port[1];
-
-		/* Update GPIO state shadow register with default value */
-		of_property_read_u32(np, "xlnx,dout-default-2",
-				     &chip->gpio_state);
-
-		/* By default, all pins are inputs */
-		chip->gpio_dir = 0xFFFFFFFF;
-
-		/* Update GPIO direction shadow register with default value */
-		of_property_read_u32(np, "xlnx,tri-default-2", &chip->gpio_dir);
-
-		/*
-		 * Check device node and parent device node for device width
-		 * and assume default width of 32
-		 */
-		if (of_property_read_u32(np, "xlnx,gpio2-width", &ngpio))
-			ngpio = 32;
-		chip->mmchip.gc.ngpio = (u16)ngpio;
-
-		spin_lock_init(&chip->gpio_lock);
-
-		chip->mmchip.gc.dev = &pdev->dev;
-		chip->mmchip.gc.direction_input = xgpio_dir_in;
-		chip->mmchip.gc.direction_output = xgpio_dir_out;
-		chip->mmchip.gc.get = xgpio_get;
-		chip->mmchip.gc.set = xgpio_set;
-
-		chip->mmchip.save_regs = xgpio_save_regs;
-
-		/* Call the OF gpio helper to setup and register the GPIO dev */
-		status = of_mm_gpiochip_add(np, &chip->mmchip);
-		if (status) {
-			xgpio_remove(pdev);
-			pr_err("%s: error in probe function with status %d\n",
-			np->full_name, status);
-			return status;
-		}
-
-		/* Add dual channel offset */
-		chip->mmchip.regs += XGPIO_CHANNEL_OFFSET;
-		chip->inited = true;
-
-		pr_info("XGpio: %s: dual channel registered, base is %d\n",
-					np->full_name, chip->mmchip.gc.base);
-	}
 
 	return 0;
 }
-- 
2.1.3

Re: [PATCH v5 5/7] gpio/xilinx: Create a single gpio chip on dual cores

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Currently, we had two gpio chips on cores configured as dual.
>
> This lead to mapping the same memory region twice and duplicating the
> init and remove code.
>
> This patch creates a single gpiochip for single and dual cores.
>
> Suggested-by: Alexandre Courbot <gnurou@gmail.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v5 6/7] gpio/gpiolib-of: Create of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Create counterpart of of_mm_gpiochip_add(). This way the modules that
can be removable do not duplicate the cleanup code.

Suggested-by: Alexandre Courbot <gnurou@gmail.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 17 +++++++++++++++++
 include/linux/of_gpio.h   |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 604dbe6..3e2c6af 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -204,6 +204,23 @@ err0:
 }
 EXPORT_SYMBOL(of_mm_gpiochip_add);
 
+/**
+ * of_mm_gpiochip_remove - Remove memory mapped GPIO chip (bank)
+ * @mm_gc:	pointer to the of_mm_gpio_chip allocated structure
+ */
+void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc)
+{
+	struct gpio_chip *gc = &mm_gc->gc;
+
+	if (!mm_gc)
+		return;
+
+	gpiochip_remove(gc);
+	iounmap(mm_gc->regs);
+	kfree(gc->label);
+}
+EXPORT_SYMBOL(of_mm_gpiochip_remove);
+
 #ifdef CONFIG_PINCTRL
 static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 {
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 38fc050..69dbe31 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -52,6 +52,7 @@ extern int of_get_named_gpio_flags(struct device_node *np,
 
 extern int of_mm_gpiochip_add(struct device_node *np,
 			      struct of_mm_gpio_chip *mm_gc);
+extern void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc);
 
 extern void of_gpiochip_add(struct gpio_chip *gc);
 extern void of_gpiochip_remove(struct gpio_chip *gc);
-- 
2.1.3

Re: [PATCH v5 6/7] gpio/gpiolib-of: Create of_mm_gpiochip_remove

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Create counterpart of of_mm_gpiochip_add(). This way the modules that
> can be removable do not duplicate the cleanup code.
>
> Suggested-by: Alexandre Courbot <gnurou@gmail.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Hm.

$ git grep of_mm_gpiochip_add
arch/powerpc/sysdev/cpm1.c:     return of_mm_gpiochip_add(np, mm_gc);
arch/powerpc/sysdev/cpm1.c:     return of_mm_gpiochip_add(np, mm_gc);
arch/powerpc/sysdev/cpm_common.c:       return of_mm_gpiochip_add(np, mm_gc);
arch/powerpc/sysdev/ppc4xx_gpio.c:              ret =
of_mm_gpiochip_add(np, mm_gc);
arch/powerpc/sysdev/qe_lib/gpio.c:              ret =
of_mm_gpiochip_add(np, mm_gc);
arch/powerpc/sysdev/simple_gpio.c:      ret = of_mm_gpiochip_add(np, mm_gc);
drivers/gpio/gpio-ge.c: return of_mm_gpiochip_add(pdev->dev.of_node, mmchip);
drivers/gpio/gpio-mm-lantiq.c:  ret =
of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
drivers/gpio/gpio-mpc5200.c:    ret =
of_mm_gpiochip_add(ofdev->dev.of_node, &chip->mmchip);
drivers/gpio/gpio-mpc5200.c:    ret =
of_mm_gpiochip_add(ofdev->dev.of_node, &chip->mmchip);
drivers/gpio/gpio-mpc8xxx.c:    ret = of_mm_gpiochip_add(np, mm_gc);
drivers/gpio/gpio-xilinx.c:     status = of_mm_gpiochip_add(np, &chip->mmchip);
drivers/gpio/gpio-zevio.c:      status =
of_mm_gpiochip_add(pdev->dev.of_node, &(controller->chip));

Should these all be patched to use this, or reviewed for applicability?

Yours,
Linus Walleij

Re: [PATCH v5 6/7] gpio/gpiolib-of: Create of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Hello

I think that only gpio-mpc5200.c would benefit from the new API, I can
send a patchset to support removal and use the new API if you want.

Thanks!

On Mon, Jan 12, 2015 at 10:30 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>
>> Create counterpart of of_mm_gpiochip_add(). This way the modules that
>> can be removable do not duplicate the cleanup code.
>>
>> Suggested-by: Alexandre Courbot <gnurou@gmail.com>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>
> Patch applied.
>
> Hm.
>
> $ git grep of_mm_gpiochip_add
> arch/powerpc/sysdev/cpm1.c:     return of_mm_gpiochip_add(np, mm_gc);
> arch/powerpc/sysdev/cpm1.c:     return of_mm_gpiochip_add(np, mm_gc);
> arch/powerpc/sysdev/cpm_common.c:       return of_mm_gpiochip_add(np, mm_gc);
> arch/powerpc/sysdev/ppc4xx_gpio.c:              ret =
> of_mm_gpiochip_add(np, mm_gc);
> arch/powerpc/sysdev/qe_lib/gpio.c:              ret =
> of_mm_gpiochip_add(np, mm_gc);
> arch/powerpc/sysdev/simple_gpio.c:      ret = of_mm_gpiochip_add(np, mm_gc);
> drivers/gpio/gpio-ge.c: return of_mm_gpiochip_add(pdev->dev.of_node, mmchip);
> drivers/gpio/gpio-mm-lantiq.c:  ret =
> of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
> drivers/gpio/gpio-mpc5200.c:    ret =
> of_mm_gpiochip_add(ofdev->dev.of_node, &chip->mmchip);
> drivers/gpio/gpio-mpc5200.c:    ret =
> of_mm_gpiochip_add(ofdev->dev.of_node, &chip->mmchip);
> drivers/gpio/gpio-mpc8xxx.c:    ret = of_mm_gpiochip_add(np, mm_gc);
> drivers/gpio/gpio-xilinx.c:     status = of_mm_gpiochip_add(np, &chip->mmchip);
> drivers/gpio/gpio-zevio.c:      status =
> of_mm_gpiochip_add(pdev->dev.of_node, &(controller->chip));
>
> Should these all be patched to use this, or reviewed for applicability?
>
> Yours,
> Linus Walleij



-- 
Ricardo Ribalda

Re: [PATCH v5 6/7] gpio/gpiolib-of: Create of_mm_gpiochip_remove

[Linus Walleij]

On Mon, Jan 12, 2015 at 10:31 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> I think that only gpio-mpc5200.c would benefit from the new API, I can
> send a patchset to support removal and use the new API if you want.

That would be awesome.

Yours,
Linus Walleij

[PATCH v5 7/7] gpio/xilinx: Use of_mm_gpiochip_remove

[Ricardo Ribalda Delgado]

Use the newly created of_mm_gpiochip_remove function for cleaning up
of_mm_gpiochip_add

Suggested-by: Alexandre Courbot <gnurou@gmail.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/gpio/gpio-xilinx.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index e89fb42..61243d1 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -236,10 +236,7 @@ static int xgpio_remove(struct platform_device *pdev)
 {
 	struct xgpio_instance *chip = platform_get_drvdata(pdev);
 
-	gpiochip_remove(&chip->mmchip.gc);
-
-	iounmap(chip->mmchip.regs);
-	kfree(chip->mmchip.gc.label);
+	of_mm_gpiochip_remove(&chip->mmchip);
 
 	return 0;
 }
-- 
2.1.3

Re: [PATCH v5 7/7] gpio/xilinx: Use of_mm_gpiochip_remove

[Linus Walleij]

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> Use the newly created of_mm_gpiochip_remove function for cleaning up
> of_mm_gpiochip_add
>
> Suggested-by: Alexandre Courbot <gnurou@gmail.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH v5 0/7]gpio/xilinx: Add support for PCI mapped devices and rmmod

[Ricardo Ribalda Delgado]

Hello

I am back from holidays. If there is any change that you want on this
series please share.

Thanks and happy new year :)

On Wed, Dec 17, 2014 at 4:51 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> The main purpose of this patchset is to use the driver on a PCI mapped fpga.
>
> This patchset also converts the driver to platform device, so it will support
> hot plugged devices and rmmod.
>
> Changeset:
> v5:
> Suggested by Alexandre Courbot <gnurou@gmail.com>
> -Create a single gpio chip on dual cores
> -Create new function of_mm_gpiochip_remove
>
> v4:
> Suggested by Michal Simek <michal.simek@xilinx.com>
> -Add missing module_exit
>
> v3
> Suggested by Michal Simek <michal.simek@xilinx.com>
> -Add patch for fixing kernel-doc
> -Leave the driver at subsys_initcall
>
>
> v2
> Suggested by Alexandre Courbot <gnurou@gmail.com>
> Merge 2 and 3 (convert to platform device and implement remove)
>
> Ricardo Ribalda Delgado (7):
>   gpio/xilinx: Remove offset property
>   gpio/xilinx: Convert the driver to platform device interface
>   gpio/xilinx: Add support for X86 Arch
>   gpio/xilinx: Fix kernel-doc
>   gpio/xilinx: Create a single gpio chip on dual cores
>   gpio/gpiolib-of: Create of_mm_gpiochip_remove
>   gpio/xilinx: Use of_mm_gpiochip_remove
>
>  drivers/gpio/Kconfig       |   4 +-
>  drivers/gpio/gpio-xilinx.c | 284 ++++++++++++++++++++++++++-------------------
>  drivers/gpio/gpiolib-of.c  |  17 +++
>  include/linux/of_gpio.h    |   1 +
>  4 files changed, 182 insertions(+), 124 deletions(-)
>
> --
> 2.1.3
>



-- 
Ricardo Ribalda

Re: [PATCH v5 0/7]gpio/xilinx: Add support for PCI mapped devices and rmmod

[Linus Walleij]

On Thu, Jan 1, 2015 at 4:27 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> I am back from holidays. If there is any change that you want on this
> series please share.

Noone says anything so I applied them all.
Looks good to me anyway.

Check the question of whether we should audit other drivers to
use of_mm_gpiochip_remove().

Yours,
Linus Walleij