Re: [PATCH v7 12/34] pinctrl: airoha: add missed get_direction() function for gpio_chip

[Lorenzo Bianconi]

> This patch adds missed get_direction() function for gpio_chip.
> Also it reimplements pinconf's get_direction() function using
> newly defined function.
> 
> Fixes: 1c8ace2d0725 ("pinctrl: airoha: Add support for EN7581 SoC")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@xxxxxxxxx>
> ---
>  drivers/pinctrl/airoha/pinctrl-airoha.c | 41 ++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index 820b7b0443851..b52eb39c55ff3 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2502,6 +2502,27 @@ static int airoha_gpio_get(struct gpio_chip *chip, unsigned int gpio)
>  	return err ? err : !!(val & BIT(pin));
>  }
>  
> +static int airoha_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio)
> +{
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(chip);
> +	u32 mask, index, val;
> +	int err, field_shift;
> +
> +	field_shift = 2 * (gpio % AIROHA_REG_GPIOCTRL_NUM_PIN);
> +	mask = GENMASK(field_shift + 1, field_shift);

before we where checking just BIT(field_shift) as mask, while now we are
checking two bits.

> +	index = gpio / AIROHA_REG_GPIOCTRL_NUM_PIN;
> +
> +	err = regmap_read(pinctrl->regmap,
> +			  pinctrl->gpiochip.dir[index], &val);
> +	if (err)
> +		return err;
> +
> +	if ((val & mask) > BIT(field_shift))
> +		return -EINVAL;
> +
> +	return (val & mask) ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;

nit: unnecessary brackets.

> +}
> +
>  static int airoha_gpio_direction_output(struct gpio_chip *chip,
>  					unsigned int gpio, int value)
>  {
> @@ -2648,6 +2669,7 @@ static int airoha_pinctrl_add_gpiochip(struct airoha_pinctrl *pinctrl,
>  	gc->free = gpiochip_generic_free;
>  	gc->direction_input = pinctrl_gpio_direction_input;
>  	gc->direction_output = airoha_gpio_direction_output;
> +	gc->get_direction = airoha_gpio_get_direction;
>  	gc->set = airoha_gpio_set;
>  	gc->get = airoha_gpio_get;
>  	gc->base = -1;
> @@ -2855,21 +2877,18 @@ static int airoha_pinctrl_set_conf(struct airoha_pinctrl *pinctrl,
>  static int airoha_pinconf_get_direction(struct pinctrl_dev *pctrl_dev, u32 p)
>  {
>  	struct airoha_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
> -	u32 val, mask;
> -	int err, pin;
> -	u8 index;
> +	int err, gpio;
>  
> -	pin = airoha_convert_pin_to_reg_offset(pctrl_dev, NULL, p);
> -	if (pin < 0)
> -		return pin;
> +	gpio = airoha_convert_pin_to_reg_offset(pctrl_dev, NULL, p);

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

Regards,
Lorenzo

> +	if (gpio < 0)
> +		return gpio;
>  
> -	index = pin / AIROHA_REG_GPIOCTRL_NUM_PIN;
> -	err = regmap_read(pinctrl->regmap, pinctrl->gpiochip.dir[index], &val);
> -	if (err)
> +	err = airoha_gpio_get_direction(&pinctrl->gpiochip.chip, gpio);
> +	if (err < 0)
>  		return err;
>  
> -	mask = BIT(2 * (pin % AIROHA_REG_GPIOCTRL_NUM_PIN));
> -	return val & mask ? PIN_CONFIG_OUTPUT_ENABLE : PIN_CONFIG_INPUT_ENABLE;
> +	return err == GPIO_LINE_DIRECTION_OUT ?
> +	       PIN_CONFIG_OUTPUT_ENABLE : PIN_CONFIG_INPUT_ENABLE;
>  }
>  
>  static int airoha_pinconf_get(struct pinctrl_dev *pctrl_dev,
> -- 
> 2.53.0
>

Attachment: signature.asc
Description: PGP signature