[PATCH 0/5] gpio: ppc44x: assorted cleanups

[PATCH 0/5] gpio: ppc44x: assorted cleanups

[Rosen Penev]

A follow-up to the earlier ppc44x GPIO conversion to gpio-mmio. With the
driver now built on gpio_generic_chip, a few leftovers can go:

 - Patch 1 drops the label and parent assignments that
   gpio_generic_chip_init() already does.
 - Patches 2-3 deduplicate the OSR/TSR source selection shared by both
   direction callbacks and fold the single-caller __ppc44x_gpio_set()
   into ppc44x_gpio_dir_out().
 - Patch 4 expresses the MSB-first pin masks with BIT() and GENMASK().
 - Patch 5 trims includes, switches to devm_kzalloc_obj() and tidies
   up probe and the OF match table.

No functional change intended.

An AI coding assistant (Claude) was used to help prepare this series.

Rosen Penev (5):
  gpio: ppc44x: drop redundant label and parent assignments
  gpio: ppc44x: factor out the OSR/TSR source selection
  gpio: ppc44x: fold __ppc44x_gpio_set() into ppc44x_gpio_dir_out()
  gpio: ppc44x: use BIT() and GENMASK() for the pin masks
  gpio: ppc44x: tidy up includes and probe

 drivers/gpio/gpio-ppc44x.c | 89 ++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 52 deletions(-)

-- 
2.56.0

[PATCH 1/5] gpio: ppc44x: drop redundant label and parent assignments

[Rosen Penev]

gpio_generic_chip_init() already sets gc->parent to the device and
gc->label to dev_name() of it. Drop the duplicate assignments from
probe.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpio/gpio-ppc44x.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpio/gpio-ppc44x.c b/drivers/gpio/gpio-ppc44x.c
index 189667e0e1ba..01dd5e7cc311 100644
--- a/drivers/gpio/gpio-ppc44x.c
+++ b/drivers/gpio/gpio-ppc44x.c
@@ -178,8 +178,6 @@ static int ppc44x_gpio_probe(struct platform_device *ofdev)
 		return ret;
 
 	gc = &chip->chip.gc;
-	gc->label = dev_name(dev);
-	gc->parent = dev;
 	gc->direction_input = ppc44x_gpio_dir_in;
 	gc->direction_output = ppc44x_gpio_dir_out;
 
-- 
2.56.0

[PATCH 2/5] gpio: ppc44x: factor out the OSR/TSR source selection

[Rosen Penev]

Both direction callbacks clear the pin's 2-bit field in the output
source and three-state source registers, picking the low or high
register by bank. Move that into a helper instead of duplicating the
if/else in each callback.

No functional change; the registers are written in the same order.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpio/gpio-ppc44x.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-ppc44x.c b/drivers/gpio/gpio-ppc44x.c
index 01dd5e7cc311..b3cbc832cf24 100644
--- a/drivers/gpio/gpio-ppc44x.c
+++ b/drivers/gpio/gpio-ppc44x.c
@@ -64,6 +64,21 @@ static inline void ppc44x_setbits32(void __iomem *addr, u32 mask)
 	iowrite32be(val, addr);
 }
 
+/*
+ * Route the pin's output and three-state control to the GPIO controller
+ * rather than an alternate function. Bits 0-15 use OSRL/TSRL, bits 16-31
+ * use OSRH/TSRH.
+ */
+static void ppc44x_gpio_select_gpio(struct ppc44x_gpio __iomem *regs,
+				    unsigned int gpio)
+{
+	__be32 __iomem *osr = gpio < 16 ? &regs->osrl : &regs->osrh;
+	__be32 __iomem *tsr = gpio < 16 ? &regs->tsrl : &regs->tsrh;
+
+	ppc44x_clrbits32(osr, GPIO_MASK2(gpio));
+	ppc44x_clrbits32(tsr, GPIO_MASK2(gpio));
+}
+
 /*
  * GPIO LIB API implementation for GPIOs
  *
@@ -99,14 +114,7 @@ static int ppc44x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	ppc44x_clrbits32(&regs->tcr, GPIO_MASK(gpio));
 	gen_gc->sdir &= ~GPIO_MASK(gpio);
 
-	/* Bits 0-15 use TSRL/OSRL, bits 16-31 use TSRH/OSRH */
-	if (gpio < 16) {
-		ppc44x_clrbits32(&regs->osrl, GPIO_MASK2(gpio));
-		ppc44x_clrbits32(&regs->tsrl, GPIO_MASK2(gpio));
-	} else {
-		ppc44x_clrbits32(&regs->osrh, GPIO_MASK2(gpio));
-		ppc44x_clrbits32(&regs->tsrh, GPIO_MASK2(gpio));
-	}
+	ppc44x_gpio_select_gpio(regs, gpio);
 
 	return 0;
 }
@@ -130,14 +138,7 @@ ppc44x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	ppc44x_setbits32(&regs->tcr, GPIO_MASK(gpio));
 	gen_gc->sdir |= GPIO_MASK(gpio);
 
-	/* Bits 0-15 use TSRL, bits 16-31 use TSRH */
-	if (gpio < 16) {
-		ppc44x_clrbits32(&regs->osrl, GPIO_MASK2(gpio));
-		ppc44x_clrbits32(&regs->tsrl, GPIO_MASK2(gpio));
-	} else {
-		ppc44x_clrbits32(&regs->osrh, GPIO_MASK2(gpio));
-		ppc44x_clrbits32(&regs->tsrh, GPIO_MASK2(gpio));
-	}
+	ppc44x_gpio_select_gpio(regs, gpio);
 
 	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
 
-- 
2.56.0

[PATCH 3/5] gpio: ppc44x: fold __ppc44x_gpio_set() into ppc44x_gpio_dir_out()

[Rosen Penev]

__ppc44x_gpio_set() has a single caller, and it looks up the driver
data that the caller already has. Open-code it in the direction_output
callback, with a comment explaining why gpio_generic_chip_set() can't
be used there: the generic setter takes the chip lock, which is already
held at that point.

While at it, drop the pr_debug(); gpiolib's tracepoints already report
direction and value changes.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpio/gpio-ppc44x.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpio-ppc44x.c b/drivers/gpio/gpio-ppc44x.c
index b3cbc832cf24..20b3e9ce48e8 100644
--- a/drivers/gpio/gpio-ppc44x.c
+++ b/drivers/gpio/gpio-ppc44x.c
@@ -85,20 +85,6 @@ static void ppc44x_gpio_select_gpio(struct ppc44x_gpio __iomem *regs,
  * There are a maximum of 32 gpios in each gpio controller.
  */
 
-static inline void
-__ppc44x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
-{
-	struct ppc44x_gpio_chip *chip = gpiochip_get_data(gc);
-	struct gpio_generic_chip *gen_gc = &chip->chip;
-
-	if (val)
-		gen_gc->sdata |= GPIO_MASK(gpio);
-	else
-		gen_gc->sdata &= ~GPIO_MASK(gpio);
-
-	gpio_generic_write_reg(gen_gc, gen_gc->reg_set, gen_gc->sdata);
-}
-
 static int ppc44x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct ppc44x_gpio_chip *chip = gpiochip_get_data(gc);
@@ -128,8 +114,16 @@ ppc44x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 
 	guard(gpio_generic_lock_irqsave)(gen_gc);
 
-	/* First set initial value */
-	__ppc44x_gpio_set(gc, gpio, val);
+	/*
+	 * First set the initial value. This can't use gpio_generic_chip_set()
+	 * as the generic setter takes the chip lock, which is already held.
+	 */
+	if (val)
+		gen_gc->sdata |= GPIO_MASK(gpio);
+	else
+		gen_gc->sdata &= ~GPIO_MASK(gpio);
+
+	gpio_generic_write_reg(gen_gc, gen_gc->reg_set, gen_gc->sdata);
 
 	/* Disable open-drain function */
 	ppc44x_clrbits32(&regs->odr, GPIO_MASK(gpio));
@@ -140,8 +134,6 @@ ppc44x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 
 	ppc44x_gpio_select_gpio(regs, gpio);
 
-	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
-
 	return 0;
 }
 
-- 
2.56.0

[PATCH 4/5] gpio: ppc44x: use BIT() and GENMASK() for the pin masks

[Rosen Penev]

Express the per-pin masks with BIT() and GENMASK() rather than
open-coded shifts of 0x80000000 and 0xc0000000. The pins are numbered
from the MSB, matching the bit mapping gpio-mmio uses for
GPIO_GENERIC_BIG_ENDIAN.

No functional change; both forms yield the same mask for all 32 pins.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpio/gpio-ppc44x.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-ppc44x.c b/drivers/gpio/gpio-ppc44x.c
index 20b3e9ce48e8..8b9ea00f170c 100644
--- a/drivers/gpio/gpio-ppc44x.c
+++ b/drivers/gpio/gpio-ppc44x.c
@@ -9,6 +9,7 @@
  * Author: Steve Falco <sfalco@harris.com>
  */
 
+#include <linux/bits.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -18,8 +19,9 @@
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 
-#define GPIO_MASK(gpio)		(0x80000000 >> (gpio))
-#define GPIO_MASK2(gpio)	(0xc0000000 >> (((gpio) % 16) * 2))
+/* Pins are numbered from the MSB; the select registers use 2 bits per pin */
+#define GPIO_MASK(gpio)		BIT(31 - (gpio))
+#define GPIO_MASK2(gpio)	(GENMASK(1, 0) << (30 - ((gpio) % 16) * 2))
 
 /* Physical GPIO register layout */
 struct ppc44x_gpio {
-- 
2.56.0

[PATCH 5/5] gpio: ppc44x: tidy up includes and probe

[Rosen Penev]

Remove some unused includes that are not used directly.

While at it:
 - use devm_kzalloc_obj() for the chip allocation
 - rename the platform_device from ofdev to the conventional pdev
 - compact the OF match table
 - move module_platform_driver() next to the driver struct

No functional change.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpio/gpio-ppc44x.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-ppc44x.c b/drivers/gpio/gpio-ppc44x.c
index 8b9ea00f170c..04653e753d43 100644
--- a/drivers/gpio/gpio-ppc44x.c
+++ b/drivers/gpio/gpio-ppc44x.c
@@ -10,13 +10,8 @@
  */
 
 #include <linux/bits.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
 #include <linux/io.h>
-#include <linux/of.h>
 #include <linux/gpio/generic.h>
-#include <linux/types.h>
-#include <linux/slab.h>
 #include <linux/platform_device.h>
 
 /* Pins are numbered from the MSB; the select registers use 2 bits per pin */
@@ -139,20 +134,20 @@ ppc44x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	return 0;
 }
 
-static int ppc44x_gpio_probe(struct platform_device *ofdev)
+static int ppc44x_gpio_probe(struct platform_device *pdev)
 {
-	struct device *dev = &ofdev->dev;
+	struct device *dev = &pdev->dev;
 	struct ppc44x_gpio __iomem *regs;
 	struct ppc44x_gpio_chip *chip;
 	struct gpio_generic_chip_config config;
 	struct gpio_chip *gc;
 	int ret;
 
-	regs = devm_platform_ioremap_resource(ofdev, 0);
+	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc_obj(dev, *chip);
 	if (!chip)
 		return -ENOMEM;
 
@@ -180,9 +175,7 @@ static int ppc44x_gpio_probe(struct platform_device *ofdev)
 }
 
 static const struct of_device_id ppc44x_gpio_match[] = {
-	{
-		.compatible = "ibm,ppc4xx-gpio",
-	},
+	{ .compatible = "ibm,ppc4xx-gpio" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ppc44x_gpio_match);
@@ -194,8 +187,7 @@ static struct platform_driver ppc44x_gpio_driver = {
 		.of_match_table	= ppc44x_gpio_match,
 	},
 };
+module_platform_driver(ppc44x_gpio_driver);
 
 MODULE_DESCRIPTION("PPC44x gpio driver");
 MODULE_LICENSE("GPL");
-
-module_platform_driver(ppc44x_gpio_driver);
-- 
2.56.0