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@disroot.org>
---
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;
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[] = {
--
2.53.0
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@disroot.org>
> ---
> 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;
>
> 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[] = {
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@disroot.org>
> ---
> 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.
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[] = {
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@disroot.org>
>> ---
>> 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.
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?
> 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[] = {
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@disroot.org>
>>> ---
>>> 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[] = {
© 2016 - 2026 Red Hat, Inc.