Re: [PATCH v7 13/34] pinctrl: airoha: add set_direction() helper for gpio_chip

[Lorenzo Bianconi]

> The patch creates set_direction() helper for gpio_chip abstraction.
> It also implements/reimplements some function using newly defined
> helper.
> 
> This is cosmetic patch used to place gpio_chip specific code together.
> No functional changes.
> 
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@xxxxxxxxx>
> ---
>  drivers/pinctrl/airoha/pinctrl-airoha.c | 59 ++++++++++++++++---------
>  1 file changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index b52eb39c55ff3..faad5d3ada31c 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2523,12 +2523,43 @@ static int airoha_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio)
>  	return (val & mask) ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
>  }
>  
> +static int airoha_gpio_set_direction(struct gpio_chip *chip, unsigned int gpio,
> +				     bool input)
> +{
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(chip);
> +	u32 mask, index;
> +	int err, field_shift;
> +
> +	/* set output enable */
> +	mask = BIT(gpio % AIROHA_PIN_BANK_SIZE);
> +	index = gpio / AIROHA_PIN_BANK_SIZE;
> +	err = regmap_update_bits(pinctrl->regmap, pinctrl->gpiochip.out[index],
> +				 mask, !input ? mask : 0);
> +	if (err)
> +		return err;
> +
> +	/* set direction */
> +	field_shift = 2 * (gpio % AIROHA_REG_GPIOCTRL_NUM_PIN);
> +	mask = GENMASK(field_shift + 1, field_shift);

same here, why using 2 bits as mask?

> +	index = gpio / AIROHA_REG_GPIOCTRL_NUM_PIN;
> +
> +	return regmap_update_bits(pinctrl->regmap,
> +				  pinctrl->gpiochip.dir[index],
> +				  mask, !input ? BIT(field_shift) : 0);
> +}
> +
> +static int airoha_gpio_direction_input(struct gpio_chip *chip,
> +				       unsigned int gpio)
> +{
> +	return airoha_gpio_set_direction(chip, gpio, true);
> +}
> +
>  static int airoha_gpio_direction_output(struct gpio_chip *chip,
>  					unsigned int gpio, int value)
>  {
>  	int err;
>  
> -	err = pinctrl_gpio_direction_output(chip, gpio);
> +	err = airoha_gpio_set_direction(chip, gpio, false);
>  	if (err)
>  		return err;
>  
> @@ -2667,7 +2698,7 @@ static int airoha_pinctrl_add_gpiochip(struct airoha_pinctrl *pinctrl,
>  	gc->label = dev_name(dev);
>  	gc->request = gpiochip_generic_request;
>  	gc->free = gpiochip_generic_free;
> -	gc->direction_input = pinctrl_gpio_direction_input;
> +	gc->direction_input = airoha_gpio_direction_input;
>  	gc->direction_output = airoha_gpio_direction_output;
>  	gc->get_direction = airoha_gpio_get_direction;
>  	gc->set = airoha_gpio_set;
> @@ -2750,27 +2781,13 @@ static int airoha_pinmux_set_direction(struct pinctrl_dev *pctrl_dev,
>  					unsigned int p, bool input)
>  {
>  	struct airoha_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
> -	u32 mask, index;
> -	int err, pin;
> +	int gpio;
>  
> -	pin = airoha_convert_pin_to_reg_offset(pctrl_dev, range, p);
> -	if (pin < 0)
> -		return pin;
> -
> -	/* set output enable */
> -	mask = BIT(pin % AIROHA_PIN_BANK_SIZE);
> -	index = pin / AIROHA_PIN_BANK_SIZE;
> -	err = regmap_update_bits(pinctrl->regmap, pinctrl->gpiochip.out[index],
> -				 mask, !input ? mask : 0);
> -	if (err)
> -		return err;
> +	gpio = airoha_convert_pin_to_reg_offset(pctrl_dev, range, p);

same here, if you do not rename pin in gpio the patch will be simpler.

Regards,
Lorenzo

> +	if (gpio < 0)
> +		return gpio;
>  
> -	/* set direction */
> -	mask = BIT(2 * (pin % AIROHA_REG_GPIOCTRL_NUM_PIN));
> -	index = pin / AIROHA_REG_GPIOCTRL_NUM_PIN;
> -	return regmap_update_bits(pinctrl->regmap,
> -				  pinctrl->gpiochip.dir[index], mask,
> -				  !input ? mask : 0);
> +	return airoha_gpio_set_direction(&pinctrl->gpiochip.chip, gpio, input);
>  }
>  
>  static const struct pinmux_ops airoha_pmxops = {
> -- 
> 2.53.0
>

Attachment: signature.asc
Description: PGP signature