From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Convert the driver to using the new generic GPIO chip interfaces from
linux/gpio/generic.h.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-brcmstb.c | 112 ++++++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 52 deletions(-)

diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index e29a9589b3ccbd17d10f6671088dca3e76537927..be3ff916e134a674d3e1d334a7d431b7ad767a33 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -3,6 +3,7 @@
 
 #include <linux/bitops.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/generic.h>
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/irqdomain.h>
@@ -37,7 +38,7 @@ enum gio_reg_index {
 struct brcmstb_gpio_bank {
 	struct list_head node;
 	int id;
-	struct gpio_chip gc;
+	struct gpio_generic_chip chip;
 	struct brcmstb_gpio_priv *parent_priv;
 	u32 width;
 	u32 wake_active;
@@ -72,19 +73,18 @@ __brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
 {
 	void __iomem *reg_base = bank->parent_priv->reg_base;
 
-	return bank->gc.read_reg(reg_base + GIO_STAT(bank->id)) &
-	       bank->gc.read_reg(reg_base + GIO_MASK(bank->id));
+	return gpio_generic_read_reg(&bank->chip, reg_base + GIO_STAT(bank->id)) &
+	       gpio_generic_read_reg(&bank->chip, reg_base + GIO_MASK(bank->id));
 }
 
 static unsigned long
 brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
 {
 	unsigned long status;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&bank->gc.bgpio_lock, flags);
+	guard(gpio_generic_lock_irqsave)(&bank->chip);
+
 	status = __brcmstb_gpio_get_active_irqs(bank);
-	raw_spin_unlock_irqrestore(&bank->gc.bgpio_lock, flags);
 
 	return status;
 }
@@ -92,26 +92,26 @@ brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
 static int brcmstb_gpio_hwirq_to_offset(irq_hw_number_t hwirq,
 					struct brcmstb_gpio_bank *bank)
 {
-	return hwirq - bank->gc.offset;
+	return hwirq - bank->chip.gc.offset;
 }
 
 static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
 		unsigned int hwirq, bool enable)
 {
-	struct gpio_chip *gc = &bank->gc;
 	struct brcmstb_gpio_priv *priv = bank->parent_priv;
 	u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(hwirq, bank));
 	u32 imask;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
-	imask = gc->read_reg(priv->reg_base + GIO_MASK(bank->id));
+	guard(gpio_generic_lock_irqsave)(&bank->chip);
+
+	imask = gpio_generic_read_reg(&bank->chip,
+				      priv->reg_base + GIO_MASK(bank->id));
 	if (enable)
 		imask |= mask;
 	else
 		imask &= ~mask;
-	gc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
-	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
+	gpio_generic_write_reg(&bank->chip,
+			       priv->reg_base + GIO_MASK(bank->id), imask);
 }
 
 static int brcmstb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
@@ -150,7 +150,8 @@ static void brcmstb_gpio_irq_ack(struct irq_data *d)
 	struct brcmstb_gpio_priv *priv = bank->parent_priv;
 	u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(d->hwirq, bank));
 
-	gc->write_reg(priv->reg_base + GIO_STAT(bank->id), mask);
+	gpio_generic_write_reg(&bank->chip,
+			       priv->reg_base + GIO_STAT(bank->id), mask);
 }
 
 static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
@@ -162,7 +163,6 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	u32 edge_insensitive, iedge_insensitive;
 	u32 edge_config, iedge_config;
 	u32 level, ilevel;
-	unsigned long flags;
 
 	switch (type) {
 	case IRQ_TYPE_LEVEL_LOW:
@@ -194,23 +194,25 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 		return -EINVAL;
 	}
 
-	raw_spin_lock_irqsave(&bank->gc.bgpio_lock, flags);
+	guard(gpio_generic_lock_irqsave)(&bank->chip);
 
-	iedge_config = bank->gc.read_reg(priv->reg_base +
-			GIO_EC(bank->id)) & ~mask;
-	iedge_insensitive = bank->gc.read_reg(priv->reg_base +
-			GIO_EI(bank->id)) & ~mask;
-	ilevel = bank->gc.read_reg(priv->reg_base +
-			GIO_LEVEL(bank->id)) & ~mask;
+	iedge_config = gpio_generic_read_reg(&bank->chip,
+				priv->reg_base + GIO_EC(bank->id)) & ~mask;
+	iedge_insensitive = gpio_generic_read_reg(&bank->chip,
+				priv->reg_base + GIO_EI(bank->id)) & ~mask;
+	ilevel = gpio_generic_read_reg(&bank->chip,
+				priv->reg_base + GIO_LEVEL(bank->id)) & ~mask;
 
-	bank->gc.write_reg(priv->reg_base + GIO_EC(bank->id),
-			iedge_config | edge_config);
-	bank->gc.write_reg(priv->reg_base + GIO_EI(bank->id),
-			iedge_insensitive | edge_insensitive);
-	bank->gc.write_reg(priv->reg_base + GIO_LEVEL(bank->id),
-			ilevel | level);
+	gpio_generic_write_reg(&bank->chip,
+			       priv->reg_base + GIO_EC(bank->id),
+			       iedge_config | edge_config);
+	gpio_generic_write_reg(&bank->chip,
+			       priv->reg_base + GIO_EI(bank->id),
+			       iedge_insensitive | edge_insensitive);
+	gpio_generic_write_reg(&bank->chip,
+			       priv->reg_base + GIO_LEVEL(bank->id),
+			       ilevel | level);
 
-	raw_spin_unlock_irqrestore(&bank->gc.bgpio_lock, flags);
 	return 0;
 }
 
@@ -263,7 +265,7 @@ static void brcmstb_gpio_irq_bank_handler(struct brcmstb_gpio_bank *bank)
 {
 	struct brcmstb_gpio_priv *priv = bank->parent_priv;
 	struct irq_domain *domain = priv->irq_domain;
-	int hwbase = bank->gc.offset;
+	int hwbase = bank->chip.gc.offset;
 	unsigned long status;
 
 	while ((status = brcmstb_gpio_get_active_irqs(bank))) {
@@ -303,7 +305,7 @@ static struct brcmstb_gpio_bank *brcmstb_gpio_hwirq_to_bank(
 
 	/* banks are in descending order */
 	list_for_each_entry_reverse(bank, &priv->bank_list, node) {
-		i += bank->gc.ngpio;
+		i += bank->chip.gc.ngpio;
 		if (hwirq < i)
 			return bank;
 	}
@@ -332,7 +334,7 @@ static int brcmstb_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 
 	dev_dbg(&pdev->dev, "Mapping irq %d for gpio line %d (bank %d)\n",
 		irq, (int)hwirq, bank->id);
-	ret = irq_set_chip_data(irq, &bank->gc);
+	ret = irq_set_chip_data(irq, &bank->chip.gc);
 	if (ret < 0)
 		return ret;
 	irq_set_lockdep_class(irq, &brcmstb_gpio_irq_lock_class,
@@ -394,7 +396,7 @@ static void brcmstb_gpio_remove(struct platform_device *pdev)
 	 * more important to actually perform all of the steps.
 	 */
 	list_for_each_entry(bank, &priv->bank_list, node)
-		gpiochip_remove(&bank->gc);
+		gpiochip_remove(&bank->chip.gc);
 }
 
 static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
@@ -412,7 +414,7 @@ static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
 	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
 		return -EINVAL;
 
-	offset = gpiospec->args[0] - bank->gc.offset;
+	offset = gpiospec->args[0] - bank->chip.gc.offset;
 	if (offset >= gc->ngpio || offset < 0)
 		return -EINVAL;
 
@@ -493,19 +495,17 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
 static void brcmstb_gpio_bank_save(struct brcmstb_gpio_priv *priv,
 				   struct brcmstb_gpio_bank *bank)
 {
-	struct gpio_chip *gc = &bank->gc;
 	unsigned int i;
 
 	for (i = 0; i < GIO_REG_STAT; i++)
-		bank->saved_regs[i] = gc->read_reg(priv->reg_base +
-						   GIO_BANK_OFF(bank->id, i));
+		bank->saved_regs[i] = gpio_generic_read_reg(&bank->chip,
+					priv->reg_base + GIO_BANK_OFF(bank->id, i));
 }
 
 static void brcmstb_gpio_quiesce(struct device *dev, bool save)
 {
 	struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
 	struct brcmstb_gpio_bank *bank;
-	struct gpio_chip *gc;
 	u32 imask;
 
 	/* disable non-wake interrupt */
@@ -513,8 +513,6 @@ static void brcmstb_gpio_quiesce(struct device *dev, bool save)
 		disable_irq(priv->parent_irq);
 
 	list_for_each_entry(bank, &priv->bank_list, node) {
-		gc = &bank->gc;
-
 		if (save)
 			brcmstb_gpio_bank_save(priv, bank);
 
@@ -523,8 +521,9 @@ static void brcmstb_gpio_quiesce(struct device *dev, bool save)
 			imask = bank->wake_active;
 		else
 			imask = 0;
-		gc->write_reg(priv->reg_base + GIO_MASK(bank->id),
-			       imask);
+		gpio_generic_write_reg(&bank->chip,
+				       priv->reg_base + GIO_MASK(bank->id),
+				       imask);
 	}
 }
 
@@ -538,12 +537,12 @@ static void brcmstb_gpio_shutdown(struct platform_device *pdev)
 static void brcmstb_gpio_bank_restore(struct brcmstb_gpio_priv *priv,
 				      struct brcmstb_gpio_bank *bank)
 {
-	struct gpio_chip *gc = &bank->gc;
 	unsigned int i;
 
 	for (i = 0; i < GIO_REG_STAT; i++)
-		gc->write_reg(priv->reg_base + GIO_BANK_OFF(bank->id, i),
-			      bank->saved_regs[i]);
+		gpio_generic_write_reg(&bank->chip,
+				       priv->reg_base + GIO_BANK_OFF(bank->id, i),
+				       bank->saved_regs[i]);
 }
 
 static int brcmstb_gpio_suspend(struct device *dev)
@@ -585,6 +584,7 @@ static const struct dev_pm_ops brcmstb_gpio_pm_ops = {
 
 static int brcmstb_gpio_probe(struct platform_device *pdev)
 {
+	struct gpio_generic_chip_config config;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	void __iomem *reg_base;
@@ -665,17 +665,24 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 			bank->width = bank_width;
 		}
 
+		gc = &bank->chip.gc;
+
 		/*
 		 * Regs are 4 bytes wide, have data reg, no set/clear regs,
 		 * and direction bits have 0 = output and 1 = input
 		 */
-		gc = &bank->gc;
-		err = bgpio_init(gc, dev, 4,
-				reg_base + GIO_DATA(bank->id),
-				NULL, NULL, NULL,
-				reg_base + GIO_IODIR(bank->id), flags);
+
+		config = (struct gpio_generic_chip_config) {
+			.dev = dev,
+			.sz = 4,
+			.dat = reg_base + GIO_DATA(bank->id),
+			.dirin = reg_base + GIO_IODIR(bank->id),
+			.flags = flags,
+		};
+
+		err = gpio_generic_chip_init(&bank->chip, &config);
 		if (err) {
-			dev_err(dev, "bgpio_init() failed\n");
+			dev_err(dev, "failed to initialize generic GPIO chip\n");
 			goto fail;
 		}
 
@@ -700,7 +707,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 		 * be retained from S5 cold boot
 		 */
 		need_wakeup_event |= !!__brcmstb_gpio_get_active_irqs(bank);
-		gc->write_reg(reg_base + GIO_MASK(bank->id), 0);
+		gpio_generic_write_reg(&bank->chip,
+				       reg_base + GIO_MASK(bank->id), 0);
 
 		err = gpiochip_add_data(gc, bank);
 		if (err) {

-- 
2.48.1

On 9/10/2025 12:12 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Convert the driver to using the new generic GPIO chip interfaces from
> linux/gpio/generic.h.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>   drivers/gpio/gpio-brcmstb.c | 112 ++++++++++++++++++++++++--------------------
>   1 file changed, 60 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
> index e29a9589b3ccbd17d10f6671088dca3e76537927..be3ff916e134a674d3e1d334a7d431b7ad767a33 100644
> --- a/drivers/gpio/gpio-brcmstb.c
> +++ b/drivers/gpio/gpio-brcmstb.c
> @@ -3,6 +3,7 @@
>   
>   #include <linux/bitops.h>
>   #include <linux/gpio/driver.h>
> +#include <linux/gpio/generic.h>
>   #include <linux/of.h>
>   #include <linux/module.h>
>   #include <linux/irqdomain.h>
> @@ -37,7 +38,7 @@ enum gio_reg_index {
>   struct brcmstb_gpio_bank {
>   	struct list_head node;
>   	int id;
> -	struct gpio_chip gc;
> +	struct gpio_generic_chip chip;
>   	struct brcmstb_gpio_priv *parent_priv;
>   	u32 width;
>   	u32 wake_active;
> @@ -72,19 +73,18 @@ __brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
>   {
>   	void __iomem *reg_base = bank->parent_priv->reg_base;
>   
> -	return bank->gc.read_reg(reg_base + GIO_STAT(bank->id)) &
> -	       bank->gc.read_reg(reg_base + GIO_MASK(bank->id));
> +	return gpio_generic_read_reg(&bank->chip, reg_base + GIO_STAT(bank->id)) &
> +	       gpio_generic_read_reg(&bank->chip, reg_base + GIO_MASK(bank->id));
>   }
>   
>   static unsigned long
>   brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
>   {
>   	unsigned long status;
> -	unsigned long flags;
>   
> -	raw_spin_lock_irqsave(&bank->gc.bgpio_lock, flags);
> +	guard(gpio_generic_lock_irqsave)(&bank->chip);
> +
>   	status = __brcmstb_gpio_get_active_irqs(bank);
> -	raw_spin_unlock_irqrestore(&bank->gc.bgpio_lock, flags);
>   
>   	return status;
>   }
> @@ -92,26 +92,26 @@ brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
>   static int brcmstb_gpio_hwirq_to_offset(irq_hw_number_t hwirq,
>   					struct brcmstb_gpio_bank *bank)
>   {
> -	return hwirq - bank->gc.offset;
> +	return hwirq - bank->chip.gc.offset;
>   }
>   
>   static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
>   		unsigned int hwirq, bool enable)
>   {
> -	struct gpio_chip *gc = &bank->gc;
>   	struct brcmstb_gpio_priv *priv = bank->parent_priv;
>   	u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(hwirq, bank));
>   	u32 imask;
> -	unsigned long flags;
>   
> -	raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> -	imask = gc->read_reg(priv->reg_base + GIO_MASK(bank->id));
> +	guard(gpio_generic_lock_irqsave)(&bank->chip);
> +
> +	imask = gpio_generic_read_reg(&bank->chip,
> +				      priv->reg_base + GIO_MASK(bank->id));
>   	if (enable)
>   		imask |= mask;
>   	else
>   		imask &= ~mask;
> -	gc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
> -	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> +	gpio_generic_write_reg(&bank->chip,
> +			       priv->reg_base + GIO_MASK(bank->id), imask);
>   }
>   
>   static int brcmstb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
> @@ -150,7 +150,8 @@ static void brcmstb_gpio_irq_ack(struct irq_data *d)
>   	struct brcmstb_gpio_priv *priv = bank->parent_priv;
>   	u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(d->hwirq, bank));
>   
> -	gc->write_reg(priv->reg_base + GIO_STAT(bank->id), mask);
> +	gpio_generic_write_reg(&bank->chip,
> +			       priv->reg_base + GIO_STAT(bank->id), mask);
>   }
>   
>   static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> @@ -162,7 +163,6 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>   	u32 edge_insensitive, iedge_insensitive;
>   	u32 edge_config, iedge_config;
>   	u32 level, ilevel;
> -	unsigned long flags;
>   
>   	switch (type) {
>   	case IRQ_TYPE_LEVEL_LOW:
> @@ -194,23 +194,25 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>   		return -EINVAL;
>   	}
>   
> -	raw_spin_lock_irqsave(&bank->gc.bgpio_lock, flags);
> +	guard(gpio_generic_lock_irqsave)(&bank->chip);
>   
> -	iedge_config = bank->gc.read_reg(priv->reg_base +
> -			GIO_EC(bank->id)) & ~mask;
> -	iedge_insensitive = bank->gc.read_reg(priv->reg_base +
> -			GIO_EI(bank->id)) & ~mask;
> -	ilevel = bank->gc.read_reg(priv->reg_base +
> -			GIO_LEVEL(bank->id)) & ~mask;
> +	iedge_config = gpio_generic_read_reg(&bank->chip,
> +				priv->reg_base + GIO_EC(bank->id)) & ~mask;
> +	iedge_insensitive = gpio_generic_read_reg(&bank->chip,
> +				priv->reg_base + GIO_EI(bank->id)) & ~mask;
> +	ilevel = gpio_generic_read_reg(&bank->chip,
> +				priv->reg_base + GIO_LEVEL(bank->id)) & ~mask;
>   
> -	bank->gc.write_reg(priv->reg_base + GIO_EC(bank->id),
> -			iedge_config | edge_config);
> -	bank->gc.write_reg(priv->reg_base + GIO_EI(bank->id),
> -			iedge_insensitive | edge_insensitive);
> -	bank->gc.write_reg(priv->reg_base + GIO_LEVEL(bank->id),
> -			ilevel | level);
> +	gpio_generic_write_reg(&bank->chip,
> +			       priv->reg_base + GIO_EC(bank->id),
> +			       iedge_config | edge_config);
> +	gpio_generic_write_reg(&bank->chip,
> +			       priv->reg_base + GIO_EI(bank->id),
> +			       iedge_insensitive | edge_insensitive);
> +	gpio_generic_write_reg(&bank->chip,
> +			       priv->reg_base + GIO_LEVEL(bank->id),
> +			       ilevel | level);
>   
> -	raw_spin_unlock_irqrestore(&bank->gc.bgpio_lock, flags);
>   	return 0;
>   }
>   
> @@ -263,7 +265,7 @@ static void brcmstb_gpio_irq_bank_handler(struct brcmstb_gpio_bank *bank)
>   {
>   	struct brcmstb_gpio_priv *priv = bank->parent_priv;
>   	struct irq_domain *domain = priv->irq_domain;
> -	int hwbase = bank->gc.offset;
> +	int hwbase = bank->chip.gc.offset;
>   	unsigned long status;
>   
>   	while ((status = brcmstb_gpio_get_active_irqs(bank))) {
> @@ -303,7 +305,7 @@ static struct brcmstb_gpio_bank *brcmstb_gpio_hwirq_to_bank(
>   
>   	/* banks are in descending order */
>   	list_for_each_entry_reverse(bank, &priv->bank_list, node) {
> -		i += bank->gc.ngpio;
> +		i += bank->chip.gc.ngpio;
>   		if (hwirq < i)
>   			return bank;
>   	}
> @@ -332,7 +334,7 @@ static int brcmstb_gpio_irq_map(struct irq_domain *d, unsigned int irq,
>   
>   	dev_dbg(&pdev->dev, "Mapping irq %d for gpio line %d (bank %d)\n",
>   		irq, (int)hwirq, bank->id);
> -	ret = irq_set_chip_data(irq, &bank->gc);
> +	ret = irq_set_chip_data(irq, &bank->chip.gc);
>   	if (ret < 0)
>   		return ret;
>   	irq_set_lockdep_class(irq, &brcmstb_gpio_irq_lock_class,
> @@ -394,7 +396,7 @@ static void brcmstb_gpio_remove(struct platform_device *pdev)
>   	 * more important to actually perform all of the steps.
>   	 */
>   	list_for_each_entry(bank, &priv->bank_list, node)
> -		gpiochip_remove(&bank->gc);
> +		gpiochip_remove(&bank->chip.gc);
>   }
>   
>   static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
> @@ -412,7 +414,7 @@ static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
>   	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
>   		return -EINVAL;
>   
> -	offset = gpiospec->args[0] - bank->gc.offset;
> +	offset = gpiospec->args[0] - bank->chip.gc.offset;
>   	if (offset >= gc->ngpio || offset < 0)
>   		return -EINVAL;
>   
> @@ -493,19 +495,17 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
>   static void brcmstb_gpio_bank_save(struct brcmstb_gpio_priv *priv,
>   				   struct brcmstb_gpio_bank *bank)
>   {
> -	struct gpio_chip *gc = &bank->gc;
>   	unsigned int i;
>   
>   	for (i = 0; i < GIO_REG_STAT; i++)
> -		bank->saved_regs[i] = gc->read_reg(priv->reg_base +
> -						   GIO_BANK_OFF(bank->id, i));
> +		bank->saved_regs[i] = gpio_generic_read_reg(&bank->chip,
> +					priv->reg_base + GIO_BANK_OFF(bank->id, i));
>   }
>   
>   static void brcmstb_gpio_quiesce(struct device *dev, bool save)
>   {
>   	struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
>   	struct brcmstb_gpio_bank *bank;
> -	struct gpio_chip *gc;
>   	u32 imask;
>   
>   	/* disable non-wake interrupt */
> @@ -513,8 +513,6 @@ static void brcmstb_gpio_quiesce(struct device *dev, bool save)
>   		disable_irq(priv->parent_irq);
>   
>   	list_for_each_entry(bank, &priv->bank_list, node) {
> -		gc = &bank->gc;
> -
>   		if (save)
>   			brcmstb_gpio_bank_save(priv, bank);
>   
> @@ -523,8 +521,9 @@ static void brcmstb_gpio_quiesce(struct device *dev, bool save)
>   			imask = bank->wake_active;
>   		else
>   			imask = 0;
> -		gc->write_reg(priv->reg_base + GIO_MASK(bank->id),
> -			       imask);
> +		gpio_generic_write_reg(&bank->chip,
> +				       priv->reg_base + GIO_MASK(bank->id),
> +				       imask);
>   	}
>   }
>   
> @@ -538,12 +537,12 @@ static void brcmstb_gpio_shutdown(struct platform_device *pdev)
>   static void brcmstb_gpio_bank_restore(struct brcmstb_gpio_priv *priv,
>   				      struct brcmstb_gpio_bank *bank)
>   {
> -	struct gpio_chip *gc = &bank->gc;
>   	unsigned int i;
>   
>   	for (i = 0; i < GIO_REG_STAT; i++)
> -		gc->write_reg(priv->reg_base + GIO_BANK_OFF(bank->id, i),
> -			      bank->saved_regs[i]);
> +		gpio_generic_write_reg(&bank->chip,
> +				       priv->reg_base + GIO_BANK_OFF(bank->id, i),
> +				       bank->saved_regs[i]);
>   }
>   
>   static int brcmstb_gpio_suspend(struct device *dev)
> @@ -585,6 +584,7 @@ static const struct dev_pm_ops brcmstb_gpio_pm_ops = {
>   
>   static int brcmstb_gpio_probe(struct platform_device *pdev)
>   {
> +	struct gpio_generic_chip_config config;
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev->of_node;
>   	void __iomem *reg_base;
> @@ -665,17 +665,24 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
>   			bank->width = bank_width;
>   		}
>   
> +		gc = &bank->chip.gc;
> +
>   		/*
>   		 * Regs are 4 bytes wide, have data reg, no set/clear regs,
>   		 * and direction bits have 0 = output and 1 = input
>   		 */
> -		gc = &bank->gc;
> -		err = bgpio_init(gc, dev, 4,
> -				reg_base + GIO_DATA(bank->id),
> -				NULL, NULL, NULL,
> -				reg_base + GIO_IODIR(bank->id), flags);
> +
> +		config = (struct gpio_generic_chip_config) {
> +			.dev = dev,
> +			.sz = 4,
> +			.dat = reg_base + GIO_DATA(bank->id),
> +			.dirin = reg_base + GIO_IODIR(bank->id),
> +			.flags = flags,
> +		};
> +
> +		err = gpio_generic_chip_init(&bank->chip, &config);
>   		if (err) {
> -			dev_err(dev, "bgpio_init() failed\n");
> +			dev_err(dev, "failed to initialize generic GPIO chip\n");
>   			goto fail;
>   		}
>   
> @@ -700,7 +707,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
>   		 * be retained from S5 cold boot
>   		 */
>   		need_wakeup_event |= !!__brcmstb_gpio_get_active_irqs(bank);
> -		gc->write_reg(reg_base + GIO_MASK(bank->id), 0);
> +		gpio_generic_write_reg(&bank->chip,
> +				       reg_base + GIO_MASK(bank->id), 0);
>   
>   		err = gpiochip_add_data(gc, bank);
>   		if (err) {
> 
I suppose I'm OK with all of this, but I'm just curious about the longer 
term plans for the member accesses. Is there an intent to have helpers 
for things like?:
chip.gc.offset
chip.gc.ngpio

Thanks,
     Doug

On Thu, Sep 11, 2025 at 2:11 AM Doug Berger <opendmb@gmail.com> wrote:
>
> >
> > @@ -700,7 +707,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
> >                * be retained from S5 cold boot
> >                */
> >               need_wakeup_event |= !!__brcmstb_gpio_get_active_irqs(bank);
> > -             gc->write_reg(reg_base + GIO_MASK(bank->id), 0);
> > +             gpio_generic_write_reg(&bank->chip,
> > +                                    reg_base + GIO_MASK(bank->id), 0);
> >
> >               err = gpiochip_add_data(gc, bank);
> >               if (err) {
> >
> I suppose I'm OK with all of this, but I'm just curious about the longer
> term plans for the member accesses. Is there an intent to have helpers
> for things like?:
> chip.gc.offset
> chip.gc.ngpio

I don't think so. It would require an enormous effort and these fields
in struct gpio_chip are pretty stable so there's no real reason for
it.

Bart

On 9/11/2025 12:56 AM, Bartosz Golaszewski wrote:
> On Thu, Sep 11, 2025 at 2:11 AM Doug Berger <opendmb@gmail.com> wrote:
>>
>>>
>>> @@ -700,7 +707,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
>>>                 * be retained from S5 cold boot
>>>                 */
>>>                need_wakeup_event |= !!__brcmstb_gpio_get_active_irqs(bank);
>>> -             gc->write_reg(reg_base + GIO_MASK(bank->id), 0);
>>> +             gpio_generic_write_reg(&bank->chip,
>>> +                                    reg_base + GIO_MASK(bank->id), 0);
>>>
>>>                err = gpiochip_add_data(gc, bank);
>>>                if (err) {
>>>
>> I suppose I'm OK with all of this, but I'm just curious about the longer
>> term plans for the member accesses. Is there an intent to have helpers
>> for things like?:
>> chip.gc.offset
>> chip.gc.ngpio
> 
> I don't think so. It would require an enormous effort and these fields
> in struct gpio_chip are pretty stable so there's no real reason for
> it.
> 
> Bart
Ok, so assuming struct gpio_chip is sticking around long term that makes 
sense to me.

Thanks!

Acked-by: Doug Berger <opendmb@gmail.com>

On Thu, Sep 11, 2025 at 09:56:28AM +0200, Bartosz Golaszewski wrote:
> On Thu, Sep 11, 2025 at 2:11 AM Doug Berger <opendmb@gmail.com> wrote:

...

> > I'm just curious about the longer term plans for the member accesses. Is
> > there an intent to have helpers for things like?:
> > chip.gc.offset
> > chip.gc.ngpio
> 
> I don't think so. It would require an enormous effort and these fields
> in struct gpio_chip are pretty stable so there's no real reason for
> it.

What I would like to see in TODO is to "make struct gpio_chip const" when
passing to the gpiochip_add_*().

-- 
With Best Regards,
Andy Shevchenko

On 9/10/25 00:12, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Convert the driver to using the new generic GPIO chip interfaces from
> linux/gpio/generic.h.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian