Re: [PATCH 4/8] i2c: rtl9300: introduce a property for 8 bit width reg address

[Chris Packham]

On 17/03/2026 05:28, Rustam Adilov wrote:
> Hello,
> On 2026-03-15 21:31, Chris Packham wrote:
>> Hi Rustam,
>>
>> (sorry I might have prematurely hit send on an earlier reply)
>>
>> On 14/03/2026 21:26, Rustam Adilov wrote:
>>> In RTL9607C i2c controller, in order to indicate that the width of
>>> memory address is 8 bits, 0 is written to MEM_ADDR_WIDTH field as
>>> opposed to 1 for RTL9300 and RTL9310.
>>>
>>> Introduce a new property to a driver data to indicate what value
>>> need to written to MEM_ADDR_WIDTH field for this case.
>>>
>>> Signed-off-by: Rustam Adilov <adilov@xxxxxxxxxxx>
>>> ---
>>>    drivers/i2c/busses/i2c-rtl9300.c | 8 +++++++-
>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
>>> index 2525b57a9d03..86a82f2c3ce0 100644
>>> --- a/drivers/i2c/busses/i2c-rtl9300.c
>>> +++ b/drivers/i2c/busses/i2c-rtl9300.c
>>> @@ -60,6 +60,7 @@ struct rtl9300_i2c_drv_data {
>>>    	u32 wd_reg;
>>>    	u8 max_nchan;
>>>    	u8 max_data_len;
>>> +	u8 reg_addr_8bit_len;
>>>    };
>>>    
>>>    #define RTL9300_I2C_MUX_NCHAN	8
>>> @@ -105,6 +106,7 @@ struct rtl9300_i2c_xfer {
>>>    #define RTL9300_I2C_MST_DATA_WORD2			0x10
>>>    #define RTL9300_I2C_MST_DATA_WORD3			0x14
>>>    #define RTL9300_I2C_MST_GLB_CTRL			0x384
>>> +#define RTL9300_REG_ADDR_8BIT_LEN			1
>>>    
>>>    #define RTL9310_I2C_MST_IF_CTRL				0x1004
>>>    #define RTL9310_I2C_MST_IF_SEL				0x1008
>>> @@ -299,6 +301,7 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
>>>    				  union i2c_smbus_data *data)
>>>    {
>>>    	struct rtl9300_i2c_chan *chan = i2c_get_adapdata(adap);
>>> +	const struct rtl9300_i2c_drv_data *drv_data;
>>>    	struct rtl9300_i2c *i2c = chan->i2c;
>>>    	struct rtl9300_i2c_xfer xfer = {0};
>>>    	int ret;
>>> @@ -308,6 +311,7 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
>>>    
>>>    	guard(rtl9300_i2c)(i2c);
>>>    
>>> +	drv_data = device_get_match_data(i2c->dev);
>>>    	ret = rtl9300_i2c_config_chan(i2c, chan);
>>>    	if (ret)
>>>    		return ret;
>>> @@ -315,7 +319,7 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
>>>    	xfer.dev_addr = addr & 0x7f;
>>>    	xfer.write = (read_write == I2C_SMBUS_WRITE);
>>>    	xfer.reg_addr = command;
>>> -	xfer.reg_addr_len = 1;
>>> +	xfer.reg_addr_len = drv_data->reg_addr_8bit_len;
>> For this one I wonder if we could come up with something that involves a
>> subtraction for the rtl9607? len = 1 just makes sense to me so maybe
>> RTL_9300_ADDR_LEN(1) which expands to 1 - 0 on the rtl9300 and 1 - 1 on
>> the rtl9607 would be easier to follow.
> I am not entirely sure i understand this approach. Is RTL_9300_ADDR_LEN(1)
> gonna be a "#define"? Would like to know before i proceed with possible
> changes for v2.

I was thinking

#define RTL_9300_ADDR_LEN(n)    ((n) - drv_data->subtract_len)

Or something like that (I couldn't think of a better macro name).

> And wouldn't something like this work just as well:
>
> xfer.reg_addr_len = 1 - drv_data->subtract_len;
>
> But then, i honestly think "drv_data->reg_addr_8bit_len" would be a bit less
> cluttery?
Probably a question of taste. I'm fine with keeping your original 
reg_addr_8bit_len it was more a case of me thinking "how is the length 
of an 8-bit command anything other than 1" but I know the answer is 
"because realtek".
>> Not a deal breaker just thought I'd mention it.
>>
>>>    
>>>    	switch (size) {
>>>    	case I2C_SMBUS_BYTE:
>>> @@ -501,6 +505,7 @@ static const struct rtl9300_i2c_drv_data rtl9300_i2c_drv_data = {
>>>    	.wd_reg = RTL9300_I2C_MST_DATA_WORD0,
>>>    	.max_nchan = RTL9300_I2C_MUX_NCHAN,
>>>    	.max_data_len = RTL9300_I2C_MAX_DATA_LEN,
>>> +	.reg_addr_8bit_len = RTL9300_REG_ADDR_8BIT_LEN,
>>>    };
>>>    
>>>    static const struct rtl9300_i2c_drv_data rtl9310_i2c_drv_data = {
>>> @@ -524,6 +529,7 @@ static const struct rtl9300_i2c_drv_data rtl9310_i2c_drv_data = {
>>>    	.wd_reg = RTL9310_I2C_MST_DATA_CTRL,
>>>    	.max_nchan = RTL9310_I2C_MUX_NCHAN,
>>>    	.max_data_len = RTL9300_I2C_MAX_DATA_LEN,
>>> +	.reg_addr_8bit_len = RTL9300_REG_ADDR_8BIT_LEN,
>>>    };
>>>    
>>>    static const struct of_device_id i2c_rtl9300_dt_ids[] = {