linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()
@ 2021-08-04 16:00 Andy Shevchenko
  2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-04 16:00 UTC (permalink / raw)
  To: Andy Shevchenko, Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

Shared IRQ is only enabled for ACPI enumeration, there is no need
to have a special flag for that, since we simple can test if device
has been enumerated by ACPI. This unifies the checks in dwapb_get_irq()
and dwapb_configure_irqs().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Serge Semin <fancer.lancer@gmail.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: added tags (Lee, Serge), rephrased comments near to for-loop (Serge)
 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, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 3eb13d6d31ef..4c7153cb646c 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -436,21 +436,17 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 	pirq->irqchip.irq_set_wake = dwapb_irq_set_wake;
 #endif
 
-	if (!pp->irq_shared) {
-		girq->num_parents = pirq->nr_irqs;
-		girq->parents = pirq->irq;
-		girq->parent_handler_data = gpio;
-		girq->parent_handler = dwapb_irq_handler;
-	} else {
-		/* This will let us handle the parent IRQ in the driver */
+	/*
+	 * Intel ACPI-based platforms mostly have the DesignWare APB GPIO
+	 * IRQ lane shared between several devices. In that case the parental
+	 * IRQ has to be handled in the shared way so to be properly delivered
+	 * to all the connected devices.
+	 */
+	if (has_acpi_companion(gpio->dev)) {
 		girq->num_parents = 0;
 		girq->parents = NULL;
 		girq->parent_handler = NULL;
 
-		/*
-		 * Request a shared IRQ since where MFD would have devices
-		 * using the same irq pin
-		 */
 		err = devm_request_irq(gpio->dev, pp->irq[0],
 				       dwapb_irq_handler_mfd,
 				       IRQF_SHARED, DWAPB_DRIVER_NAME, gpio);
@@ -458,6 +454,11 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 			dev_err(gpio->dev, "error requesting IRQ\n");
 			goto err_kfree_pirq;
 		}
+	} else {
+		girq->num_parents = pirq->nr_irqs;
+		girq->parents = pirq->irq;
+		girq->parent_handler_data = gpio;
+		girq->parent_handler = dwapb_irq_handler;
 	}
 
 	girq->chip = &pirq->irqchip;
@@ -581,7 +582,6 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
 			pp->ngpio = DWAPB_MAX_GPIOS;
 		}
 
-		pp->irq_shared	= false;
 		pp->gpio_base	= -1;
 
 		/*
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 01935ae4e9e1..a43993e38b6e 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -227,7 +227,6 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev)
 	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
 	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
 	pdata->properties->irq[0]	= pci_irq_vector(pdev, 0);
-	pdata->properties->irq_shared	= true;
 
 	cell->platform_data = pdata;
 	cell->pdata_size = sizeof(*pdata);
diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
index 0aa5c6720259..535e5ed549d9 100644
--- a/include/linux/platform_data/gpio-dwapb.h
+++ b/include/linux/platform_data/gpio-dwapb.h
@@ -14,7 +14,6 @@ struct dwapb_port_property {
 	unsigned int	ngpio;
 	unsigned int	gpio_base;
 	int		irq[DWAPB_MAX_GPIOS];
-	bool		irq_shared;
 };
 
 struct dwapb_platform_data {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property
  2021-08-04 16:00 [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
@ 2021-08-04 16:00 ` Andy Shevchenko
  2021-08-05 10:38   ` Andy Shevchenko
  2021-08-11 12:50   ` Linus Walleij
  2021-08-04 16:00 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-04 16:00 UTC (permalink / raw)
  To: Andy Shevchenko, Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

For backward compatibility with some legacy devices introduce
a new (*) property gpio-base to read GPIO base. This will allow
further cleaning up of the driver.

*) Note, it's not new for the GPIO library since the mockup driver
   is using it already.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: added check to ensure that the property won't be used by FW (Serge)
 drivers/gpio/gpio-dwapb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 4c7153cb646c..674e91e69cc5 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -584,6 +584,10 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
 
 		pp->gpio_base	= -1;
 
+		/* For internal use only, new platforms mustn't exercise this */
+		if (is_software_node(fwnode))
+			fwnode_property_read_u32(fwnode, "gpio-base", &pp->gpio_base);
+
 		/*
 		 * Only port A can provide interrupts in all configurations of
 		 * the IP.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes
  2021-08-04 16:00 [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
  2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
@ 2021-08-04 16:00 ` Andy Shevchenko
  2021-08-05  8:02   ` Lee Jones
  2021-08-11 12:48   ` Linus Walleij
  2021-08-04 16:00 ` [PATCH v2 4/4] gpio: dwapb: Get rid of legacy platform data Andy Shevchenko
  2021-08-11 12:42 ` [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
  3 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-04 16:00 UTC (permalink / raw)
  To: Andy Shevchenko, Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

The driver can provide a software node group instead of
passing legacy platform data. This will allow to drop
the legacy platform data structures along with unifying
a child device driver to use same interface for all
property providers, i.e. Device Tree, ACPI, and board files.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
---
v2: added tag (Serge)
 drivers/mfd/intel_quark_i2c_gpio.c | 70 ++++++++++++++++--------------
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index a43993e38b6e..9b9c76bd067b 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -17,7 +17,6 @@
 #include <linux/clk-provider.h>
 #include <linux/dmi.h>
 #include <linux/i2c.h>
-#include <linux/platform_data/gpio-dwapb.h>
 #include <linux/property.h>
 
 /* PCI BAR for register base address */
@@ -28,15 +27,6 @@
 #define MFD_ACPI_MATCH_GPIO	0ULL
 #define MFD_ACPI_MATCH_I2C	1ULL
 
-/* The base GPIO number under GPIOLIB framework */
-#define INTEL_QUARK_MFD_GPIO_BASE	8
-
-/* The default number of South-Cluster GPIO on Quark. */
-#define INTEL_QUARK_MFD_NGPIO		8
-
-/* The DesignWare GPIO ports on Quark. */
-#define INTEL_QUARK_GPIO_NPORTS	1
-
 #define INTEL_QUARK_IORES_MEM	0
 #define INTEL_QUARK_IORES_IRQ	1
 
@@ -111,12 +101,38 @@ static struct resource intel_quark_gpio_res[] = {
 	[INTEL_QUARK_IORES_MEM] = {
 		.flags = IORESOURCE_MEM,
 	},
+	[INTEL_QUARK_IORES_IRQ] = {
+		.flags = IORESOURCE_IRQ,
+	},
 };
 
 static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
 	.adr = MFD_ACPI_MATCH_GPIO,
 };
 
+static const struct software_node intel_quark_gpio_controller_node = {
+	.name = "intel-quark-gpio-controller",
+};
+
+static const struct property_entry intel_quark_gpio_portA_properties[] = {
+	PROPERTY_ENTRY_U32("reg", 0),
+	PROPERTY_ENTRY_U32("snps,nr-gpios", 8),
+	PROPERTY_ENTRY_U32("gpio-base", 8),
+	{ }
+};
+
+static const struct software_node intel_quark_gpio_portA_node = {
+	.name = "portA",
+	.parent = &intel_quark_gpio_controller_node,
+	.properties = intel_quark_gpio_portA_properties,
+};
+
+static const struct software_node *intel_quark_gpio_node_group[] = {
+	&intel_quark_gpio_controller_node,
+	&intel_quark_gpio_portA_node,
+	NULL
+};
+
 static struct mfd_cell intel_quark_mfd_cells[] = {
 	[MFD_I2C_BAR] = {
 		.id = MFD_I2C_BAR,
@@ -203,34 +219,19 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev)
 {
 	struct mfd_cell *cell = &intel_quark_mfd_cells[MFD_GPIO_BAR];
 	struct resource *res = intel_quark_gpio_res;
-	struct dwapb_platform_data *pdata;
-	struct device *dev = &pdev->dev;
+	int ret;
 
 	res[INTEL_QUARK_IORES_MEM].start = pci_resource_start(pdev, MFD_GPIO_BAR);
 	res[INTEL_QUARK_IORES_MEM].end = pci_resource_end(pdev, MFD_GPIO_BAR);
 
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	/* For intel quark x1000, it has only one port: portA */
-	pdata->nports = INTEL_QUARK_GPIO_NPORTS;
-	pdata->properties = devm_kcalloc(dev, pdata->nports,
-					 sizeof(*pdata->properties),
-					 GFP_KERNEL);
-	if (!pdata->properties)
-		return -ENOMEM;
-
-	/* Set the properties for portA */
-	pdata->properties->fwnode	= NULL;
-	pdata->properties->idx		= 0;
-	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
-	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
-	pdata->properties->irq[0]	= pci_irq_vector(pdev, 0);
+	res[INTEL_QUARK_IORES_IRQ].start = pci_irq_vector(pdev, 0);
+	res[INTEL_QUARK_IORES_IRQ].end = pci_irq_vector(pdev, 0);
 
-	cell->platform_data = pdata;
-	cell->pdata_size = sizeof(*pdata);
+	ret = software_node_register_node_group(intel_quark_gpio_node_group);
+	if (ret)
+		return ret;
 
+	cell->swnode = &intel_quark_gpio_controller_node;
 	return 0;
 }
 
@@ -273,10 +274,12 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 			      ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
 			      NULL);
 	if (ret)
-		goto err_free_irq_vectors;
+		goto err_unregister_gpio_node_group;
 
 	return 0;
 
+err_unregister_gpio_node_group:
+	software_node_unregister_node_group(intel_quark_gpio_node_group);
 err_free_irq_vectors:
 	pci_free_irq_vectors(pdev);
 err_unregister_i2c_clk:
@@ -287,6 +290,7 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 static void intel_quark_mfd_remove(struct pci_dev *pdev)
 {
 	mfd_remove_devices(&pdev->dev);
+	software_node_unregister_node_group(intel_quark_gpio_node_group);
 	pci_free_irq_vectors(pdev);
 	intel_quark_unregister_i2c_clk(&pdev->dev);
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/4] gpio: dwapb: Get rid of legacy platform data
  2021-08-04 16:00 [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
  2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
  2021-08-04 16:00 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
@ 2021-08-04 16:00 ` Andy Shevchenko
  2021-08-11 12:42 ` [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
  3 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-04 16:00 UTC (permalink / raw)
  To: Andy Shevchenko, Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

Platform data is a legacy interface to supply device properties
to the driver. In this case we don't have anymore in-kernel users
for it. Just remove it for good.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Serge Semin <fancer.lancer@gmail.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
---
v2: added tags, put structures after forward declaration (Serge)
 drivers/gpio/gpio-dwapb.c                | 28 +++++++++++++++---------
 include/linux/platform_data/gpio-dwapb.h | 24 --------------------
 2 files changed, 18 insertions(+), 34 deletions(-)
 delete mode 100644 include/linux/platform_data/gpio-dwapb.h

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 674e91e69cc5..f98fa33e1679 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -16,7 +16,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/platform_data/gpio-dwapb.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/reset.h>
@@ -48,6 +47,7 @@
 
 #define DWAPB_DRIVER_NAME	"gpio-dwapb"
 #define DWAPB_MAX_PORTS		4
+#define DWAPB_MAX_GPIOS		32
 
 #define GPIO_EXT_PORT_STRIDE	0x04 /* register stride 32 bits */
 #define GPIO_SWPORT_DR_STRIDE	0x0c /* register stride 3*32 bits */
@@ -65,6 +65,19 @@
 
 struct dwapb_gpio;
 
+struct dwapb_port_property {
+	struct fwnode_handle *fwnode;
+	unsigned int idx;
+	unsigned int ngpio;
+	unsigned int gpio_base;
+	int irq[DWAPB_MAX_GPIOS];
+};
+
+struct dwapb_platform_data {
+	struct dwapb_port_property *properties;
+	unsigned int nports;
+};
+
 #ifdef CONFIG_PM_SLEEP
 /* Store GPIO context across system-wide suspend/resume transitions */
 struct dwapb_context {
@@ -674,17 +687,12 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
 	unsigned int i;
 	struct dwapb_gpio *gpio;
 	int err;
+	struct dwapb_platform_data *pdata;
 	struct device *dev = &pdev->dev;
-	struct dwapb_platform_data *pdata = dev_get_platdata(dev);
-
-	if (!pdata) {
-		pdata = dwapb_gpio_get_pdata(dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
-	}
 
-	if (!pdata->nports)
-		return -ENODEV;
+	pdata = dwapb_gpio_get_pdata(dev);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
 
 	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
 	if (!gpio)
diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
deleted file mode 100644
index 535e5ed549d9..000000000000
--- a/include/linux/platform_data/gpio-dwapb.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright(c) 2014 Intel Corporation.
- */
-
-#ifndef GPIO_DW_APB_H
-#define GPIO_DW_APB_H
-
-#define DWAPB_MAX_GPIOS		32
-
-struct dwapb_port_property {
-	struct fwnode_handle *fwnode;
-	unsigned int	idx;
-	unsigned int	ngpio;
-	unsigned int	gpio_base;
-	int		irq[DWAPB_MAX_GPIOS];
-};
-
-struct dwapb_platform_data {
-	struct dwapb_port_property *properties;
-	unsigned int nports;
-};
-
-#endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes
  2021-08-04 16:00 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
@ 2021-08-05  8:02   ` Lee Jones
  2021-08-11 13:00     ` Andy Shevchenko
  2021-08-11 12:48   ` Linus Walleij
  1 sibling, 1 reply; 14+ messages in thread
From: Lee Jones @ 2021-08-05  8:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, linux-kernel, linux-gpio, Hoan Tran, Serge Semin,
	Linus Walleij, Bartosz Golaszewski

On Wed, 04 Aug 2021, Andy Shevchenko wrote:

> The driver can provide a software node group instead of
> passing legacy platform data. This will allow to drop
> the legacy platform data structures along with unifying
> a child device driver to use same interface for all
> property providers, i.e. Device Tree, ACPI, and board files.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Serge Semin <fancer.lancer@gmail.com>
> ---
> v2: added tag (Serge)
>  drivers/mfd/intel_quark_i2c_gpio.c | 70 ++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 33 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property
  2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
@ 2021-08-05 10:38   ` Andy Shevchenko
  2021-08-05 12:08     ` Serge Semin
  2021-08-11 12:50   ` Linus Walleij
  1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-05 10:38 UTC (permalink / raw)
  To: Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

On Wed, Aug 04, 2021 at 07:00:17PM +0300, Andy Shevchenko wrote:
> For backward compatibility with some legacy devices introduce
> a new (*) property gpio-base to read GPIO base. This will allow
> further cleaning up of the driver.
> 
> *) Note, it's not new for the GPIO library since the mockup driver
>    is using it already.

Serge, I haven't put any tags here since the patch has been modified.
It still works for my case. I hope that's what you wanted me to do.

Since Lee gave his ACKs, this is the last in the series to be reviewed.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property
  2021-08-05 10:38   ` Andy Shevchenko
@ 2021-08-05 12:08     ` Serge Semin
  0 siblings, 0 replies; 14+ messages in thread
From: Serge Semin @ 2021-08-05 12:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Lee Jones, linux-kernel, linux-gpio, Hoan Tran,
	Linus Walleij, Bartosz Golaszewski

Hi Andy

On Thu, Aug 05, 2021 at 01:38:46PM +0300, Andy Shevchenko wrote:
> On Wed, Aug 04, 2021 at 07:00:17PM +0300, Andy Shevchenko wrote:
> > For backward compatibility with some legacy devices introduce
> > a new (*) property gpio-base to read GPIO base. This will allow
> > further cleaning up of the driver.
> > 
> > *) Note, it's not new for the GPIO library since the mockup driver
> >    is using it already.
> 
> Serge, I haven't put any tags here since the patch has been modified.
> It still works for my case. I hope that's what you wanted me to do.


Thanks for the update. Yep, it works like a charm. No gpio-base is read
from DT.

Tested-by: Serge Semin <fancer.lancer@gmail.com>
Acked-by: Serge Semin <fancer.lancer@gmail.com>

-Sergey

> 
> Since Lee gave his ACKs, this is the last in the series to be reviewed.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()
  2021-08-04 16:00 [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
                   ` (2 preceding siblings ...)
  2021-08-04 16:00 ` [PATCH v2 4/4] gpio: dwapb: Get rid of legacy platform data Andy Shevchenko
@ 2021-08-11 12:42 ` Andy Shevchenko
  2021-08-11 12:55   ` Andy Shevchenko
  3 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-11 12:42 UTC (permalink / raw)
  To: Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

On Wed, Aug 04, 2021 at 07:00:16PM +0300, Andy Shevchenko wrote:
> Shared IRQ is only enabled for ACPI enumeration, there is no need
> to have a special flag for that, since we simple can test if device
> has been enumerated by ACPI. This unifies the checks in dwapb_get_irq()
> and dwapb_configure_irqs().

Bart, are you okay with the series? Can it be applied, please?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes
  2021-08-04 16:00 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
  2021-08-05  8:02   ` Lee Jones
@ 2021-08-11 12:48   ` Linus Walleij
  1 sibling, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2021-08-11 12:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Lee Jones, linux-kernel, open list:GPIO SUBSYSTEM,
	Hoan Tran, Serge Semin, Bartosz Golaszewski

On Wed, Aug 4, 2021 at 6:15 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> The driver can provide a software node group instead of
> passing legacy platform data. This will allow to drop
> the legacy platform data structures along with unifying
> a child device driver to use same interface for all
> property providers, i.e. Device Tree, ACPI, and board files.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Serge Semin <fancer.lancer@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property
  2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
  2021-08-05 10:38   ` Andy Shevchenko
@ 2021-08-11 12:50   ` Linus Walleij
  2021-08-11 13:05     ` Andy Shevchenko
  1 sibling, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2021-08-11 12:50 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Lee Jones, linux-kernel, open list:GPIO SUBSYSTEM,
	Hoan Tran, Serge Semin, Bartosz Golaszewski

On Wed, Aug 4, 2021 at 6:15 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> For backward compatibility with some legacy devices introduce
> a new (*) property gpio-base to read GPIO base. This will allow
> further cleaning up of the driver.
>
> *) Note, it's not new for the GPIO library since the mockup driver
>    is using it already.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: added check to ensure that the property won't be used by FW (Serge)
>  drivers/gpio/gpio-dwapb.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 4c7153cb646c..674e91e69cc5 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -584,6 +584,10 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
>
>                 pp->gpio_base   = -1;
>
> +               /* For internal use only, new platforms mustn't exercise this */
> +               if (is_software_node(fwnode))
> +                       fwnode_property_read_u32(fwnode, "gpio-base", &pp->gpio_base);

You rewrite the code quicker than I can review  :D

So this is elegant, I would prefer "linux,gpio-base" but the
overall change is more important, with or without that change:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()
  2021-08-11 12:42 ` [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
@ 2021-08-11 12:55   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-11 12:55 UTC (permalink / raw)
  To: Serge Semin, Lee Jones, linux-kernel, linux-gpio
  Cc: Hoan Tran, Serge Semin, Linus Walleij, Bartosz Golaszewski

On Wed, Aug 11, 2021 at 03:42:27PM +0300, Andy Shevchenko wrote:
> On Wed, Aug 04, 2021 at 07:00:16PM +0300, Andy Shevchenko wrote:
> > Shared IRQ is only enabled for ACPI enumeration, there is no need
> > to have a special flag for that, since we simple can test if device
> > has been enumerated by ACPI. This unifies the checks in dwapb_get_irq()
> > and dwapb_configure_irqs().
> 
> Bart, are you okay with the series? Can it be applied, please?

I can send it inside my PR if you prefer, btw. (I jest realized that I have
already couple of patches in the queue.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes
  2021-08-05  8:02   ` Lee Jones
@ 2021-08-11 13:00     ` Andy Shevchenko
  2021-08-16  7:17       ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-11 13:00 UTC (permalink / raw)
  To: Lee Jones
  Cc: Serge Semin, linux-kernel, linux-gpio, Hoan Tran, Serge Semin,
	Linus Walleij, Bartosz Golaszewski

On Thu, Aug 05, 2021 at 09:02:47AM +0100, Lee Jones wrote:
> On Wed, 04 Aug 2021, Andy Shevchenko wrote:
> 
> > The driver can provide a software node group instead of
> > passing legacy platform data. This will allow to drop
> > the legacy platform data structures along with unifying
> > a child device driver to use same interface for all
> > property providers, i.e. Device Tree, ACPI, and board files.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Tested-by: Serge Semin <fancer.lancer@gmail.com>
> > ---
> > v2: added tag (Serge)
> >  drivers/mfd/intel_quark_i2c_gpio.c | 70 ++++++++++++++++--------------
> >  1 file changed, 37 insertions(+), 33 deletions(-)
> 
> For my own reference (apply this as-is to your sign-off block):
> 
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Thanks!

Just for your information. I think due to spaces in front of your tag
the `b4` tool can't catch this automatically.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property
  2021-08-11 12:50   ` Linus Walleij
@ 2021-08-11 13:05     ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-11 13:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Serge Semin, Lee Jones, linux-kernel, open list:GPIO SUBSYSTEM,
	Hoan Tran, Serge Semin, Bartosz Golaszewski

On Wed, Aug 11, 2021 at 02:50:46PM +0200, Linus Walleij wrote:
> On Wed, Aug 4, 2021 at 6:15 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > For backward compatibility with some legacy devices introduce
> > a new (*) property gpio-base to read GPIO base. This will allow
> > further cleaning up of the driver.
> >
> > *) Note, it's not new for the GPIO library since the mockup driver
> >    is using it already.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2: added check to ensure that the property won't be used by FW (Serge)
> >  drivers/gpio/gpio-dwapb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> > index 4c7153cb646c..674e91e69cc5 100644
> > --- a/drivers/gpio/gpio-dwapb.c
> > +++ b/drivers/gpio/gpio-dwapb.c
> > @@ -584,6 +584,10 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
> >
> >                 pp->gpio_base   = -1;
> >
> > +               /* For internal use only, new platforms mustn't exercise this */
> > +               if (is_software_node(fwnode))
> > +                       fwnode_property_read_u32(fwnode, "gpio-base", &pp->gpio_base);
> 
> You rewrite the code quicker than I can review  :D

Sorry for that :-)

> So this is elegant, I would prefer "linux,gpio-base" but the
> overall change is more important, with or without that change:

I'm okay with the either, but the thing is that gpio-base is already in use.
Perhaps in the future somebody can change both (gpio-mockup and this driver)
to use the proposed one (AFAIU we free to change it since it's not part of FW
interface).

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes
  2021-08-11 13:00     ` Andy Shevchenko
@ 2021-08-16  7:17       ` Lee Jones
  0 siblings, 0 replies; 14+ messages in thread
From: Lee Jones @ 2021-08-16  7:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, linux-kernel, linux-gpio, Hoan Tran, Serge Semin,
	Linus Walleij, Bartosz Golaszewski

On Wed, 11 Aug 2021, Andy Shevchenko wrote:

> On Thu, Aug 05, 2021 at 09:02:47AM +0100, Lee Jones wrote:
> > On Wed, 04 Aug 2021, Andy Shevchenko wrote:
> > 
> > > The driver can provide a software node group instead of
> > > passing legacy platform data. This will allow to drop
> > > the legacy platform data structures along with unifying
> > > a child device driver to use same interface for all
> > > property providers, i.e. Device Tree, ACPI, and board files.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Tested-by: Serge Semin <fancer.lancer@gmail.com>
> > > ---
> > > v2: added tag (Serge)
> > >  drivers/mfd/intel_quark_i2c_gpio.c | 70 ++++++++++++++++--------------
> > >  1 file changed, 37 insertions(+), 33 deletions(-)
> > 
> > For my own reference (apply this as-is to your sign-off block):
> > 
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks!
> 
> Just for your information. I think due to spaces in front of your tag
> the `b4` tool can't catch this automatically.

Why would it need to?

This tag is "for my reference", for when I merge it.

Please add it to your sign-off block when you resubmit.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-08-16  7:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-04 16:00 [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
2021-08-04 16:00 ` [PATCH v2 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
2021-08-05 10:38   ` Andy Shevchenko
2021-08-05 12:08     ` Serge Semin
2021-08-11 12:50   ` Linus Walleij
2021-08-11 13:05     ` Andy Shevchenko
2021-08-04 16:00 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
2021-08-05  8:02   ` Lee Jones
2021-08-11 13:00     ` Andy Shevchenko
2021-08-16  7:17       ` Lee Jones
2021-08-11 12:48   ` Linus Walleij
2021-08-04 16:00 ` [PATCH v2 4/4] gpio: dwapb: Get rid of legacy platform data Andy Shevchenko
2021-08-11 12:42 ` [PATCH v2 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Andy Shevchenko
2021-08-11 12:55   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).