[PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[Jiang Qiu]

This patchset adds gpio-signaled acpi events support for power button on hisilicon
D02 board.

The three patches respectively:
        - remove name from dwapb_port_property
	- convert device node to fwnode
        - add gpio-signaled acpi event support

   This patchset is based on
   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
   branch "devel"

Changes v8 -> v9:
   - fixed a low-level compile warning

Changes v7 -> v8:
   - fixed few minors

Changes v6 -> v7:
   - add patch1 by Alan's suggestion

Changes v5 -> v6:
   - merge patch 2~3 to one patch
   - small fixed from Alan's suggestion
   - fixed subject title reference commit history

Changes v4 -> v5:
   - split into three patchs
   - add Andy's ACKs
   
Changes v3 -> v4:
   - re-organize this two patchs by Andy's suggestion

Changes v2 -> v3:
   - fixed the build error reported by Kbuild test robot

Changes v1 -> v2: 
   - rebase to branch "devel" of Linus Walleij's repository
   - split in two patch as suggested by Andy S
   - add Mika's ACKs

Jiang Qiu (3):
  gpio: dwapb: remove name from dwapb_port_property
  gpio: dwapb: convert device node to fwnode
  gpio: dwapb: add gpio-signaled acpi event support

 drivers/gpio/gpio-dwapb.c                | 78 +++++++++++++++++++-------------
 drivers/mfd/intel_quark_i2c_gpio.c       |  3 +-
 include/linux/platform_data/gpio-dwapb.h |  3 +-
 3 files changed, 48 insertions(+), 36 deletions(-)

-- 
1.9.1

[PATCH v9 1/3] gpio: dwapb: remove name from dwapb_port_property

[Jiang Qiu]

This patch removed the name property from dwapb_port_property.
The name property is redundant, since we can get this info
from dwapb_gpio dev node.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jiang Qiu <qiujiang@huawei.com>
---
 drivers/gpio/gpio-dwapb.c                | 24 +++++++++++-------------
 drivers/mfd/intel_quark_i2c_gpio.c       |  1 -
 include/linux/platform_data/gpio-dwapb.h |  1 -
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 597de1e..772d743 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -409,8 +409,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 	err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
 			 NULL, false);
 	if (err) {
-		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
-			pp->name);
+		dev_err(gpio->dev, "failed to init gpio chip for port%d\n",
+			port->idx);
 		return err;
 	}
 
@@ -429,8 +429,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 
 	err = gpiochip_add_data(&port->gc, port);
 	if (err)
-		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
-			pp->name);
+		dev_err(gpio->dev, "failed to register gpiochip for port%d\n",
+			port->idx);
 	else
 		port->is_registered = true;
 
@@ -480,15 +480,16 @@ dwapb_gpio_get_pdata_of(struct device *dev)
 
 		if (of_property_read_u32(port_np, "reg", &pp->idx) ||
 		    pp->idx >= DWAPB_MAX_PORTS) {
-			dev_err(dev, "missing/invalid port index for %s\n",
-				port_np->full_name);
+			dev_err(dev,
+				"missing/invalid port index for port%d\n", i);
 			return ERR_PTR(-EINVAL);
 		}
 
 		if (of_property_read_u32(port_np, "snps,nr-gpios",
 					 &pp->ngpio)) {
-			dev_info(dev, "failed to get number of gpios for %s\n",
-				 port_np->full_name);
+			dev_info(dev,
+				 "failed to get number of gpios for port%d\n",
+				 i);
 			pp->ngpio = 32;
 		}
 
@@ -499,15 +500,12 @@ dwapb_gpio_get_pdata_of(struct device *dev)
 		if (pp->idx == 0 &&
 		    of_property_read_bool(port_np, "interrupt-controller")) {
 			pp->irq = irq_of_parse_and_map(port_np, 0);
-			if (!pp->irq) {
-				dev_warn(dev, "no irq for bank %s\n",
-					 port_np->full_name);
-			}
+			if (!pp->irq)
+				dev_warn(dev, "no irq for port%d\n", pp->idx);
 		}
 
 		pp->irq_shared	= false;
 		pp->gpio_base	= -1;
-		pp->name	= port_np->full_name;
 	}
 
 	return pdata;
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index bdc5e27..a4ef99b 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -220,7 +220,6 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 
 	/* Set the properties for portA */
 	pdata->properties->node		= NULL;
-	pdata->properties->name		= "intel-quark-x1000-gpio-portA";
 	pdata->properties->idx		= 0;
 	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
 	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
index 28702c8..955b579 100644
--- a/include/linux/platform_data/gpio-dwapb.h
+++ b/include/linux/platform_data/gpio-dwapb.h
@@ -16,7 +16,6 @@
 
 struct dwapb_port_property {
 	struct device_node *node;
-	const char	*name;
 	unsigned int	idx;
 	unsigned int	ngpio;
 	unsigned int	gpio_base;
-- 
1.9.1

[PATCH v9 2/3] gpio: dwapb: convert device node to fwnode

[Jiang Qiu]

This patch converts device node to fwnode for dwapb driver, so
as to provide a unified fwnode for DT and ACPI bindings.

Tested-by: Alan Tull <delicious.quinoa@gmail.com>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jiang Qiu <qiujiang@huawei.com>
---
 drivers/gpio/gpio-dwapb.c                | 37 ++++++++++++++++----------------
 drivers/mfd/intel_quark_i2c_gpio.c       |  2 +-
 include/linux/platform_data/gpio-dwapb.h |  2 +-
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 772d743..92bc204 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -22,6 +22,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 #include <linux/platform_data/gpio-dwapb.h>
 #include <linux/slab.h>
@@ -290,14 +291,14 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 				 struct dwapb_port_property *pp)
 {
 	struct gpio_chip *gc = &port->gc;
-	struct device_node *node = pp->node;
+	struct fwnode_handle  *fwnode = pp->fwnode;
 	struct irq_chip_generic	*irq_gc = NULL;
 	unsigned int hwirq, ngpio = gc->ngpio;
 	struct irq_chip_type *ct;
 	int err, i;
 
-	gpio->domain = irq_domain_add_linear(node, ngpio,
-					     &irq_generic_chip_ops, gpio);
+	gpio->domain = irq_domain_create_linear(fwnode, ngpio,
+						 &irq_generic_chip_ops, gpio);
 	if (!gpio->domain)
 		return;
 
@@ -415,7 +416,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 	}
 
 #ifdef CONFIG_OF_GPIO
-	port->gc.of_node = pp->node;
+	port->gc.of_node = is_of_node(pp->fwnode) ?
+		to_of_node(pp->fwnode) : NULL;
 #endif
 	port->gc.ngpio = pp->ngpio;
 	port->gc.base = pp->gpio_base;
@@ -447,19 +449,15 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
 }
 
 static struct dwapb_platform_data *
-dwapb_gpio_get_pdata_of(struct device *dev)
+dwapb_gpio_get_pdata(struct device *dev)
 {
-	struct device_node *node, *port_np;
+	struct fwnode_handle *fwnode;
 	struct dwapb_platform_data *pdata;
 	struct dwapb_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);
+	nports = device_get_child_node_count(dev);
 	if (nports == 0)
 		return ERR_PTR(-ENODEV);
 
@@ -474,18 +472,18 @@ dwapb_gpio_get_pdata_of(struct device *dev)
 	pdata->nports = nports;
 
 	i = 0;
-	for_each_child_of_node(node, port_np) {
+	device_for_each_child_node(dev, fwnode)  {
 		pp = &pdata->properties[i++];
-		pp->node = port_np;
+		pp->fwnode = fwnode;
 
-		if (of_property_read_u32(port_np, "reg", &pp->idx) ||
+		if (fwnode_property_read_u32(fwnode, "reg", &pp->idx) ||
 		    pp->idx >= DWAPB_MAX_PORTS) {
 			dev_err(dev,
 				"missing/invalid port index for port%d\n", i);
 			return ERR_PTR(-EINVAL);
 		}
 
-		if (of_property_read_u32(port_np, "snps,nr-gpios",
+		if (fwnode_property_read_u32(fwnode, "snps,nr-gpios",
 					 &pp->ngpio)) {
 			dev_info(dev,
 				 "failed to get number of gpios for port%d\n",
@@ -497,9 +495,10 @@ dwapb_gpio_get_pdata_of(struct device *dev)
 		 * Only port A can provide interrupts in all configurations of
 		 * the IP.
 		 */
-		if (pp->idx == 0 &&
-		    of_property_read_bool(port_np, "interrupt-controller")) {
-			pp->irq = irq_of_parse_and_map(port_np, 0);
+		if (dev->of_node && pp->idx == 0 &&
+			fwnode_property_read_bool(fwnode,
+						  "interrupt-controller")) {
+			pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0);
 			if (!pp->irq)
 				dev_warn(dev, "no irq for port%d\n", pp->idx);
 		}
@@ -521,7 +520,7 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
 	struct dwapb_platform_data *pdata = dev_get_platdata(dev);
 
 	if (!pdata) {
-		pdata = dwapb_gpio_get_pdata_of(dev);
+		pdata = dwapb_gpio_get_pdata(dev);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
 	}
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index a4ef99b..a24b35f 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -219,7 +219,7 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 		return -ENOMEM;
 
 	/* Set the properties for portA */
-	pdata->properties->node		= NULL;
+	pdata->properties->fwnode	= NULL;
 	pdata->properties->idx		= 0;
 	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
 	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
index 955b579..2dc7f4a 100644
--- a/include/linux/platform_data/gpio-dwapb.h
+++ b/include/linux/platform_data/gpio-dwapb.h
@@ -15,7 +15,7 @@
 #define GPIO_DW_APB_H
 
 struct dwapb_port_property {
-	struct device_node *node;
+	struct fwnode_handle *fwnode;
 	unsigned int	idx;
 	unsigned int	ngpio;
 	unsigned int	gpio_base;
-- 
1.9.1

[PATCH v9 3/3] gpio: dwapb: add gpio-signaled acpi event support

[Jiang Qiu]

This patch adds gpio-signaled acpi event support. It is used for
power button on hisilicon D02 board, an arm64 platform.

The corresponding DSDT file is defined as follows:
Device(GPI0) {
	Name(_HID, "HISI0181")
	Name(_ADR, 0)
	Name(_UID, 0)

	Name (_CRS, ResourceTemplate ()  {
		Memory32Fixed (ReadWrite, 0x802e0000, 0x10000)
		Interrupt (ResourceConsumer, Level, ActiveHigh,
		Exclusive,,,)  {344}
	})

	Device(PRTa) {
		Name (_DSD, Package () {
		Package () {
			Package () {"reg",0},
			Package () {"snps,nr-gpios",32},
		}
		})
	}

	Name (_AEI, ResourceTemplate () {
		GpioInt(Edge, ActiveLow, ExclusiveAndWake,
		PullUp, , " \\_SB.GPI0") {8}
	})

	Method (_E08, 0x0, NotSerialized) {
		Notify (\_SB.PWRB, 0x80)
	}
}

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jiang Qiu <qiujiang@huawei.com>
---
 drivers/gpio/gpio-dwapb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 92bc204..c411e95 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -7,6 +7,7 @@
  *
  * All enquiries to support@picochip.com
  */
+#include <linux/acpi.h>
 #include <linux/gpio/driver.h>
 /* FIXME: for gpio_get_value(), replace this with direct register read */
 #include <linux/gpio.h>
@@ -27,6 +28,8 @@
 #include <linux/platform_data/gpio-dwapb.h>
 #include <linux/slab.h>
 
+#include "gpiolib.h"
+
 #define GPIO_SWPORTA_DR		0x00
 #define GPIO_SWPORTA_DDR	0x04
 #define GPIO_SWPORTB_DR		0x0c
@@ -436,6 +439,10 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 	else
 		port->is_registered = true;
 
+	/* Add GPIO-signaled ACPI event support */
+	if (pp->irq)
+		acpi_gpiochip_request_interrupts(&port->gc);
+
 	return err;
 }
 
@@ -503,6 +510,9 @@ dwapb_gpio_get_pdata(struct device *dev)
 				dev_warn(dev, "no irq for port%d\n", pp->idx);
 		}
 
+		if (has_acpi_companion(dev) && pp->idx == 0)
+			pp->irq = platform_get_irq(to_platform_device(dev), 0);
+
 		pp->irq_shared	= false;
 		pp->gpio_base	= -1;
 	}
@@ -577,6 +587,12 @@ static const struct of_device_id dwapb_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, dwapb_of_match);
 
+static const struct acpi_device_id dwapb_acpi_match[] = {
+	{"HISI0181", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, dwapb_acpi_match);
+
 #ifdef CONFIG_PM_SLEEP
 static int dwapb_gpio_suspend(struct device *dev)
 {
@@ -671,6 +687,7 @@ static struct platform_driver dwapb_gpio_driver = {
 		.name	= "gpio-dwapb",
 		.pm	= &dwapb_gpio_pm_ops,
 		.of_match_table = of_match_ptr(dwapb_of_match),
+		.acpi_match_table = ACPI_PTR(dwapb_acpi_match),
 	},
 	.probe		= dwapb_gpio_probe,
 	.remove		= dwapb_gpio_remove,
-- 
1.9.1

Re: [PATCH v9 2/3] gpio: dwapb: convert device node to fwnode

[Andy Shevchenko]

On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
> This patch converts device node to fwnode for dwapb driver, so
> as to provide a unified fwnode for DT and ACPI bindings.
>
> Tested-by: Alan Tull <delicious.quinoa@gmail.com>
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jiang Qiu <qiujiang@huawei.com>
> ---
>  drivers/gpio/gpio-dwapb.c                | 37 ++++++++++++++++----------------
>  drivers/mfd/intel_quark_i2c_gpio.c       |  2 +-
>  include/linux/platform_data/gpio-dwapb.h |  2 +-
>  3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 772d743..92bc204 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -22,6 +22,7 @@
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/spinlock.h>
>  #include <linux/platform_data/gpio-dwapb.h>
>  #include <linux/slab.h>
> @@ -290,14 +291,14 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>                                  struct dwapb_port_property *pp)
>  {
>         struct gpio_chip *gc = &port->gc;
> -       struct device_node *node = pp->node;
> +       struct fwnode_handle  *fwnode = pp->fwnode;
>         struct irq_chip_generic *irq_gc = NULL;
>         unsigned int hwirq, ngpio = gc->ngpio;
>         struct irq_chip_type *ct;
>         int err, i;
>
> -       gpio->domain = irq_domain_add_linear(node, ngpio,
> -                                            &irq_generic_chip_ops, gpio);
> +       gpio->domain = irq_domain_create_linear(fwnode, ngpio,
> +                                                &irq_generic_chip_ops, gpio);
>         if (!gpio->domain)
>                 return;
>
> @@ -415,7 +416,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>         }
>
>  #ifdef CONFIG_OF_GPIO
> -       port->gc.of_node = pp->node;
> +       port->gc.of_node = is_of_node(pp->fwnode) ?
> +               to_of_node(pp->fwnode) : NULL;

Someone commented on this that it's simple
…of_node = to_of_node(…);

>  #endif
>         port->gc.ngpio = pp->ngpio;
>         port->gc.base = pp->gpio_base;
> @@ -447,19 +449,15 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
>  }
>
>  static struct dwapb_platform_data *
> -dwapb_gpio_get_pdata_of(struct device *dev)
> +dwapb_gpio_get_pdata(struct device *dev)
>  {
> -       struct device_node *node, *port_np;
> +       struct fwnode_handle *fwnode;
>         struct dwapb_platform_data *pdata;
>         struct dwapb_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);
> +       nports = device_get_child_node_count(dev);
>         if (nports == 0)
>                 return ERR_PTR(-ENODEV);
>
> @@ -474,18 +472,18 @@ dwapb_gpio_get_pdata_of(struct device *dev)
>         pdata->nports = nports;
>
>         i = 0;
> -       for_each_child_of_node(node, port_np) {
> +       device_for_each_child_node(dev, fwnode)  {
>                 pp = &pdata->properties[i++];
> -               pp->node = port_np;
> +               pp->fwnode = fwnode;
>
> -               if (of_property_read_u32(port_np, "reg", &pp->idx) ||
> +               if (fwnode_property_read_u32(fwnode, "reg", &pp->idx) ||
>                     pp->idx >= DWAPB_MAX_PORTS) {
>                         dev_err(dev,
>                                 "missing/invalid port index for port%d\n", i);
>                         return ERR_PTR(-EINVAL);
>                 }
>
> -               if (of_property_read_u32(port_np, "snps,nr-gpios",
> +               if (fwnode_property_read_u32(fwnode, "snps,nr-gpios",
>                                          &pp->ngpio)) {
>                         dev_info(dev,
>                                  "failed to get number of gpios for port%d\n",
> @@ -497,9 +495,10 @@ dwapb_gpio_get_pdata_of(struct device *dev)
>                  * Only port A can provide interrupts in all configurations of
>                  * the IP.
>                  */
> -               if (pp->idx == 0 &&
> -                   of_property_read_bool(port_np, "interrupt-controller")) {
> -                       pp->irq = irq_of_parse_and_map(port_np, 0);
> +               if (dev->of_node && pp->idx == 0 &&
> +                       fwnode_property_read_bool(fwnode,
> +                                                 "interrupt-controller")) {
> +                       pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0);
>                         if (!pp->irq)
>                                 dev_warn(dev, "no irq for port%d\n", pp->idx);
>                 }
> @@ -521,7 +520,7 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
>         struct dwapb_platform_data *pdata = dev_get_platdata(dev);
>
>         if (!pdata) {
> -               pdata = dwapb_gpio_get_pdata_of(dev);
> +               pdata = dwapb_gpio_get_pdata(dev);
>                 if (IS_ERR(pdata))
>                         return PTR_ERR(pdata);
>         }
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index a4ef99b..a24b35f 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -219,7 +219,7 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
>                 return -ENOMEM;
>
>         /* Set the properties for portA */
> -       pdata->properties->node         = NULL;
> +       pdata->properties->fwnode       = NULL;
>         pdata->properties->idx          = 0;
>         pdata->properties->ngpio        = INTEL_QUARK_MFD_NGPIO;
>         pdata->properties->gpio_base    = INTEL_QUARK_MFD_GPIO_BASE;
> diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
> index 955b579..2dc7f4a 100644
> --- a/include/linux/platform_data/gpio-dwapb.h
> +++ b/include/linux/platform_data/gpio-dwapb.h
> @@ -15,7 +15,7 @@
>  #define GPIO_DW_APB_H
>
>  struct dwapb_port_property {
> -       struct device_node *node;
> +       struct fwnode_handle *fwnode;
>         unsigned int    idx;
>         unsigned int    ngpio;
>         unsigned int    gpio_base;
> --
> 1.9.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[Andy Shevchenko]

On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
> This patchset adds gpio-signaled acpi events support for power button on hisilicon
> D02 board.
>
> The three patches respectively:
>         - remove name from dwapb_port_property
>         - convert device node to fwnode
>         - add gpio-signaled acpi event support
>
>    This patchset is based on
>    https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
>    branch "devel"
>
> Changes v8 -> v9:
>    - fixed a low-level compile warning

Please, don't send updates to often. You see, now I noticed one more
thing you have to address.
Usually the time between fixup series is something like 24h. Thus,
waiting for v10 at least tomorrow or even next week.

Linus, for my point of view next version should be fine and final. The
current code is in a good shape already.

>
> Changes v7 -> v8:
>    - fixed few minors
>
> Changes v6 -> v7:
>    - add patch1 by Alan's suggestion
>
> Changes v5 -> v6:
>    - merge patch 2~3 to one patch
>    - small fixed from Alan's suggestion
>    - fixed subject title reference commit history
>
> Changes v4 -> v5:
>    - split into three patchs
>    - add Andy's ACKs
>
> Changes v3 -> v4:
>    - re-organize this two patchs by Andy's suggestion
>
> Changes v2 -> v3:
>    - fixed the build error reported by Kbuild test robot
>
> Changes v1 -> v2:
>    - rebase to branch "devel" of Linus Walleij's repository
>    - split in two patch as suggested by Andy S
>    - add Mika's ACKs
>
> Jiang Qiu (3):
>   gpio: dwapb: remove name from dwapb_port_property
>   gpio: dwapb: convert device node to fwnode
>   gpio: dwapb: add gpio-signaled acpi event support
>
>  drivers/gpio/gpio-dwapb.c                | 78 +++++++++++++++++++-------------
>  drivers/mfd/intel_quark_i2c_gpio.c       |  3 +-
>  include/linux/platform_data/gpio-dwapb.h |  3 +-
>  3 files changed, 48 insertions(+), 36 deletions(-)
>
> --
> 1.9.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[Alan Tull]

On Wed, Apr 20, 2016 at 8:06 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
>> This patchset adds gpio-signaled acpi events support for power button on hisilicon
>> D02 board.
>>
>> The three patches respectively:
>>         - remove name from dwapb_port_property
>>         - convert device node to fwnode
>>         - add gpio-signaled acpi event support
>>
>>    This patchset is based on
>>    https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
>>    branch "devel"
>>
>> Changes v8 -> v9:
>>    - fixed a low-level compile warning
>
> Please, don't send updates to often. You see, now I noticed one more
> thing you have to address.
> Usually the time between fixup series is something like 24h. Thus,
> waiting for v10 at least tomorrow or even next week.
>
> Linus, for my point of view next version should be fine and final. The
> current code is in a good shape already.

Reviewed, tested v9, looks good.

Alan Tull


>
>>
>> Changes v7 -> v8:
>>    - fixed few minors
>>
>> Changes v6 -> v7:
>>    - add patch1 by Alan's suggestion
>>
>> Changes v5 -> v6:
>>    - merge patch 2~3 to one patch
>>    - small fixed from Alan's suggestion
>>    - fixed subject title reference commit history
>>
>> Changes v4 -> v5:
>>    - split into three patchs
>>    - add Andy's ACKs
>>
>> Changes v3 -> v4:
>>    - re-organize this two patchs by Andy's suggestion
>>
>> Changes v2 -> v3:
>>    - fixed the build error reported by Kbuild test robot
>>
>> Changes v1 -> v2:
>>    - rebase to branch "devel" of Linus Walleij's repository
>>    - split in two patch as suggested by Andy S
>>    - add Mika's ACKs
>>
>> Jiang Qiu (3):
>>   gpio: dwapb: remove name from dwapb_port_property
>>   gpio: dwapb: convert device node to fwnode
>>   gpio: dwapb: add gpio-signaled acpi event support
>>
>>  drivers/gpio/gpio-dwapb.c                | 78 +++++++++++++++++++-------------
>>  drivers/mfd/intel_quark_i2c_gpio.c       |  3 +-
>>  include/linux/platform_data/gpio-dwapb.h |  3 +-
>>  3 files changed, 48 insertions(+), 36 deletions(-)
>>
>> --
>> 1.9.1
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v9 2/3] gpio: dwapb: convert device node to fwnode

[Jiang Qiu]

在 2016/4/20 21:04, Andy Shevchenko 写道:
> On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
>> This patch converts device node to fwnode for dwapb driver, so
>> as to provide a unified fwnode for DT and ACPI bindings.
>>
>> Tested-by: Alan Tull <delicious.quinoa@gmail.com>
>> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Jiang Qiu <qiujiang@huawei.com>
>> ---
>>  drivers/gpio/gpio-dwapb.c                | 37 ++++++++++++++++----------------
>>  drivers/mfd/intel_quark_i2c_gpio.c       |  2 +-
>>  include/linux/platform_data/gpio-dwapb.h |  2 +-
>>  3 files changed, 20 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
>> index 772d743..92bc204 100644
>> --- a/drivers/gpio/gpio-dwapb.c
>> +++ b/drivers/gpio/gpio-dwapb.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/property.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/platform_data/gpio-dwapb.h>
>>  #include <linux/slab.h>
>> @@ -290,14 +291,14 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>>                                  struct dwapb_port_property *pp)
>>  {
>>         struct gpio_chip *gc = &port->gc;
>> -       struct device_node *node = pp->node;
>> +       struct fwnode_handle  *fwnode = pp->fwnode;
>>         struct irq_chip_generic *irq_gc = NULL;
>>         unsigned int hwirq, ngpio = gc->ngpio;
>>         struct irq_chip_type *ct;
>>         int err, i;
>>
>> -       gpio->domain = irq_domain_add_linear(node, ngpio,
>> -                                            &irq_generic_chip_ops, gpio);
>> +       gpio->domain = irq_domain_create_linear(fwnode, ngpio,
>> +                                                &irq_generic_chip_ops, gpio);
>>         if (!gpio->domain)
>>                 return;
>>
>> @@ -415,7 +416,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>>         }
>>
>>  #ifdef CONFIG_OF_GPIO
>> -       port->gc.of_node = pp->node;
>> +       port->gc.of_node = is_of_node(pp->fwnode) ?
>> +               to_of_node(pp->fwnode) : NULL;
> 
> Someone commented on this that it's simple
> …of_node = to_of_node(…);
>
Yes, the to_of_node() function has checked as like above, as follow:

static inline struct device_node *to_of_node(struct fwnode_handle *fwnode)
{
	return is_of_node(fwnode) ?
		container_of(fwnode, struct device_node, fwnode) : NULL;
}

Next version, I will fixed it.

>>  #endif
>>         port->gc.ngpio = pp->ngpio;
>>         port->gc.base = pp->gpio_base;
>> @@ -447,19 +449,15 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
>>  }
>>
>>  static struct dwapb_platform_data *
>> -dwapb_gpio_get_pdata_of(struct device *dev)
>> +dwapb_gpio_get_pdata(struct device *dev)
>>  {
>> -       struct device_node *node, *port_np;
>> +       struct fwnode_handle *fwnode;
>>         struct dwapb_platform_data *pdata;
>>         struct dwapb_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);
>> +       nports = device_get_child_node_count(dev);
>>         if (nports == 0)
>>                 return ERR_PTR(-ENODEV);
>>
>> @@ -474,18 +472,18 @@ dwapb_gpio_get_pdata_of(struct device *dev)
>>         pdata->nports = nports;
>>
>>         i = 0;
>> -       for_each_child_of_node(node, port_np) {
>> +       device_for_each_child_node(dev, fwnode)  {
>>                 pp = &pdata->properties[i++];
>> -               pp->node = port_np;
>> +               pp->fwnode = fwnode;
>>
>> -               if (of_property_read_u32(port_np, "reg", &pp->idx) ||
>> +               if (fwnode_property_read_u32(fwnode, "reg", &pp->idx) ||
>>                     pp->idx >= DWAPB_MAX_PORTS) {
>>                         dev_err(dev,
>>                                 "missing/invalid port index for port%d\n", i);
>>                         return ERR_PTR(-EINVAL);
>>                 }
>>
>> -               if (of_property_read_u32(port_np, "snps,nr-gpios",
>> +               if (fwnode_property_read_u32(fwnode, "snps,nr-gpios",
>>                                          &pp->ngpio)) {
>>                         dev_info(dev,
>>                                  "failed to get number of gpios for port%d\n",
>> @@ -497,9 +495,10 @@ dwapb_gpio_get_pdata_of(struct device *dev)
>>                  * Only port A can provide interrupts in all configurations of
>>                  * the IP.
>>                  */
>> -               if (pp->idx == 0 &&
>> -                   of_property_read_bool(port_np, "interrupt-controller")) {
>> -                       pp->irq = irq_of_parse_and_map(port_np, 0);
>> +               if (dev->of_node && pp->idx == 0 &&
>> +                       fwnode_property_read_bool(fwnode,
>> +                                                 "interrupt-controller")) {
>> +                       pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0);
>>                         if (!pp->irq)
>>                                 dev_warn(dev, "no irq for port%d\n", pp->idx);
>>                 }
>> @@ -521,7 +520,7 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
>>         struct dwapb_platform_data *pdata = dev_get_platdata(dev);
>>
>>         if (!pdata) {
>> -               pdata = dwapb_gpio_get_pdata_of(dev);
>> +               pdata = dwapb_gpio_get_pdata(dev);
>>                 if (IS_ERR(pdata))
>>                         return PTR_ERR(pdata);
>>         }
>> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
>> index a4ef99b..a24b35f 100644
>> --- a/drivers/mfd/intel_quark_i2c_gpio.c
>> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
>> @@ -219,7 +219,7 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
>>                 return -ENOMEM;
>>
>>         /* Set the properties for portA */
>> -       pdata->properties->node         = NULL;
>> +       pdata->properties->fwnode       = NULL;
>>         pdata->properties->idx          = 0;
>>         pdata->properties->ngpio        = INTEL_QUARK_MFD_NGPIO;
>>         pdata->properties->gpio_base    = INTEL_QUARK_MFD_GPIO_BASE;
>> diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
>> index 955b579..2dc7f4a 100644
>> --- a/include/linux/platform_data/gpio-dwapb.h
>> +++ b/include/linux/platform_data/gpio-dwapb.h
>> @@ -15,7 +15,7 @@
>>  #define GPIO_DW_APB_H
>>
>>  struct dwapb_port_property {
>> -       struct device_node *node;
>> +       struct fwnode_handle *fwnode;
>>         unsigned int    idx;
>>         unsigned int    ngpio;
>>         unsigned int    gpio_base;
>> --
>> 1.9.1
>>
> 
> 
> 

Re: [PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[Jiang Qiu]

在 2016/4/20 21:06, Andy Shevchenko 写道:
> On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
>> This patchset adds gpio-signaled acpi events support for power button on hisilicon
>> D02 board.
>>
>> The three patches respectively:
>>         - remove name from dwapb_port_property
>>         - convert device node to fwnode
>>         - add gpio-signaled acpi event support
>>
>>    This patchset is based on
>>    https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
>>    branch "devel"
>>
>> Changes v8 -> v9:
>>    - fixed a low-level compile warning
> 
> Please, don't send updates to often. You see, now I noticed one more
> thing you have to address.
> Usually the time between fixup series is something like 24h. Thus,
> waiting for v10 at least tomorrow or even next week.
> 
Thanks for the reminder. I made a mistake when I finished my test and merge
the last code because my careless. I'm too anxious to correct it :)

> Linus, for my point of view next version should be fine and final. The
> current code is in a good shape already.
> 
>>
>> Changes v7 -> v8:
>>    - fixed few minors
>>
>> Changes v6 -> v7:
>>    - add patch1 by Alan's suggestion
>>
>> Changes v5 -> v6:
>>    - merge patch 2~3 to one patch
>>    - small fixed from Alan's suggestion
>>    - fixed subject title reference commit history
>>
>> Changes v4 -> v5:
>>    - split into three patchs
>>    - add Andy's ACKs
>>
>> Changes v3 -> v4:
>>    - re-organize this two patchs by Andy's suggestion
>>
>> Changes v2 -> v3:
>>    - fixed the build error reported by Kbuild test robot
>>
>> Changes v1 -> v2:
>>    - rebase to branch "devel" of Linus Walleij's repository
>>    - split in two patch as suggested by Andy S
>>    - add Mika's ACKs
>>
>> Jiang Qiu (3):
>>   gpio: dwapb: remove name from dwapb_port_property
>>   gpio: dwapb: convert device node to fwnode
>>   gpio: dwapb: add gpio-signaled acpi event support
>>
>>  drivers/gpio/gpio-dwapb.c                | 78 +++++++++++++++++++-------------
>>  drivers/mfd/intel_quark_i2c_gpio.c       |  3 +-
>>  include/linux/platform_data/gpio-dwapb.h |  3 +-
>>  3 files changed, 48 insertions(+), 36 deletions(-)
>>
>> --
>> 1.9.1
>>
> 
> 
> 

Re: [PATCH v9 0/3] gpio: dwapb: add gpio-signaled acpi event support for power button

[Jiang Qiu]

在 2016/4/21 0:10, Alan Tull 写道:
> On Wed, Apr 20, 2016 at 8:06 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Wed, Apr 20, 2016 at 10:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:
>>> This patchset adds gpio-signaled acpi events support for power button on hisilicon
>>> D02 board.
>>>
>>> The three patches respectively:
>>>         - remove name from dwapb_port_property
>>>         - convert device node to fwnode
>>>         - add gpio-signaled acpi event support
>>>
>>>    This patchset is based on
>>>    https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
>>>    branch "devel"
>>>
>>> Changes v8 -> v9:
>>>    - fixed a low-level compile warning
>>
>> Please, don't send updates to often. You see, now I noticed one more
>> thing you have to address.
>> Usually the time between fixup series is something like 24h. Thus,
>> waiting for v10 at least tomorrow or even next week.
>>
>> Linus, for my point of view next version should be fine and final. The
>> current code is in a good shape already.
> 
> Reviewed, tested v9, looks good.
> 
> Alan Tull
> 
Alan, Thanks a lot. :)
> 
>>
>>>
>>> Changes v7 -> v8:
>>>    - fixed few minors
>>>
>>> Changes v6 -> v7:
>>>    - add patch1 by Alan's suggestion
>>>
>>> Changes v5 -> v6:
>>>    - merge patch 2~3 to one patch
>>>    - small fixed from Alan's suggestion
>>>    - fixed subject title reference commit history
>>>
>>> Changes v4 -> v5:
>>>    - split into three patchs
>>>    - add Andy's ACKs
>>>
>>> Changes v3 -> v4:
>>>    - re-organize this two patchs by Andy's suggestion
>>>
>>> Changes v2 -> v3:
>>>    - fixed the build error reported by Kbuild test robot
>>>
>>> Changes v1 -> v2:
>>>    - rebase to branch "devel" of Linus Walleij's repository
>>>    - split in two patch as suggested by Andy S
>>>    - add Mika's ACKs
>>>
>>> Jiang Qiu (3):
>>>   gpio: dwapb: remove name from dwapb_port_property
>>>   gpio: dwapb: convert device node to fwnode
>>>   gpio: dwapb: add gpio-signaled acpi event support
>>>
>>>  drivers/gpio/gpio-dwapb.c                | 78 +++++++++++++++++++-------------
>>>  drivers/mfd/intel_quark_i2c_gpio.c       |  3 +-
>>>  include/linux/platform_data/gpio-dwapb.h |  3 +-
>>>  3 files changed, 48 insertions(+), 36 deletions(-)
>>>
>>> --
>>> 1.9.1
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
> 
> .
> 

Re: [PATCH v9 1/3] gpio: dwapb: remove name from dwapb_port_property

[Linus Walleij]

On Wed, Apr 20, 2016 at 9:13 AM, Jiang Qiu <qiujiang@huawei.com> wrote:

> This patch removed the name property from dwapb_port_property.
> The name property is redundant, since we can get this info
> from dwapb_gpio dev node.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jiang Qiu <qiujiang@huawei.com>

The people patching this driver in the past didn't say anything so
OK patch applied.

Yours,
Linus Walleij