Re: [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access

[andriy.shevchenko@xxxxxxxxx]

On Fri, Oct 02, 2026 at 02:25:48PM +0000, Kanak Shilledar wrote:

> On Fri, 2026-10-02 at 16:12 +0300, Andy Shevchenko wrote:
> > On Fri, Oct 02, 2026 at 01:54:29PM +0200, Kanak Shilledar wrote:

...

> > > +static int inv_icm42607_mreg_read(struct regmap *map, unsigned int
> > > reg,
> > > +				  u8 *data, size_t count)
> > > +{
> > > +	unsigned int val;
> > > +	bool idle_set;
> > > +	u8 blk_sel;
> > > +	int ret;
> > > +
> > > +	/* MREG access is one byte per transaction, no burst
> > > support. */
> > > +	if (count != 1)
> > > +		return -EINVAL;
> > > +
> > > +	ret = inv_icm42607_blk_sel(reg, &blk_sel);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = inv_icm42607_mclk_get(map, &idle_set);
> > > +	if (ret)
> > > +		return ret;
> > 
> > > +	ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R,
> > > blk_sel);
> > > +	if (ret)
> > > +		goto out;
> > 
> > So, can we use regmap ranges instead?
> 
> We can't use regmap ranges because the register accesses for different
> banks guarded by a specific routine of writing the bank selector, the
> address pointer and then finally accessing the value along with
> checking for timings and clocks. There is also a limitation that,
> accessing the registers in banks other than USER BANK 0 can only be
> done serially. It doesn't support bulk reads. This is documented in
> section 13 of the datasheet [1].

Sorry if I missed it, but this is an important part of the driver design,
if not yet, please add a respective note to the commit message on it.

> > > +	ret = regmap_write(map, INV_ICM42607_REG_MADDR_R,
> > > +			   FIELD_GET(INV_ICM42607_REG_ADDR_MASK,
> > > reg));
> > > +	if (ret)
> > > +		goto out;
> > > +
> > > +	fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> > > +
> > > +	ret = regmap_read(map, INV_ICM42607_REG_M_R, &val);
> > > +	if (ret)
> > > +		goto out;
> > > +
> > > +	fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> > > +
> > > +	*data = val;
> > > +out:
> > > +	/* Restore direct access. */
> > > +	ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, 0);
> > > +	inv_icm42607_mclk_put(map, idle_set);
> > > +
> > > +	return ret;
> > >  }

...

> > > +static const struct regmap_config inv_icm42607_regmap_config = {
> > > +	.reg_bits = 8,
> > > +	.val_bits = 8,
> > 
> > No cache? Why?
> As the virtual regmap config has some caching for USER BANK 0 registers
> only. The indirect banks doesn't support caching [1].

This is strange. You mean they are volatile by definition (HW)?
Otherwise if it's just a software issue, it can still be done,
for regcache there is no difference if the offset is real or paged.

> > > +};

> [1] Datasheet: https://www.lcsc.com/product-detail/C5129967.html

-- 
With Best Regards,
Andy Shevchenko