Re: [PATCH 2/2] pinctrl: ocelot: fix pinconf

[Colin Foster <colin.foster@xxxxxxxxxxxxxxxx>]

Hi Robert,

On Thu, Jul 14, 2022 at 12:51:51AM +0200, Robert Marko wrote:
> Commit "pinctrl: ocelot: convert pinctrl to regmap" moved to using
> regmap_read/write, however it neglected to also carry out alignment
> to register stride of 4.
> 
> This would cause the following error:
> [    1.720873] pinctrl-ocelot 6110101e0.pinctrl: pin_config_set op failed for pin 34
> [    1.728110] sdhci-sparx5 600800000.mmc: Error applying setting, reverse things back
> 
> So, regmap_read would return -EINVAL as it was being passed address
> of the pin without stride, so for example pin 34 would end up being
> 0x22 in hex.
> 
> Fix this by accouting for the stride in register address.

Sorry for the bug. Horaitu found this as well and recently submitted
patches:

https://patchwork.ozlabs.org/project/linux-gpio/patch/20220713193750.4079621-3-horatiu.vultur@xxxxxxxxxxxxx/

The second patch in his set fixes both of these issues (reg_stride and
max_register).

> 
> Fixes: 076d9e71bcf8 ("pinctrl: ocelot: convert pinctrl to regmap")
> Signed-off-by: Robert Marko <robert.marko@xxxxxxxxxx>
> ---
>  drivers/pinctrl/pinctrl-ocelot.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
> index 84bfbe649b67..a71145367b15 100644
> --- a/drivers/pinctrl/pinctrl-ocelot.c
> +++ b/drivers/pinctrl/pinctrl-ocelot.c
> @@ -1327,7 +1327,9 @@ static int ocelot_hw_get_value(struct ocelot_pinctrl *info,
>  	if (info->pincfg) {
>  		u32 regcfg;
>  
> -		ret = regmap_read(info->pincfg, pin, &regcfg);
> +		ret = regmap_read(info->pincfg,
> +				  pin * regmap_get_reg_stride(info->pincfg),
> +				  &regcfg);
>  		if (ret)
>  			return ret;
>  
> @@ -1359,14 +1361,18 @@ static int ocelot_pincfg_clrsetbits(struct ocelot_pinctrl *info, u32 regaddr,
>  	u32 val;
>  	int ret;
>  
> -	ret = regmap_read(info->pincfg, regaddr, &val);
> +	ret = regmap_read(info->pincfg,
> +			  regaddr * regmap_get_reg_stride(info->pincfg),
> +			  &val);
>  	if (ret)
>  		return ret;
>  
>  	val &= ~clrbits;
>  	val |= setbits;
>  
> -	ret = regmap_write(info->pincfg, regaddr, val);
> +	ret = regmap_write(info->pincfg,
> +			   regaddr * regmap_get_reg_stride(info->pincfg),
> +			   val);
>  
>  	return ret;
>  }
> -- 
> 2.36.1
>