[PATCH v3] clk: rs9: Fix I2C accessors

[PATCH v3] clk: rs9: Fix I2C accessors

[Marek Vasut]

Add custom I2C accessors to this driver, since the regular I2C regmap ones
do not generate the exact I2C transfers required by the chip. On I2C write,
it is mandatory to send transfer length first, on read the chip returns the
transfer length in first byte. Instead of always reading back 8 bytes, which
is the default and also the size of the entire register file, set BCP register
to 1 to read out 1 byte which is less wasteful.

Fixes: 892e0ddea1aa6 ("clk: rs9: Add Renesas 9-series PCIe clock generator driver")
Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Marek Vasut <marex@denx.de>
---
V2: Fix endianness handling in rs9_regmap_i2c_read() i2c_transfer
V3: - Disable regcache, the driver does a couple of I2C writes on boot
      and that is all, so it only adds complexity
    - Set regmap max_register to RS9_REG_BCP which is the correct one
---
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk-renesas-pcie.c | 60 ++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
index 4f5df1fc74b46..138aaab05fc8a 100644
--- a/drivers/clk/clk-renesas-pcie.c
+++ b/drivers/clk/clk-renesas-pcie.c
@@ -90,13 +90,61 @@ static const struct regmap_access_table rs9_writeable_table = {
 	.n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges),
 };
 
+static int rs9_regmap_i2c_write(void *context,
+				unsigned int reg, unsigned int val)
+{
+	struct i2c_client *i2c = context;
+	const u8 data[3] = { reg, 1, val };
+	const int count = ARRAY_SIZE(data);
+	int ret;
+
+	ret = i2c_master_send(i2c, data, count);
+	if (ret == count)
+		return 0;
+	else if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static int rs9_regmap_i2c_read(void *context,
+			       unsigned int reg, unsigned int *val)
+{
+	struct i2c_client *i2c = context;
+	struct i2c_msg xfer[2];
+	u8 txdata = reg;
+	u8 rxdata[2];
+	int ret;
+
+	xfer[0].addr = i2c->addr;
+	xfer[0].flags = 0;
+	xfer[0].len = 1;
+	xfer[0].buf = (void *)&txdata;
+
+	xfer[1].addr = i2c->addr;
+	xfer[1].flags = I2C_M_RD | I2C_M_RECV_LEN;
+	xfer[1].len = 1;
+	xfer[1].buf = (void *)rxdata;
+
+	ret = i2c_transfer(i2c->adapter, xfer, 2);
+	if (ret < 0)
+		return ret;
+	if (ret != 2)
+		return -EIO;
+
+	*val = rxdata[1];
+	return 0;
+}
+
 static const struct regmap_config rs9_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
-	.cache_type = REGCACHE_FLAT,
-	.max_register = 0x8,
+	.cache_type = REGCACHE_NONE,
+	.max_register = RS9_REG_BCP,
 	.rd_table = &rs9_readable_table,
 	.wr_table = &rs9_writeable_table,
+	.reg_write = rs9_regmap_i2c_write,
+	.reg_read = rs9_regmap_i2c_read,
 };
 
 static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
@@ -242,11 +290,17 @@ static int rs9_probe(struct i2c_client *client)
 			return ret;
 	}
 
-	rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config);
+	rs9->regmap = devm_regmap_init(&client->dev, NULL,
+				       client, &rs9_regmap_config);
 	if (IS_ERR(rs9->regmap))
 		return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap),
 				     "Failed to allocate register map\n");
 
+	/* Always read back 1 Byte via I2C */
+	ret = regmap_write(rs9->regmap, RS9_REG_BCP, 1);
+	if (ret < 0)
+		return ret;
+
 	/* Register clock */
 	for (i = 0; i < rs9->chip_info->num_clks; i++) {
 		snprintf(name, 5, "DIF%d", i);
-- 
2.35.1

Re: [PATCH v3] clk: rs9: Fix I2C accessors

[Alexander Stein]

Hi Marek,

thanks for the update.

Am Dienstag, 27. September 2022, 23:44:14 CEST schrieb Marek Vasut:
> Add custom I2C accessors to this driver, since the regular I2C regmap ones
> do not generate the exact I2C transfers required by the chip. On I2C write,
> it is mandatory to send transfer length first, on read the chip returns the
> transfer length in first byte. Instead of always reading back 8 bytes, which
> is the default and also the size of the entire register file, set BCP
> register to 1 to read out 1 byte which is less wasteful.
> 
> Fixes: 892e0ddea1aa6 ("clk: rs9: Add Renesas 9-series PCIe clock generator
> driver") Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> V2: Fix endianness handling in rs9_regmap_i2c_read() i2c_transfer
> V3: - Disable regcache, the driver does a couple of I2C writes on boot
>       and that is all, so it only adds complexity
>     - Set regmap max_register to RS9_REG_BCP which is the correct one
> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> ---
>  drivers/clk/clk-renesas-pcie.c | 60 ++++++++++++++++++++++++++++++++--
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
> index 4f5df1fc74b46..138aaab05fc8a 100644
> --- a/drivers/clk/clk-renesas-pcie.c
> +++ b/drivers/clk/clk-renesas-pcie.c
> @@ -90,13 +90,61 @@ static const struct regmap_access_table
> rs9_writeable_table = { .n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges),
>  };
> 
> +static int rs9_regmap_i2c_write(void *context,
> +				unsigned int reg, unsigned int 
val)
> +{
> +	struct i2c_client *i2c = context;
> +	const u8 data[3] = { reg, 1, val };
> +	const int count = ARRAY_SIZE(data);
> +	int ret;
> +
> +	ret = i2c_master_send(i2c, data, count);
> +	if (ret == count)
> +		return 0;
> +	else if (ret < 0)
> +		return ret;
> +	else
> +		return -EIO;
> +}
> +
> +static int rs9_regmap_i2c_read(void *context,
> +			       unsigned int reg, unsigned int *val)
> +{
> +	struct i2c_client *i2c = context;
> +	struct i2c_msg xfer[2];
> +	u8 txdata = reg;
> +	u8 rxdata[2];
> +	int ret;
> +
> +	xfer[0].addr = i2c->addr;
> +	xfer[0].flags = 0;
> +	xfer[0].len = 1;
> +	xfer[0].buf = (void *)&txdata;
> +
> +	xfer[1].addr = i2c->addr;
> +	xfer[1].flags = I2C_M_RD | I2C_M_RECV_LEN;
> +	xfer[1].len = 1;

I'm still in favor of removing I2C_M_RECV_LEN and setting len=2. This 
currently works only, because there is no read access between 
devm_regmap_init() call and regmap_write() to RS9_REG_BCP.
Enabling cache later on again this will corrupt the stack.

Best regards,
Alexander

> +	xfer[1].buf = (void *)rxdata;
> +
> +	ret = i2c_transfer(i2c->adapter, xfer, 2);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	*val = rxdata[1];
> +	return 0;
> +}
> +
>  static const struct regmap_config rs9_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
> -	.cache_type = REGCACHE_FLAT,
> -	.max_register = 0x8,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = RS9_REG_BCP,
>  	.rd_table = &rs9_readable_table,
>  	.wr_table = &rs9_writeable_table,
> +	.reg_write = rs9_regmap_i2c_write,
> +	.reg_read = rs9_regmap_i2c_read,
>  };
> 
>  static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
> @@ -242,11 +290,17 @@ static int rs9_probe(struct i2c_client *client)
>  			return ret;
>  	}
> 
> -	rs9->regmap = devm_regmap_init_i2c(client, &rs9_regmap_config);
> +	rs9->regmap = devm_regmap_init(&client->dev, NULL,
> +				       client, 
&rs9_regmap_config);
>  	if (IS_ERR(rs9->regmap))
>  		return dev_err_probe(&client->dev, PTR_ERR(rs9->regmap),
>  				     "Failed to allocate register 
map\n");
> 
> +	/* Always read back 1 Byte via I2C */
> +	ret = regmap_write(rs9->regmap, RS9_REG_BCP, 1);
> +	if (ret < 0)
> +		return ret;
> +
>  	/* Register clock */
>  	for (i = 0; i < rs9->chip_info->num_clks; i++) {
>  		snprintf(name, 5, "DIF%d", i);

Re: [PATCH v3] clk: rs9: Fix I2C accessors

[Marek Vasut]

On 9/28/22 09:35, Alexander Stein wrote:
> Hi Marek,

Hi,

> thanks for the update.

Thanks for the reviews.

> Am Dienstag, 27. September 2022, 23:44:14 CEST schrieb Marek Vasut:
>> Add custom I2C accessors to this driver, since the regular I2C regmap ones
>> do not generate the exact I2C transfers required by the chip. On I2C write,
>> it is mandatory to send transfer length first, on read the chip returns the
>> transfer length in first byte. Instead of always reading back 8 bytes, which
>> is the default and also the size of the entire register file, set BCP
>> register to 1 to read out 1 byte which is less wasteful.
>>
>> Fixes: 892e0ddea1aa6 ("clk: rs9: Add Renesas 9-series PCIe clock generator
>> driver") Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> V2: Fix endianness handling in rs9_regmap_i2c_read() i2c_transfer
>> V3: - Disable regcache, the driver does a couple of I2C writes on boot
>>        and that is all, so it only adds complexity
>>      - Set regmap max_register to RS9_REG_BCP which is the correct one
>> ---
>> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
>> Cc: Michael Turquette <mturquette@baylibre.com>
>> Cc: Stephen Boyd <sboyd@kernel.org>
>> ---
>>   drivers/clk/clk-renesas-pcie.c | 60 ++++++++++++++++++++++++++++++++--
>>   1 file changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
>> index 4f5df1fc74b46..138aaab05fc8a 100644
>> --- a/drivers/clk/clk-renesas-pcie.c
>> +++ b/drivers/clk/clk-renesas-pcie.c
>> @@ -90,13 +90,61 @@ static const struct regmap_access_table
>> rs9_writeable_table = { .n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges),
>>   };
>>
>> +static int rs9_regmap_i2c_write(void *context,
>> +				unsigned int reg, unsigned int
> val)
>> +{
>> +	struct i2c_client *i2c = context;
>> +	const u8 data[3] = { reg, 1, val };
>> +	const int count = ARRAY_SIZE(data);
>> +	int ret;
>> +
>> +	ret = i2c_master_send(i2c, data, count);
>> +	if (ret == count)
>> +		return 0;
>> +	else if (ret < 0)
>> +		return ret;
>> +	else
>> +		return -EIO;
>> +}
>> +
>> +static int rs9_regmap_i2c_read(void *context,
>> +			       unsigned int reg, unsigned int *val)
>> +{
>> +	struct i2c_client *i2c = context;
>> +	struct i2c_msg xfer[2];
>> +	u8 txdata = reg;
>> +	u8 rxdata[2];
>> +	int ret;
>> +
>> +	xfer[0].addr = i2c->addr;
>> +	xfer[0].flags = 0;
>> +	xfer[0].len = 1;
>> +	xfer[0].buf = (void *)&txdata;
>> +
>> +	xfer[1].addr = i2c->addr;
>> +	xfer[1].flags = I2C_M_RD | I2C_M_RECV_LEN;
>> +	xfer[1].len = 1;
> 
> I'm still in favor of removing I2C_M_RECV_LEN and setting len=2. This
> currently works only, because there is no read access between
> devm_regmap_init() call and regmap_write() to RS9_REG_BCP.
> Enabling cache later on again this will corrupt the stack.

The device does send you the amount of data in first byte of I2C READ, 
so I think I2C_M_RECV_LEN is the right flag here.

I had a look into the above topic too before sending V3, and I think you 
can use regcache_cache_bypass(..., true) right after devm_regmap_init() 
and then again regcache_cache_bypass(..., false) close to the end of 
probe() callback to deal with the problem you're describing.

Would that work ?

Re: [PATCH v3] clk: rs9: Fix I2C accessors

[Alexander Stein]

Hi Marek,

Am Mittwoch, 28. September 2022, 14:59:07 CEST schrieb Marek Vasut:
> On 9/28/22 09:35, Alexander Stein wrote:
> > Hi Marek,
> 
> Hi,
> 
> > thanks for the update.
> 
> Thanks for the reviews.
> 
> > Am Dienstag, 27. September 2022, 23:44:14 CEST schrieb Marek Vasut:
> >> Add custom I2C accessors to this driver, since the regular I2C regmap
> >> ones
> >> do not generate the exact I2C transfers required by the chip. On I2C
> >> write,
> >> it is mandatory to send transfer length first, on read the chip returns
> >> the
> >> transfer length in first byte. Instead of always reading back 8 bytes,
> >> which is the default and also the size of the entire register file, set
> >> BCP register to 1 to read out 1 byte which is less wasteful.
> >> 
> >> Fixes: 892e0ddea1aa6 ("clk: rs9: Add Renesas 9-series PCIe clock
> >> generator
> >> driver") Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> ---
> >> V2: Fix endianness handling in rs9_regmap_i2c_read() i2c_transfer
> >> V3: - Disable regcache, the driver does a couple of I2C writes on boot
> >> 
> >>        and that is all, so it only adds complexity
> >>      
> >>      - Set regmap max_register to RS9_REG_BCP which is the correct one
> >> 
> >> ---
> >> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> >> Cc: Michael Turquette <mturquette@baylibre.com>
> >> Cc: Stephen Boyd <sboyd@kernel.org>
> >> ---
> >> 
> >>   drivers/clk/clk-renesas-pcie.c | 60 ++++++++++++++++++++++++++++++++--
> >>   1 file changed, 57 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/clk/clk-renesas-pcie.c
> >> b/drivers/clk/clk-renesas-pcie.c index 4f5df1fc74b46..138aaab05fc8a
> >> 100644
> >> --- a/drivers/clk/clk-renesas-pcie.c
> >> +++ b/drivers/clk/clk-renesas-pcie.c
> >> @@ -90,13 +90,61 @@ static const struct regmap_access_table
> >> rs9_writeable_table = { .n_yes_ranges = ARRAY_SIZE(rs9_writeable_ranges),
> >> 
> >>   };
> >> 
> >> +static int rs9_regmap_i2c_write(void *context,
> >> +				unsigned int reg, unsigned int
> > 
> > val)
> > 
> >> +{
> >> +	struct i2c_client *i2c = context;
> >> +	const u8 data[3] = { reg, 1, val };
> >> +	const int count = ARRAY_SIZE(data);
> >> +	int ret;
> >> +
> >> +	ret = i2c_master_send(i2c, data, count);
> >> +	if (ret == count)
> >> +		return 0;
> >> +	else if (ret < 0)
> >> +		return ret;
> >> +	else
> >> +		return -EIO;
> >> +}
> >> +
> >> +static int rs9_regmap_i2c_read(void *context,
> >> +			       unsigned int reg, unsigned int *val)
> >> +{
> >> +	struct i2c_client *i2c = context;
> >> +	struct i2c_msg xfer[2];
> >> +	u8 txdata = reg;
> >> +	u8 rxdata[2];
> >> +	int ret;
> >> +
> >> +	xfer[0].addr = i2c->addr;
> >> +	xfer[0].flags = 0;
> >> +	xfer[0].len = 1;
> >> +	xfer[0].buf = (void *)&txdata;
> >> +
> >> +	xfer[1].addr = i2c->addr;
> >> +	xfer[1].flags = I2C_M_RD | I2C_M_RECV_LEN;
> >> +	xfer[1].len = 1;
> > 
> > I'm still in favor of removing I2C_M_RECV_LEN and setting len=2. This
> > currently works only, because there is no read access between
> > devm_regmap_init() call and regmap_write() to RS9_REG_BCP.
> > Enabling cache later on again this will corrupt the stack.
> 
> The device does send you the amount of data in first byte of I2C READ,
> so I think I2C_M_RECV_LEN is the right flag here.

But you'll need to take I2C_M_RECV_LEN into account, and store at least 
I2C_SMBUS_BLOCK_MAX + length byte in the reception buffer, see [1].
Otherwise bad things can happen, see below.

> I had a look into the above topic too before sending V3, and I think you
> can use regcache_cache_bypass(..., true) right after devm_regmap_init()
> and then again regcache_cache_bypass(..., false) close to the end of
> probe() callback to deal with the problem you're describing.
> 
> Would that work ?

Unfortunately no, if cache is enabled and cache size is set as well, you'll 
read from hardware from within devm_regmap_init call, no way to enable cache 
bypassing (yet).

--8<--
diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
index 4e627855583f..99e5fd867efc 100644
--- a/drivers/clk/clk-renesas-pcie.c
+++ b/drivers/clk/clk-renesas-pcie.c
@@ -139,8 +139,9 @@ static int rs9_regmap_i2c_read(void *context,
 static const struct regmap_config rs9_regmap_config = {
        .reg_bits = 8,
        .val_bits = 8,
-       .cache_type = REGCACHE_NONE,
+       .cache_type = REGCACHE_FLAT,
        .max_register = RS9_REG_BCP,
+       .num_reg_defaults_raw = 0x8,
        .rd_table = &rs9_readable_table,
        .wr_table = &rs9_writeable_table,
        .reg_write = rs9_regmap_i2c_write,
--8<--

This results in the following kernel panic:

clk-renesas-pcie-9series 1-0068: No cache defaults, reading back from HW
Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: 
rs9_regmap_i2c_read+0xb4/0xb4 [clk_renesas_pcie]
CPU: 1 PID: 278 Comm: systemd-udevd Not tainted 6.0.0-rc7-next-20220927+ #816 
5af64c0444274ab7984baf7c2d7f82d1e1bca080
Hardware name: TQ-Systems GmbH i.MX8MQ TQMa8MQ on MBa8Mx (DT)
Call trace:
 dump_backtrace+0xd4/0x130
 show_stack+0x14/0x40
 dump_stack_lvl+0x64/0x7c
 dump_stack+0x14/0x2c
 panic+0x19c/0x364
 __stack_chk_fail+0x24/0x30
 rs9_get_common_config+0x0/0x19c [clk_renesas_pcie 
0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
 _regmap_read+0x74/0x160
 regmap_read+0x48/0x70
 regcache_hw_init+0x184/0x2d0
 regcache_init+0x1d4/0x2c0
 __regmap_init+0x7dc/0xf30
 __devm_regmap_init+0x74/0xc0
 rs9_probe+0x110/0x298 [clk_renesas_pcie 
0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
 i2c_device_probe+0x100/0x340
 call_driver_probe+0x28/0x140
 really_probe+0xc0/0x334
 __driver_probe_device+0x84/0x144
 driver_probe_device+0x38/0x150
 __driver_attach+0xac/0x244
 bus_for_each_dev+0x6c/0xc0
 driver_attach+0x20/0x30
 bus_add_driver+0x174/0x244
 driver_register+0x74/0x120
 i2c_register_driver+0x50/0xf0
 rs9_driver_init+0x20/0x1000 [clk_renesas_pcie 
0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
 do_one_initcall+0x58/0x200
 do_init_module+0x40/0x1d4
 load_module+0x634/0x6e4
 __do_sys_finit_module+0xc0/0x140
 __arm64_sys_finit_module+0x1c/0x24
 invoke_syscall+0x6c/0xf0
 el0_svc_common.constprop.0+0xc0/0xe0
 do_el0_svc+0x24/0x30
 el0_svc+0x1c/0x50
 el0t_64_sync_handler+0xb0/0xb4
 el0t_64_sync+0x148/0x14c
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x00000,00800084,0000420b
Memory Limit: none
---[ end Kernel panic - not syncing: stack-protector: Kernel stack is 
corrupted in: rs9_regmap_i2c_read+0xb4/0xb4 [clk_renesas_pcie] ]---

IMHO, I2C_M_RECV_LEN makes only sense for regmap's (raw) 'read' callback which 
supports reading multiple registers.
But for a single register read, there is no need to take length into account, 
just read 2 bytes (length + data).

Best regards,
Alexander

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
include/uapi/linux/i2c.h#n45

Re: [PATCH v3] clk: rs9: Fix I2C accessors

[Marek Vasut]

On 9/29/22 09:17, Alexander Stein wrote:
> Hi Marek,

Hi,

>>> I'm still in favor of removing I2C_M_RECV_LEN and setting len=2. This
>>> currently works only, because there is no read access between
>>> devm_regmap_init() call and regmap_write() to RS9_REG_BCP.
>>> Enabling cache later on again this will corrupt the stack.
>>
>> The device does send you the amount of data in first byte of I2C READ,
>> so I think I2C_M_RECV_LEN is the right flag here.
> 
> But you'll need to take I2C_M_RECV_LEN into account, and store at least
> I2C_SMBUS_BLOCK_MAX + length byte in the reception buffer, see [1].
> Otherwise bad things can happen, see below.

Oh, I missed that, fixed in V4.

>> I had a look into the above topic too before sending V3, and I think you
>> can use regcache_cache_bypass(..., true) right after devm_regmap_init()
>> and then again regcache_cache_bypass(..., false) close to the end of
>> probe() callback to deal with the problem you're describing.
>>
>> Would that work ?
> 
> Unfortunately no, if cache is enabled and cache size is set as well, you'll
> read from hardware from within devm_regmap_init call, no way to enable cache
> bypassing (yet).

I can imagine that could be fixed by a small patch to regmap though, 
maybe just add another bool flag into struct regmap_config .

> --8<--
> diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
> index 4e627855583f..99e5fd867efc 100644
> --- a/drivers/clk/clk-renesas-pcie.c
> +++ b/drivers/clk/clk-renesas-pcie.c
> @@ -139,8 +139,9 @@ static int rs9_regmap_i2c_read(void *context,
>   static const struct regmap_config rs9_regmap_config = {
>          .reg_bits = 8,
>          .val_bits = 8,
> -       .cache_type = REGCACHE_NONE,
> +       .cache_type = REGCACHE_FLAT,
>          .max_register = RS9_REG_BCP,
> +       .num_reg_defaults_raw = 0x8,
>          .rd_table = &rs9_readable_table,
>          .wr_table = &rs9_writeable_table,
>          .reg_write = rs9_regmap_i2c_write,
> --8<--
> 
> This results in the following kernel panic:
> 
> clk-renesas-pcie-9series 1-0068: No cache defaults, reading back from HW
> Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in:
> rs9_regmap_i2c_read+0xb4/0xb4 [clk_renesas_pcie]
> CPU: 1 PID: 278 Comm: systemd-udevd Not tainted 6.0.0-rc7-next-20220927+ #816
> 5af64c0444274ab7984baf7c2d7f82d1e1bca080
> Hardware name: TQ-Systems GmbH i.MX8MQ TQMa8MQ on MBa8Mx (DT)
> Call trace:
>   dump_backtrace+0xd4/0x130
>   show_stack+0x14/0x40
>   dump_stack_lvl+0x64/0x7c
>   dump_stack+0x14/0x2c
>   panic+0x19c/0x364
>   __stack_chk_fail+0x24/0x30
>   rs9_get_common_config+0x0/0x19c [clk_renesas_pcie
> 0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
>   _regmap_read+0x74/0x160
>   regmap_read+0x48/0x70
>   regcache_hw_init+0x184/0x2d0
>   regcache_init+0x1d4/0x2c0
>   __regmap_init+0x7dc/0xf30
>   __devm_regmap_init+0x74/0xc0
>   rs9_probe+0x110/0x298 [clk_renesas_pcie
> 0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
>   i2c_device_probe+0x100/0x340
>   call_driver_probe+0x28/0x140
>   really_probe+0xc0/0x334
>   __driver_probe_device+0x84/0x144
>   driver_probe_device+0x38/0x150
>   __driver_attach+0xac/0x244
>   bus_for_each_dev+0x6c/0xc0
>   driver_attach+0x20/0x30
>   bus_add_driver+0x174/0x244
>   driver_register+0x74/0x120
>   i2c_register_driver+0x50/0xf0
>   rs9_driver_init+0x20/0x1000 [clk_renesas_pcie
> 0f42365202f6346c1b8a6cb8867a1ff0d4899aee]
>   do_one_initcall+0x58/0x200
>   do_init_module+0x40/0x1d4
>   load_module+0x634/0x6e4
>   __do_sys_finit_module+0xc0/0x140
>   __arm64_sys_finit_module+0x1c/0x24
>   invoke_syscall+0x6c/0xf0
>   el0_svc_common.constprop.0+0xc0/0xe0
>   do_el0_svc+0x24/0x30
>   el0_svc+0x1c/0x50
>   el0t_64_sync_handler+0xb0/0xb4
>   el0t_64_sync+0x148/0x14c
> SMP: stopping secondary CPUs
> Kernel Offset: disabled
> CPU features: 0x00000,00800084,0000420b
> Memory Limit: none
> ---[ end Kernel panic - not syncing: stack-protector: Kernel stack is
> corrupted in: rs9_regmap_i2c_read+0xb4/0xb4 [clk_renesas_pcie] ]---
> 
> IMHO, I2C_M_RECV_LEN makes only sense for regmap's (raw) 'read' callback which
> supports reading multiple registers.
> But for a single register read, there is no need to take length into account,
> just read 2 bytes (length + data).

All right, let's do that.