[PATCH 0/5] GPIO PCH/ML-IOH consolidation baby steps

[PATCH 0/5] GPIO PCH/ML-IOH consolidation baby steps

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

These are tiny cleanups to pch and ml-ioh to try to make them easier to
merge together.  I can't really test either driver, so I'm not pushing too
hard on this.

If anybody wants to go further, Andy mentioned some docs here:
  https://lore.kernel.org/lkml/CAHp75VfDcQXqmK9=4k4rqi7t2OZaVPC13b45vLY7fELr7zBG_Q@mail.gmail.com/

Bjorn Helgaas (5):
  gpio: pch: Use .driver_data instead of checking Device IDs again
  gpio: pch: Cache &pdev->dev to reduce repetition
  gpio: ml-ioh: Cache &pdev->dev to reduce repetition
  gpio: ml-ioh: Use BIT() to match gpio-pch.c
  gpio: ml-ioh: Change whitespace to match gpio-pch.c

 drivers/gpio/gpio-ml-ioh.c | 52 +++++++++++++++++++-------------------
 drivers/gpio/gpio-pch.c    | 42 +++++++++++++++---------------
 2 files changed, 46 insertions(+), 48 deletions(-)

-- 
2.25.1

[PATCH 1/5] gpio: pch: Use .driver_data instead of checking Device IDs again

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Previously, pch_gpio_probe() tested the Device ID to determine the type of
IOH.  But the driver core has already matched the Device ID with one of the
IDs in the pch_gpio_pcidev_id[] table, and we can supply the IOH type there
as .driver_data.

Use the pci_device_id.driver_data to learn the IOH type instead of testing
the Device ID again.

No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpio/gpio-pch.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-pch.c b/drivers/gpio/gpio-pch.c
index a552df298a97..625920421990 100644
--- a/drivers/gpio/gpio-pch.c
+++ b/drivers/gpio/gpio-pch.c
@@ -368,14 +368,7 @@ static int pch_gpio_probe(struct pci_dev *pdev,
 	}
 
 	chip->base = pcim_iomap_table(pdev)[1];
-
-	if (pdev->device == 0x8803)
-		chip->ioh = INTEL_EG20T_PCH;
-	else if (pdev->device == 0x8014)
-		chip->ioh = OKISEMI_ML7223m_IOH;
-	else if (pdev->device == 0x8043)
-		chip->ioh = OKISEMI_ML7223n_IOH;
-
+	chip->ioh = id->driver_data;
 	chip->reg = chip->base;
 	pci_set_drvdata(pdev, chip);
 	spin_lock_init(&chip->spinlock);
@@ -439,10 +432,14 @@ static int __maybe_unused pch_gpio_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(pch_gpio_pm_ops, pch_gpio_suspend, pch_gpio_resume);
 
 static const struct pci_device_id pch_gpio_pcidev_id[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x8803) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8014) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8043) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8803) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x8803),
+	  .driver_data = INTEL_EG20T_PCH },
+	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8014),
+	  .driver_data = OKISEMI_ML7223m_IOH },
+	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8043),
+	  .driver_data = OKISEMI_ML7223n_IOH },
+	{ PCI_DEVICE(PCI_VENDOR_ID_ROHM, 0x8803),
+	  .driver_data = INTEL_EG20T_PCH },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, pch_gpio_pcidev_id);
-- 
2.25.1

[PATCH 2/5] gpio: pch: Cache &pdev->dev to reduce repetition

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

pch_gpio_probe() repeats the "&pdev->dev" expression several times.  Cache
the result as "struct device *dev" to reduce the repetition.  No functional
change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpio/gpio-pch.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-pch.c b/drivers/gpio/gpio-pch.c
index 625920421990..3a0bd8795741 100644
--- a/drivers/gpio/gpio-pch.c
+++ b/drivers/gpio/gpio-pch.c
@@ -346,24 +346,25 @@ static int pch_gpio_alloc_generic_chip(struct pch_gpio *chip,
 static int pch_gpio_probe(struct pci_dev *pdev,
 				    const struct pci_device_id *id)
 {
+	struct device *dev = &pdev->dev;
 	s32 ret;
 	struct pch_gpio *chip;
 	int irq_base;
 
-	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
 	if (chip == NULL)
 		return -ENOMEM;
 
-	chip->dev = &pdev->dev;
+	chip->dev = dev;
 	ret = pcim_enable_device(pdev);
 	if (ret) {
-		dev_err(&pdev->dev, "pci_enable_device FAILED");
+		dev_err(dev, "pci_enable_device FAILED");
 		return ret;
 	}
 
 	ret = pcim_iomap_regions(pdev, BIT(1), KBUILD_MODNAME);
 	if (ret) {
-		dev_err(&pdev->dev, "pci_request_regions FAILED-%d", ret);
+		dev_err(dev, "pci_request_regions FAILED-%d", ret);
 		return ret;
 	}
 
@@ -374,16 +375,16 @@ static int pch_gpio_probe(struct pci_dev *pdev,
 	spin_lock_init(&chip->spinlock);
 	pch_gpio_setup(chip);
 
-	ret = devm_gpiochip_add_data(&pdev->dev, &chip->gpio, chip);
+	ret = devm_gpiochip_add_data(dev, &chip->gpio, chip);
 	if (ret) {
-		dev_err(&pdev->dev, "PCH gpio: Failed to register GPIO\n");
+		dev_err(dev, "PCH gpio: Failed to register GPIO\n");
 		return ret;
 	}
 
-	irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0,
+	irq_base = devm_irq_alloc_descs(dev, -1, 0,
 					gpio_pins[chip->ioh], NUMA_NO_NODE);
 	if (irq_base < 0) {
-		dev_warn(&pdev->dev, "PCH gpio: Failed to get IRQ base num\n");
+		dev_warn(dev, "PCH gpio: Failed to get IRQ base num\n");
 		chip->irq_base = -1;
 		return 0;
 	}
@@ -393,10 +394,10 @@ static int pch_gpio_probe(struct pci_dev *pdev,
 	iowrite32(BIT(gpio_pins[chip->ioh]) - 1, &chip->reg->imask);
 	iowrite32(BIT(gpio_pins[chip->ioh]) - 1, &chip->reg->ien);
 
-	ret = devm_request_irq(&pdev->dev, pdev->irq, pch_gpio_handler,
+	ret = devm_request_irq(dev, pdev->irq, pch_gpio_handler,
 			       IRQF_SHARED, KBUILD_MODNAME, chip);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq failed\n");
+		dev_err(dev, "request_irq failed\n");
 		return ret;
 	}
 
-- 
2.25.1

[PATCH 3/5] gpio: ml-ioh: Cache &pdev->dev to reduce repetition

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

ioh_gpio_probe() repeats the "&pdev->dev" expression several times.  Cache
the result as "struct device *dev" to reduce the repetition.  No functional
change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpio/gpio-ml-ioh.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
index efa9acdc320a..4e9528dd1152 100644
--- a/drivers/gpio/gpio-ml-ioh.c
+++ b/drivers/gpio/gpio-ml-ioh.c
@@ -401,6 +401,7 @@ static int ioh_gpio_alloc_generic_chip(struct ioh_gpio *chip,
 static int ioh_gpio_probe(struct pci_dev *pdev,
 				    const struct pci_device_id *id)
 {
+	struct device *dev = &pdev->dev;
 	int ret;
 	int i, j;
 	struct ioh_gpio *chip;
@@ -410,19 +411,19 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 
 	ret = pci_enable_device(pdev);
 	if (ret) {
-		dev_err(&pdev->dev, "%s : pci_enable_device failed", __func__);
+		dev_err(dev, "%s : pci_enable_device failed", __func__);
 		goto err_pci_enable;
 	}
 
 	ret = pci_request_regions(pdev, KBUILD_MODNAME);
 	if (ret) {
-		dev_err(&pdev->dev, "pci_request_regions failed-%d", ret);
+		dev_err(dev, "pci_request_regions failed-%d", ret);
 		goto err_request_regions;
 	}
 
 	base = pci_iomap(pdev, 1, 0);
 	if (!base) {
-		dev_err(&pdev->dev, "%s : pci_iomap failed", __func__);
+		dev_err(dev, "%s : pci_iomap failed", __func__);
 		ret = -ENOMEM;
 		goto err_iomap;
 	}
@@ -435,7 +436,7 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 
 	chip = chip_save;
 	for (i = 0; i < 8; i++, chip++) {
-		chip->dev = &pdev->dev;
+		chip->dev = dev;
 		chip->base = base;
 		chip->reg = chip->base;
 		chip->ch = i;
@@ -443,17 +444,17 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 		ioh_gpio_setup(chip, num_ports[i]);
 		ret = gpiochip_add_data(&chip->gpio, chip);
 		if (ret) {
-			dev_err(&pdev->dev, "IOH gpio: Failed to register GPIO\n");
+			dev_err(dev, "IOH gpio: Failed to register GPIO\n");
 			goto err_gpiochip_add;
 		}
 	}
 
 	chip = chip_save;
 	for (j = 0; j < 8; j++, chip++) {
-		irq_base = devm_irq_alloc_descs(&pdev->dev, -1, IOH_IRQ_BASE,
+		irq_base = devm_irq_alloc_descs(dev, -1, IOH_IRQ_BASE,
 						num_ports[j], NUMA_NO_NODE);
 		if (irq_base < 0) {
-			dev_warn(&pdev->dev,
+			dev_warn(dev,
 				"ml_ioh_gpio: Failed to get IRQ base num\n");
 			ret = irq_base;
 			goto err_gpiochip_add;
@@ -467,11 +468,10 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 	}
 
 	chip = chip_save;
-	ret = devm_request_irq(&pdev->dev, pdev->irq, ioh_gpio_handler,
+	ret = devm_request_irq(dev, pdev->irq, ioh_gpio_handler,
 			       IRQF_SHARED, KBUILD_MODNAME, chip);
 	if (ret != 0) {
-		dev_err(&pdev->dev,
-			"%s request_irq failed\n", __func__);
+		dev_err(dev, "%s request_irq failed\n", __func__);
 		goto err_gpiochip_add;
 	}
 
@@ -498,7 +498,7 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 
 err_pci_enable:
 
-	dev_err(&pdev->dev, "%s Failed returns %d\n", __func__, ret);
+	dev_err(dev, "%s Failed returns %d\n", __func__, ret);
 	return ret;
 }
 
-- 
2.25.1

[PATCH 4/5] gpio: ml-ioh: Use BIT() to match gpio-pch.c

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The ML IOH driver is very similar to the PCH driver.  To make it more
similar, replace "1 << nr" with "BIT(nr)".  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpio/gpio-ml-ioh.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
index 4e9528dd1152..0fb9c8bc9b2d 100644
--- a/drivers/gpio/gpio-ml-ioh.c
+++ b/drivers/gpio/gpio-ml-ioh.c
@@ -98,9 +98,9 @@ static void ioh_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
 	spin_lock_irqsave(&chip->spinlock, flags);
 	reg_val = ioread32(&chip->reg->regs[chip->ch].po);
 	if (val)
-		reg_val |= (1 << nr);
+		reg_val |= BIT(nr);
 	else
-		reg_val &= ~(1 << nr);
+		reg_val &= ~BIT(nr);
 
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
 	spin_unlock_irqrestore(&chip->spinlock, flags);
@@ -110,7 +110,7 @@ static int ioh_gpio_get(struct gpio_chip *gpio, unsigned nr)
 {
 	struct ioh_gpio *chip =	gpiochip_get_data(gpio);
 
-	return !!(ioread32(&chip->reg->regs[chip->ch].pi) & (1 << nr));
+	return !!(ioread32(&chip->reg->regs[chip->ch].pi) & BIT(nr));
 }
 
 static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
@@ -123,15 +123,15 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 
 	spin_lock_irqsave(&chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm) &
-					((1 << num_ports[chip->ch]) - 1);
-	pm |= (1 << nr);
+					(BIT(num_ports[chip->ch]) - 1);
+	pm |= BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
 
 	reg_val = ioread32(&chip->reg->regs[chip->ch].po);
 	if (val)
-		reg_val |= (1 << nr);
+		reg_val |= BIT(nr);
 	else
-		reg_val &= ~(1 << nr);
+		reg_val &= ~BIT(nr);
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
 
 	spin_unlock_irqrestore(&chip->spinlock, flags);
@@ -147,8 +147,8 @@ static int ioh_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 
 	spin_lock_irqsave(&chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm) &
-				((1 << num_ports[chip->ch]) - 1);
-	pm &= ~(1 << nr);
+				(BIT(num_ports[chip->ch]) - 1);
+	pm &= ~BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
 	spin_unlock_irqrestore(&chip->spinlock, flags);
 
@@ -304,7 +304,7 @@ static void ioh_irq_unmask(struct irq_data *d)
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	struct ioh_gpio *chip = gc->private;
 
-	iowrite32(1 << (d->irq - chip->irq_base),
+	iowrite32(BIT(d->irq - chip->irq_base),
 		  &chip->reg->regs[chip->ch].imaskclr);
 }
 
@@ -313,7 +313,7 @@ static void ioh_irq_mask(struct irq_data *d)
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	struct ioh_gpio *chip = gc->private;
 
-	iowrite32(1 << (d->irq - chip->irq_base),
+	iowrite32(BIT(d->irq - chip->irq_base),
 		  &chip->reg->regs[chip->ch].imask);
 }
 
@@ -326,7 +326,7 @@ static void ioh_irq_disable(struct irq_data *d)
 
 	spin_lock_irqsave(&chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
-	ien &= ~(1 << (d->irq - chip->irq_base));
+	ien &= ~BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
 	spin_unlock_irqrestore(&chip->spinlock, flags);
 }
@@ -340,7 +340,7 @@ static void ioh_irq_enable(struct irq_data *d)
 
 	spin_lock_irqsave(&chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
-	ien |= 1 << (d->irq - chip->irq_base);
+	ien |= BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
 	spin_unlock_irqrestore(&chip->spinlock, flags);
 }
-- 
2.25.1

[PATCH 5/5] gpio: ml-ioh: Change whitespace to match gpio-pch.c

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The ML IOH driver is very similar to the PCH driver.  To make it more
similar, tweak the whitespace in ioh_gpio_direction_output() and
ioh_gpio_direction_input().  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpio/gpio-ml-ioh.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
index 0fb9c8bc9b2d..b060c4773698 100644
--- a/drivers/gpio/gpio-ml-ioh.c
+++ b/drivers/gpio/gpio-ml-ioh.c
@@ -122,8 +122,8 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 	unsigned long flags;
 
 	spin_lock_irqsave(&chip->spinlock, flags);
-	pm = ioread32(&chip->reg->regs[chip->ch].pm) &
-					(BIT(num_ports[chip->ch]) - 1);
+	pm = ioread32(&chip->reg->regs[chip->ch].pm);
+	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm |= BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
 
@@ -146,8 +146,8 @@ static int ioh_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 	unsigned long flags;
 
 	spin_lock_irqsave(&chip->spinlock, flags);
-	pm = ioread32(&chip->reg->regs[chip->ch].pm) &
-				(BIT(num_ports[chip->ch]) - 1);
+	pm = ioread32(&chip->reg->regs[chip->ch].pm);
+	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm &= ~BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
 	spin_unlock_irqrestore(&chip->spinlock, flags);
-- 
2.25.1

Re: [PATCH 0/5] GPIO PCH/ML-IOH consolidation baby steps

[Andy Shevchenko]

On Tue, Nov 30, 2021 at 04:08:36PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> These are tiny cleanups to pch and ml-ioh to try to make them easier to
> merge together.  I can't really test either driver, so I'm not pushing too
> hard on this.

Thanks, all pushed to my review and testing queue!

> If anybody wants to go further, Andy mentioned some docs here:
>   https://lore.kernel.org/lkml/CAHp75VfDcQXqmK9=4k4rqi7t2OZaVPC13b45vLY7fELr7zBG_Q@mail.gmail.com/
> 
> Bjorn Helgaas (5):
>   gpio: pch: Use .driver_data instead of checking Device IDs again
>   gpio: pch: Cache &pdev->dev to reduce repetition
>   gpio: ml-ioh: Cache &pdev->dev to reduce repetition
>   gpio: ml-ioh: Use BIT() to match gpio-pch.c
>   gpio: ml-ioh: Change whitespace to match gpio-pch.c
> 
>  drivers/gpio/gpio-ml-ioh.c | 52 +++++++++++++++++++-------------------
>  drivers/gpio/gpio-pch.c    | 42 +++++++++++++++---------------
>  2 files changed, 46 insertions(+), 48 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko