Re: [PATCH 2/3] pinctrl: cy8c95x0: Use regmap ranges

[Andy Shevchenko <andy@black.fi.intel.com>]

On Tue, May 21, 2024 at 05:25:58PM +0200, Patrick Rudolph wrote:
> Instead of implementing a custom register paging mechanism in
> the driver use the existing regmap ranges feature.

...

> +#define MUXED_STRIDE		(CY8C95X0_DRV_HIZ - CY8C95X0_INTMASK)

> +#define CY8C95X0_MUX_REGMAP_TO_OFFSET(x, p) \
> +	(CY8C95X0_VIRTUAL + (x) - CY8C95X0_INTMASK + (p) * MUXED_STRIDE)

> +static const struct regmap_range_cfg cy8c95x0_ranges[] = {
> +	{
> +		.range_min = CY8C95X0_VIRTUAL,
> +		.range_max = 0,		/* Updated at runtime */
> +		.selector_reg = CY8C95X0_PORTSEL,
> +		.selector_mask = 0x07,
> +		.selector_shift = 0x0,
> +		.window_start = CY8C95X0_INTMASK,
> +		.window_len = MUXED_STRIDE,

Looking at this again, are you sure you have no off-by-one error in
MUXED_STRIDE value?

Also a comment in the regmap core suggests that we may include selector
in each of the window.

With this, we probably should simply use PORTSEL as window start with a
fixed window len of 16, having a few more (reserved) registers in the
dump seems not a big deal to me, but it will be much easier to decipher
a port number based on (virtual) offset.

Besides above, why virtual start is not well aligned? I would expect not
0x31, but 0x40 or alike. It might explain some bugs with cache you have
seen.

P.S. It might still be bugs in regmap core, if it is the case, they
need to be addressed.

> +	}
>  };

-- 
With Best Regards,
Andy Shevchenko