[PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

MediaTek mt65xx family SoCs have no auto restart, however, they still
support the WRRD mode in the hardware. Because auto_restart is set to 0,
the WRRD mode will be never enabled, leading to read errors.

Fix this by removing auto_restart check from the WRRD enable path.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
This is a preparation for the mt6572/6595 upstreaming.

mt65xx family SoCs don't have auto restart, but vendor kernels keep using
WRRD mode. Lack of the WRRD mode makes i2c reads impossible from both
userspace and kernel drivers.

Without patch (mt6595, da9210 buck at 0x68):
~ # i2cget -y 1 0x68 0x00
Error: Read failed
~ # i2cget -y 1 0x68 0x01
Error: Read failed

With patch:
~ # i2cget -y 1 0x68 0x00
0x80
~ # i2cget -y 1 0x68 0x01
0x00

Same behavior observed on mt6572 devices.

This change doesn't affect SoCs with auto restart option.
---
 drivers/i2c/busses/i2c-mt65xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 126040ca05f1..307925fb78e3 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -1258,7 +1258,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
 	i2c->auto_restart = i2c->dev_comp->auto_restart;
 
 	/* checking if we can skip restart and optimize using WRRD mode */
-	if (i2c->auto_restart && num == 2) {
+	if (num == 2) {
 		if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
 		    msgs[0].addr == msgs[1].addr) {
 			i2c->auto_restart = 0;

---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260623-6572-6595-i2c-6ec9c4e6a6a6

Best regards,
--  
Roman Vivchar <rva333@protonmail.com>

Re: [PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[Roman Vivchar]

Hi everyone,

Just sending a gentle ping on this patch. It fixes broken I2C
functionality for mt65xx platforms.

Please let me know if you have any feedback.

Best regards,
Roman

Re: [PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[AngeloGioacchino Del Regno]

On 6/23/26 23:40, Roman Vivchar via B4 Relay wrote:
> From: Roman Vivchar <rva333@protonmail.com>
> 
> MediaTek mt65xx family SoCs have no auto restart, however, they still
> support the WRRD mode in the hardware. Because auto_restart is set to 0,
> the WRRD mode will be never enabled, leading to read errors.
> 
> Fix this by removing auto_restart check from the WRRD enable path.
> 

Are you sure that MT6577 and MT6589 do support WRRD?

If you're not 100% sure, or you want to be careful (which I would, in this case),
please add a `unsigned char wrrd_supported:1` to struct mtk_i2c_compatible, set
it to `1` on *all* of the SoCs that support auto_restart (as that's effectively
a different feature - it just happens that all the ones supporting this also do
support that, up until today), and add a new one for your MT6572, declaring no
auto_restart but wrrd_supported.

That would make me sure that you're not breaking anything, and would guarantee
you a R-b.

Cheers,
Angelo

> Signed-off-by: Roman Vivchar <rva333@protonmail.com>
> ---
> This is a preparation for the mt6572/6595 upstreaming.
> 
> mt65xx family SoCs don't have auto restart, but vendor kernels keep using
> WRRD mode. Lack of the WRRD mode makes i2c reads impossible from both
> userspace and kernel drivers.
> 
> Without patch (mt6595, da9210 buck at 0x68):
> ~ # i2cget -y 1 0x68 0x00
> Error: Read failed
> ~ # i2cget -y 1 0x68 0x01
> Error: Read failed
> 
> With patch:
> ~ # i2cget -y 1 0x68 0x00
> 0x80
> ~ # i2cget -y 1 0x68 0x01
> 0x00
> 
> Same behavior observed on mt6572 devices.
> 
> This change doesn't affect SoCs with auto restart option.
> ---
>   drivers/i2c/busses/i2c-mt65xx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index 126040ca05f1..307925fb78e3 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -1258,7 +1258,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
>   	i2c->auto_restart = i2c->dev_comp->auto_restart;
>   
>   	/* checking if we can skip restart and optimize using WRRD mode */
> -	if (i2c->auto_restart && num == 2) {
> +	if (num == 2) {
>   		if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
>   		    msgs[0].addr == msgs[1].addr) {
>   			i2c->auto_restart = 0;
> 
> ---
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
> change-id: 20260623-6572-6595-i2c-6ec9c4e6a6a6
> 
> Best regards,
> --
> Roman Vivchar <rva333@protonmail.com>
> 
> 

Re: [PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[Roman Vivchar]

Hi AngeloGioacchino,

On Thursday, July 9th, 2026 at 2:46 PM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:

> On 6/23/26 23:40, Roman Vivchar via B4 Relay wrote:
> > From: Roman Vivchar <rva333@protonmail.com>
> >
> > MediaTek mt65xx family SoCs have no auto restart, however, they still
> > support the WRRD mode in the hardware. Because auto_restart is set to 0,
> > the WRRD mode will be never enabled, leading to read errors.
> >
> > Fix this by removing auto_restart check from the WRRD enable path.
> >
> 
> Are you sure that MT6577 and MT6589 do support WRRD?

Yes, I have checked their downstream kernels[1][2] and they both mention
WRRD.

We also tested mt6589 (privately) and the i2c was working. To be fair,
it had some other fixes, but at worst this patch is no-op.

mt6577 is a tough one... I don't think there's any recent kernel with
it. I plan to make it work with upstream, but that's a TODO. Either way,
vendor code has WRRD support.

> If you're not 100% sure, or you want to be careful (which I would, in this case),
> please add a `unsigned char wrrd_supported:1` to struct mtk_i2c_compatible, set
> it to `1` on *all* of the SoCs that support auto_restart (as that's effectively
> a different feature - it just happens that all the ones supporting this also do
> support that, up until today), and add a new one for your MT6572, declaring no
> auto_restart but wrrd_supported.
> 
> That would make me sure that you're not breaking anything, and would guarantee
> you a R-b.
>

As far as I know, there's no SoC without both auto restart and WRRD.
Adding wrrd_supported that always equals to true sounds redundant to
me.

Let me know if you still prefer having separate flag for the WRRD.

> Cheers,
> Angelo
> 

[1]: https://github.com/GoldRenard/lenovo_A390_mt6577_uboot_kernel/blob/38ea59263984e26c0dd8e5891b22a35f61e60ed5/mediatek/platform/mt6577/kernel/drivers/i2c/i2c.c#L963-L975
[2]: https://github.com/bq/aquaris-5/blob/bc1d0d0e35c969670e2d869001be944ad0c9c5e5/mediatek/platform/mt6589/kernel/drivers/i2c/i2c.c#L728

Best regards,
Roman

Re: [PATCH] i2c: mediatek: fix WRRD for SoCs without auto_restart option

[AngeloGioacchino Del Regno]

On 7/9/26 14:12, Roman Vivchar wrote:
> Hi AngeloGioacchino,
> 
> On Thursday, July 9th, 2026 at 2:46 PM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:
> 
>> On 6/23/26 23:40, Roman Vivchar via B4 Relay wrote:
>>> From: Roman Vivchar <rva333@protonmail.com>
>>>
>>> MediaTek mt65xx family SoCs have no auto restart, however, they still
>>> support the WRRD mode in the hardware. Because auto_restart is set to 0,
>>> the WRRD mode will be never enabled, leading to read errors.
>>>
>>> Fix this by removing auto_restart check from the WRRD enable path.
>>>
>>
>> Are you sure that MT6577 and MT6589 do support WRRD?
> 
> Yes, I have checked their downstream kernels[1][2] and they both mention
> WRRD.
> 
> We also tested mt6589 (privately) and the i2c was working. To be fair,
> it had some other fixes, but at worst this patch is no-op.
> 
> mt6577 is a tough one... I don't think there's any recent kernel with
> it. I plan to make it work with upstream, but that's a TODO. Either way,
> vendor code has WRRD support.
> 
>> If you're not 100% sure, or you want to be careful (which I would, in this case),
>> please add a `unsigned char wrrd_supported:1` to struct mtk_i2c_compatible, set
>> it to `1` on *all* of the SoCs that support auto_restart (as that's effectively
>> a different feature - it just happens that all the ones supporting this also do
>> support that, up until today), and add a new one for your MT6572, declaring no
>> auto_restart but wrrd_supported.
>>
>> That would make me sure that you're not breaking anything, and would guarantee
>> you a R-b.
>>
> 
> As far as I know, there's no SoC without both auto restart and WRRD.
> Adding wrrd_supported that always equals to true sounds redundant to
> me.

I have just verified on datasheets.
You're right, both MT6577 and MT6589 do support WRRD, hence your change is good.

With this commit, you're fixing a fix... Leilk from MediaTek was indeed wrong, as
in the commit description he says "the old IC does not support I2C_MASTER_WRRD",
and that's *very* wrong.

I can see though what he did, as he inadvertently also fixed the WRRD algo because
the fix was to increment msgs by 2 instead of 1 when op==WRRD.


Please, resend your commit as a v2, and add:

Fixes: b49218365280 ("i2c: mediatek: fix potential incorrect use of I2C_MASTER_WRRD")

and also
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers!

> 
> [1]: https://github.com/GoldRenard/lenovo_A390_mt6577_uboot_kernel/blob/38ea59263984e26c0dd8e5891b22a35f61e60ed5/mediatek/platform/mt6577/kernel/drivers/i2c/i2c.c#L963-L975
> [2]: https://github.com/bq/aquaris-5/blob/bc1d0d0e35c969670e2d869001be944ad0c9c5e5/mediatek/platform/mt6589/kernel/drivers/i2c/i2c.c#L728
> 
> Best regards,
> Roman