[PATCH 0/5] powerpc: convert board-file GPIO chips to using new value setters

[PATCH 0/5] powerpc: convert board-file GPIO chips to using new value setters

[Bartosz Golaszewski]

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. We're in the process of 
converting all GPIO drivers to using the new API. This series converts 
all powerpc board-file level controllers.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Bartosz Golaszewski (5):
      powerpc: sysdev/gpio: use new line value setter callbacks
      powerpc: 83xx/gpio: use new line value setter callbacks
      powerpc: 44x/gpio: use new line value setter callbacks
      powerpc: 52xx/gpio: use new line value setter callbacks
      powerpc: 8xx/gpio: use new line value setter callbacks

 arch/powerpc/platforms/44x/gpio.c              |  7 ++++---
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c      |  6 ++++--
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c |  6 ++++--
 arch/powerpc/platforms/8xx/cpm1.c              | 12 ++++++++----
 arch/powerpc/sysdev/cpm_common.c               |  6 ++++--
 5 files changed, 24 insertions(+), 13 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250326-gpiochip-set-rv-powerpc-1e98d28222aa

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[PATCH 1/5] powerpc: sysdev/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

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

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/powerpc/sysdev/cpm_common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index 47db732981a8..e22fc638dbc7 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -138,7 +138,7 @@ static void __cpm2_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask,
 	out_be32(&iop->dat, cpm2_gc->cpdata);
 }
 
-static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
+static int cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct cpm2_gpio32_chip *cpm2_gc = gpiochip_get_data(gc);
@@ -150,6 +150,8 @@ static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
 	__cpm2_gpio32_set(mm_gc, pin_mask, value);
 
 	spin_unlock_irqrestore(&cpm2_gc->lock, flags);
+
+	return 0;
 }
 
 static int cpm2_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
@@ -208,7 +210,7 @@ int cpm2_gpiochip_add32(struct device *dev)
 	gc->direction_input = cpm2_gpio32_dir_in;
 	gc->direction_output = cpm2_gpio32_dir_out;
 	gc->get = cpm2_gpio32_get;
-	gc->set = cpm2_gpio32_set;
+	gc->set_rv = cpm2_gpio32_set;
 	gc->parent = dev;
 	gc->owner = THIS_MODULE;
 

-- 
2.45.2

[PATCH 2/5] powerpc: 83xx/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

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

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 4d8fa9ed1a67..d4ba6dbb86b2 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -92,7 +92,7 @@ static void mcu_power_off(void)
 	mutex_unlock(&mcu->lock);
 }
 
-static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+static int mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	struct mcu *mcu = gpiochip_get_data(gc);
 	u8 bit = 1 << (4 + gpio);
@@ -105,6 +105,8 @@ static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 
 	i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
 	mutex_unlock(&mcu->lock);
+
+	return 0;
 }
 
 static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
@@ -123,7 +125,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
 	gc->can_sleep = 1;
 	gc->ngpio = MCU_NUM_GPIO;
 	gc->base = -1;
-	gc->set = mcu_gpio_set;
+	gc->set_rv = mcu_gpio_set;
 	gc->direction_output = mcu_gpio_dir_out;
 	gc->parent = dev;
 

-- 
2.45.2

[PATCH 3/5] powerpc: 44x/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

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

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/powerpc/platforms/44x/gpio.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/44x/gpio.c b/arch/powerpc/platforms/44x/gpio.c
index e5f2319e5cbe..d540e261d85a 100644
--- a/arch/powerpc/platforms/44x/gpio.c
+++ b/arch/powerpc/platforms/44x/gpio.c
@@ -75,8 +75,7 @@ __ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 		clrbits32(&regs->or, GPIO_MASK(gpio));
 }
 
-static void
-ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+static int ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	struct ppc4xx_gpio_chip *chip = gpiochip_get_data(gc);
 	unsigned long flags;
@@ -88,6 +87,8 @@ ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	spin_unlock_irqrestore(&chip->lock, flags);
 
 	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+	return 0;
 }
 
 static int ppc4xx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
@@ -179,7 +180,7 @@ static int __init ppc4xx_add_gpiochips(void)
 		gc->direction_input = ppc4xx_gpio_dir_in;
 		gc->direction_output = ppc4xx_gpio_dir_out;
 		gc->get = ppc4xx_gpio_get;
-		gc->set = ppc4xx_gpio_set;
+		gc->set_rv = ppc4xx_gpio_set;
 
 		ret = of_mm_gpiochip_add_data(np, mm_gc, ppc4xx_gc);
 		if (ret)

-- 
2.45.2

[PATCH 4/5] powerpc: 52xx/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

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

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 1ea591ec6083..c96af6b0eab4 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -280,7 +280,7 @@ static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
 	return (in_be32(&gpt->regs->status) >> 8) & 1;
 }
 
-static void
+static int
 mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int v)
 {
 	struct mpc52xx_gpt_priv *gpt = gpiochip_get_data(gc);
@@ -293,6 +293,8 @@ mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int v)
 	raw_spin_lock_irqsave(&gpt->lock, flags);
 	clrsetbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK, r);
 	raw_spin_unlock_irqrestore(&gpt->lock, flags);
+
+	return 0;
 }
 
 static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
@@ -334,7 +336,7 @@ static void mpc52xx_gpt_gpio_setup(struct mpc52xx_gpt_priv *gpt)
 	gpt->gc.direction_input  = mpc52xx_gpt_gpio_dir_in;
 	gpt->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
 	gpt->gc.get = mpc52xx_gpt_gpio_get;
-	gpt->gc.set = mpc52xx_gpt_gpio_set;
+	gpt->gc.set_rv = mpc52xx_gpt_gpio_set;
 	gpt->gc.base = -1;
 	gpt->gc.parent = gpt->dev;
 

-- 
2.45.2

[PATCH 5/5] powerpc: 8xx/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

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

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/powerpc/platforms/8xx/cpm1.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/8xx/cpm1.c b/arch/powerpc/platforms/8xx/cpm1.c
index 1dc095ad48fc..7462c221115c 100644
--- a/arch/powerpc/platforms/8xx/cpm1.c
+++ b/arch/powerpc/platforms/8xx/cpm1.c
@@ -417,7 +417,7 @@ static void __cpm1_gpio16_set(struct cpm1_gpio16_chip *cpm1_gc, u16 pin_mask, in
 	out_be16(&iop->dat, cpm1_gc->cpdata);
 }
 
-static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
+static int cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
 {
 	struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc);
 	unsigned long flags;
@@ -428,6 +428,8 @@ static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
 	__cpm1_gpio16_set(cpm1_gc, pin_mask, value);
 
 	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
+
+	return 0;
 }
 
 static int cpm1_gpio16_to_irq(struct gpio_chip *gc, unsigned int gpio)
@@ -497,7 +499,7 @@ int cpm1_gpiochip_add16(struct device *dev)
 	gc->direction_input = cpm1_gpio16_dir_in;
 	gc->direction_output = cpm1_gpio16_dir_out;
 	gc->get = cpm1_gpio16_get;
-	gc->set = cpm1_gpio16_set;
+	gc->set_rv = cpm1_gpio16_set;
 	gc->to_irq = cpm1_gpio16_to_irq;
 	gc->parent = dev;
 	gc->owner = THIS_MODULE;
@@ -554,7 +556,7 @@ static void __cpm1_gpio32_set(struct cpm1_gpio32_chip *cpm1_gc, u32 pin_mask, in
 	out_be32(&iop->dat, cpm1_gc->cpdata);
 }
 
-static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
+static int cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
 {
 	struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc);
 	unsigned long flags;
@@ -565,6 +567,8 @@ static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
 	__cpm1_gpio32_set(cpm1_gc, pin_mask, value);
 
 	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
+
+	return 0;
 }
 
 static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
@@ -618,7 +622,7 @@ int cpm1_gpiochip_add32(struct device *dev)
 	gc->direction_input = cpm1_gpio32_dir_in;
 	gc->direction_output = cpm1_gpio32_dir_out;
 	gc->get = cpm1_gpio32_get;
-	gc->set = cpm1_gpio32_set;
+	gc->set_rv = cpm1_gpio32_set;
 	gc->parent = dev;
 	gc->owner = THIS_MODULE;
 

-- 
2.45.2

Re: [PATCH 0/5] powerpc: convert board-file GPIO chips to using new value setters

[Bartosz Golaszewski]

On Tue, Apr 8, 2025 at 9:21 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. We're in the process of
> converting all GPIO drivers to using the new API. This series converts
> all powerpc board-file level controllers.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> Bartosz Golaszewski (5):
>       powerpc: sysdev/gpio: use new line value setter callbacks
>       powerpc: 83xx/gpio: use new line value setter callbacks
>       powerpc: 44x/gpio: use new line value setter callbacks
>       powerpc: 52xx/gpio: use new line value setter callbacks
>       powerpc: 8xx/gpio: use new line value setter callbacks
>
>  arch/powerpc/platforms/44x/gpio.c              |  7 ++++---
>  arch/powerpc/platforms/52xx/mpc52xx_gpt.c      |  6 ++++--
>  arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c |  6 ++++--
>  arch/powerpc/platforms/8xx/cpm1.c              | 12 ++++++++----
>  arch/powerpc/sysdev/cpm_common.c               |  6 ++++--
>  5 files changed, 24 insertions(+), 13 deletions(-)
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250326-gpiochip-set-rv-powerpc-1e98d28222aa
>
> Best regards,
> --
> Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>

Gentle ping.

Bart

Re: [PATCH 0/5] powerpc: convert board-file GPIO chips to using new value setters

[Linus Walleij]

On Tue, Apr 8, 2025 at 9:21 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. We're in the process of
> converting all GPIO drivers to using the new API. This series converts
> all powerpc board-file level controllers.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

Yours,
Linus Walleij

Re: [PATCH 2/5] powerpc: 83xx/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>   arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> index 4d8fa9ed1a67..d4ba6dbb86b2 100644
> --- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> +++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> @@ -92,7 +92,7 @@ static void mcu_power_off(void)
>   	mutex_unlock(&mcu->lock);
>   }
>   
> -static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   {
>   	struct mcu *mcu = gpiochip_get_data(gc);
>   	u8 bit = 1 << (4 + gpio);
> @@ -105,6 +105,8 @@ static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   
>   	i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
>   	mutex_unlock(&mcu->lock);
> +
> +	return 0;

i2c_smbus_write_byte_data() can fail, why not return the value returned 
by i2c_smbus_write_byte_data() ?

>   }
>   
>   static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> @@ -123,7 +125,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
>   	gc->can_sleep = 1;
>   	gc->ngpio = MCU_NUM_GPIO;
>   	gc->base = -1;
> -	gc->set = mcu_gpio_set;
> +	gc->set_rv = mcu_gpio_set;
>   	gc->direction_output = mcu_gpio_dir_out;
>   	gc->parent = dev;
>   
> 

Re: [PATCH 1/5] powerpc: sysdev/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/sysdev/cpm_common.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
> index 47db732981a8..e22fc638dbc7 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -138,7 +138,7 @@ static void __cpm2_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask,
>   	out_be32(&iop->dat, cpm2_gc->cpdata);
>   }
>   
> -static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
> +static int cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   {
>   	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>   	struct cpm2_gpio32_chip *cpm2_gc = gpiochip_get_data(gc);
> @@ -150,6 +150,8 @@ static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   	__cpm2_gpio32_set(mm_gc, pin_mask, value);
>   
>   	spin_unlock_irqrestore(&cpm2_gc->lock, flags);
> +
> +	return 0;
>   }
>   
>   static int cpm2_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> @@ -208,7 +210,7 @@ int cpm2_gpiochip_add32(struct device *dev)
>   	gc->direction_input = cpm2_gpio32_dir_in;
>   	gc->direction_output = cpm2_gpio32_dir_out;
>   	gc->get = cpm2_gpio32_get;
> -	gc->set = cpm2_gpio32_set;
> +	gc->set_rv = cpm2_gpio32_set;
>   	gc->parent = dev;
>   	gc->owner = THIS_MODULE;
>   
> 

Re: [PATCH 3/5] powerpc: 44x/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/44x/gpio.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/44x/gpio.c b/arch/powerpc/platforms/44x/gpio.c
> index e5f2319e5cbe..d540e261d85a 100644
> --- a/arch/powerpc/platforms/44x/gpio.c
> +++ b/arch/powerpc/platforms/44x/gpio.c
> @@ -75,8 +75,7 @@ __ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   		clrbits32(&regs->or, GPIO_MASK(gpio));
>   }
>   
> -static void
> -ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   {
>   	struct ppc4xx_gpio_chip *chip = gpiochip_get_data(gc);
>   	unsigned long flags;
> @@ -88,6 +87,8 @@ ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   	spin_unlock_irqrestore(&chip->lock, flags);
>   
>   	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> +	return 0;
>   }
>   
>   static int ppc4xx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> @@ -179,7 +180,7 @@ static int __init ppc4xx_add_gpiochips(void)
>   		gc->direction_input = ppc4xx_gpio_dir_in;
>   		gc->direction_output = ppc4xx_gpio_dir_out;
>   		gc->get = ppc4xx_gpio_get;
> -		gc->set = ppc4xx_gpio_set;
> +		gc->set_rv = ppc4xx_gpio_set;
>   
>   		ret = of_mm_gpiochip_add_data(np, mm_gc, ppc4xx_gc);
>   		if (ret)
> 

Re: [PATCH 4/5] powerpc: 52xx/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> index 1ea591ec6083..c96af6b0eab4 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -280,7 +280,7 @@ static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>   	return (in_be32(&gpt->regs->status) >> 8) & 1;
>   }
>   
> -static void
> +static int
>   mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int v)
>   {
>   	struct mpc52xx_gpt_priv *gpt = gpiochip_get_data(gc);
> @@ -293,6 +293,8 @@ mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int v)
>   	raw_spin_lock_irqsave(&gpt->lock, flags);
>   	clrsetbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK, r);
>   	raw_spin_unlock_irqrestore(&gpt->lock, flags);
> +
> +	return 0;
>   }
>   
>   static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> @@ -334,7 +336,7 @@ static void mpc52xx_gpt_gpio_setup(struct mpc52xx_gpt_priv *gpt)
>   	gpt->gc.direction_input  = mpc52xx_gpt_gpio_dir_in;
>   	gpt->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
>   	gpt->gc.get = mpc52xx_gpt_gpio_get;
> -	gpt->gc.set = mpc52xx_gpt_gpio_set;
> +	gpt->gc.set_rv = mpc52xx_gpt_gpio_set;
>   	gpt->gc.base = -1;
>   	gpt->gc.parent = gpt->dev;
>   
> 

Re: [PATCH 5/5] powerpc: 8xx/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Christophe Leroy <christophe.leroy@csgroup.eu> # powerpc 8xx

> ---
>   arch/powerpc/platforms/8xx/cpm1.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/8xx/cpm1.c b/arch/powerpc/platforms/8xx/cpm1.c
> index 1dc095ad48fc..7462c221115c 100644
> --- a/arch/powerpc/platforms/8xx/cpm1.c
> +++ b/arch/powerpc/platforms/8xx/cpm1.c
> @@ -417,7 +417,7 @@ static void __cpm1_gpio16_set(struct cpm1_gpio16_chip *cpm1_gc, u16 pin_mask, in
>   	out_be16(&iop->dat, cpm1_gc->cpdata);
>   }
>   
> -static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
> +static int cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   {
>   	struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc);
>   	unsigned long flags;
> @@ -428,6 +428,8 @@ static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   	__cpm1_gpio16_set(cpm1_gc, pin_mask, value);
>   
>   	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
> +
> +	return 0;
>   }
>   
>   static int cpm1_gpio16_to_irq(struct gpio_chip *gc, unsigned int gpio)
> @@ -497,7 +499,7 @@ int cpm1_gpiochip_add16(struct device *dev)
>   	gc->direction_input = cpm1_gpio16_dir_in;
>   	gc->direction_output = cpm1_gpio16_dir_out;
>   	gc->get = cpm1_gpio16_get;
> -	gc->set = cpm1_gpio16_set;
> +	gc->set_rv = cpm1_gpio16_set;
>   	gc->to_irq = cpm1_gpio16_to_irq;
>   	gc->parent = dev;
>   	gc->owner = THIS_MODULE;
> @@ -554,7 +556,7 @@ static void __cpm1_gpio32_set(struct cpm1_gpio32_chip *cpm1_gc, u32 pin_mask, in
>   	out_be32(&iop->dat, cpm1_gc->cpdata);
>   }
>   
> -static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
> +static int cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   {
>   	struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc);
>   	unsigned long flags;
> @@ -565,6 +567,8 @@ static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value)
>   	__cpm1_gpio32_set(cpm1_gc, pin_mask, value);
>   
>   	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
> +
> +	return 0;
>   }
>   
>   static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> @@ -618,7 +622,7 @@ int cpm1_gpiochip_add32(struct device *dev)
>   	gc->direction_input = cpm1_gpio32_dir_in;
>   	gc->direction_output = cpm1_gpio32_dir_out;
>   	gc->get = cpm1_gpio32_get;
> -	gc->set = cpm1_gpio32_set;
> +	gc->set_rv = cpm1_gpio32_set;
>   	gc->parent = dev;
>   	gc->owner = THIS_MODULE;
>   
> 

Re: [PATCH 2/5] powerpc: 83xx/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

On Wed, Apr 30, 2025 at 7:33 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > struct gpio_chip now has callbacks for setting line values that return
> > an integer, allowing to indicate failures. Convert the driver to using
> > them.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >   arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> > index 4d8fa9ed1a67..d4ba6dbb86b2 100644
> > --- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> > +++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> > @@ -92,7 +92,7 @@ static void mcu_power_off(void)
> >       mutex_unlock(&mcu->lock);
> >   }
> >
> > -static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > +static int mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >   {
> >       struct mcu *mcu = gpiochip_get_data(gc);
> >       u8 bit = 1 << (4 + gpio);
> > @@ -105,6 +105,8 @@ static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >
> >       i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
> >       mutex_unlock(&mcu->lock);
> > +
> > +     return 0;
>
> i2c_smbus_write_byte_data() can fail, why not return the value returned
> by i2c_smbus_write_byte_data() ?
>

The calls to i2c_smbus_write_byte_data() in this driver are
universally not checked. I cannot test it and wasn't sure if that's on
purpose so I decided to stay safe. Someone who has access to this
platform could potentially fix it across the file.

Bartosz

> >   }
> >
> >   static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> > @@ -123,7 +125,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
> >       gc->can_sleep = 1;
> >       gc->ngpio = MCU_NUM_GPIO;
> >       gc->base = -1;
> > -     gc->set = mcu_gpio_set;
> > +     gc->set_rv = mcu_gpio_set;
> >       gc->direction_output = mcu_gpio_dir_out;
> >       gc->parent = dev;
> >
> >
>

Re: [PATCH 2/5] powerpc: 83xx/gpio: use new line value setter callbacks

[Christophe Leroy]

Le 30/04/2025 à 19:37, Bartosz Golaszewski a écrit :
> On Wed, Apr 30, 2025 at 7:33 PM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> struct gpio_chip now has callbacks for setting line values that return
>>> an integer, allowing to indicate failures. Convert the driver to using
>>> them.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>> ---
>>>    arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
>>> index 4d8fa9ed1a67..d4ba6dbb86b2 100644
>>> --- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
>>> +++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
>>> @@ -92,7 +92,7 @@ static void mcu_power_off(void)
>>>        mutex_unlock(&mcu->lock);
>>>    }
>>>
>>> -static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +static int mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>>    {
>>>        struct mcu *mcu = gpiochip_get_data(gc);
>>>        u8 bit = 1 << (4 + gpio);
>>> @@ -105,6 +105,8 @@ static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>>
>>>        i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
>>>        mutex_unlock(&mcu->lock);
>>> +
>>> +     return 0;
>>
>> i2c_smbus_write_byte_data() can fail, why not return the value returned
>> by i2c_smbus_write_byte_data() ?
>>
> 
> The calls to i2c_smbus_write_byte_data() in this driver are
> universally not checked. I cannot test it and wasn't sure if that's on
> purpose so I decided to stay safe. Someone who has access to this
> platform could potentially fix it across the file.

As far as I can see this function is called three times in this file.

First time is in mcu_power_off(), which must return void.
Second time is inside a forever loop in shutdown_thread_fn(), and I 
can't see what could be done with the returned value.

Last time is in the function you are changing. Wouldn't it make sense to 
take the value into account here ? IIUC it is the purpose of the change, 
isn't it ?

Christophe

Re: [PATCH 2/5] powerpc: 83xx/gpio: use new line value setter callbacks

[Bartosz Golaszewski]

On Wed, Apr 30, 2025 at 7:47 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/04/2025 à 19:37, Bartosz Golaszewski a écrit :
> > On Wed, Apr 30, 2025 at 7:33 PM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >>
> >>
> >> Le 08/04/2025 à 09:21, Bartosz Golaszewski a écrit :
> >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>>
> >>> struct gpio_chip now has callbacks for setting line values that return
> >>> an integer, allowing to indicate failures. Convert the driver to using
> >>> them.
> >>>
> >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>> ---
> >>>    arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 6 ++++--
> >>>    1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> >>> index 4d8fa9ed1a67..d4ba6dbb86b2 100644
> >>> --- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> >>> +++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
> >>> @@ -92,7 +92,7 @@ static void mcu_power_off(void)
> >>>        mutex_unlock(&mcu->lock);
> >>>    }
> >>>
> >>> -static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >>> +static int mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >>>    {
> >>>        struct mcu *mcu = gpiochip_get_data(gc);
> >>>        u8 bit = 1 << (4 + gpio);
> >>> @@ -105,6 +105,8 @@ static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >>>
> >>>        i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
> >>>        mutex_unlock(&mcu->lock);
> >>> +
> >>> +     return 0;
> >>
> >> i2c_smbus_write_byte_data() can fail, why not return the value returned
> >> by i2c_smbus_write_byte_data() ?
> >>
> >
> > The calls to i2c_smbus_write_byte_data() in this driver are
> > universally not checked. I cannot test it and wasn't sure if that's on
> > purpose so I decided to stay safe. Someone who has access to this
> > platform could potentially fix it across the file.
>
> As far as I can see this function is called three times in this file.
>
> First time is in mcu_power_off(), which must return void.
> Second time is inside a forever loop in shutdown_thread_fn(), and I
> can't see what could be done with the returned value.
>
> Last time is in the function you are changing. Wouldn't it make sense to
> take the value into account here ? IIUC it is the purpose of the change,
> isn't it ?
>
> Christophe
>

Sure, I can do it. The purpose is first and foremost to convert all
drivers so that we can drop the old callbacks but I see what you mean.

Bart