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@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>