Re: [PATCH 06/14] gpio: regmap: use value returning setters

[Michael Walle]

Hi,

On Tue Feb 11, 2025 at 1:09 PM CET, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
>
> struct gpio_chip now has additional variants of the set(_multiple)
> driver callbacks that return an integer to indicate success or failure.
> Convert the driver to using them.

Great!

> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> ---
>  drivers/gpio/gpio-regmap.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 05f8781b5204..e3b4e392549b 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -81,22 +81,25 @@ static int gpio_regmap_get(struct gpio_chip *chip, unsigned int offset)
>  	return !!(val & mask);
>  }
>  
> -static void gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
> -			    int val)
> +static int gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
> +			   int val)
>  {
>  	struct gpio_regmap *gpio = gpiochip_get_data(chip);
>  	unsigned int base = gpio_regmap_addr(gpio->reg_set_base);
>  	unsigned int reg, mask;
> +	int ret;
>  
>  	gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);

reg_mask_xlate() might fail. Please also check the return code.

>  	if (val)
> -		regmap_update_bits(gpio->regmap, reg, mask, mask);
> +		ret = regmap_update_bits(gpio->regmap, reg, mask, mask);
>  	else
> -		regmap_update_bits(gpio->regmap, reg, mask, 0);
> +		ret = regmap_update_bits(gpio->regmap, reg, mask, 0);
> +
> +	return ret;
>  }
>  
> -static void gpio_regmap_set_with_clear(struct gpio_chip *chip,
> -				       unsigned int offset, int val)
> +static int gpio_regmap_set_with_clear(struct gpio_chip *chip,
> +				      unsigned int offset, int val)
>  {
>  	struct gpio_regmap *gpio = gpiochip_get_data(chip);
>  	unsigned int base, reg, mask;
> @@ -107,7 +110,7 @@ static void gpio_regmap_set_with_clear(struct gpio_chip *chip,
>  		base = gpio_regmap_addr(gpio->reg_clr_base);
>  
>  	gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);

same same :)

-michael

> -	regmap_write(gpio->regmap, reg, mask);
> +	return regmap_write(gpio->regmap, reg, mask);
>  }
>  
>  static int gpio_regmap_get_direction(struct gpio_chip *chip,
> @@ -266,9 +269,9 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
>  	chip->free = gpiochip_generic_free;
>  	chip->get = gpio_regmap_get;
>  	if (gpio->reg_set_base && gpio->reg_clr_base)
> -		chip->set = gpio_regmap_set_with_clear;
> +		chip->set_rv = gpio_regmap_set_with_clear;
>  	else if (gpio->reg_set_base)
> -		chip->set = gpio_regmap_set;
> +		chip->set_rv = gpio_regmap_set;
>  
>  	chip->get_direction = gpio_regmap_get_direction;
>  	if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {

Attachment: signature.asc
Description: PGP signature