Re: [PATCH] rtc: pcf8563: add SMBus support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: pcf8563: time tearing on SMBus adapters lacking block read
--

commit 22b80c04bcdbffd7bde30f876c7c59ca8c532372
Author: Michael Walle <mwalle@kernel.org>

rtc: pcf8563: add SMBus support

This commit removes the restriction that the driver can only be used with
I2C controllers. It delegates bus compatibility checks to regmap, aiming
to allow the driver to work on SMBus controllers that support block
accesses.

> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
> index 7eebdb0580674..eee9bc752b52a 100644
> --- a/drivers/rtc/rtc-pcf8563.c
> +++ b/drivers/rtc/rtc-pcf8563.c
> @@ -479,9 +479,6 @@ static int pcf8563_probe(struct i2c_client *client)
>  
>  	dev_dbg(&client->dev, "%s\n", __func__);
>  
> -	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> -		return -ENODEV;
> -
[Severity: High]
Could removing this check lead to time tearing on restricted SMBus adapters?

While the commit message mentions SMBus controllers supporting block
accesses, removing the check entirely delegates bus validation to regmap. If
the adapter only supports I2C_FUNC_SMBUS_BYTE_DATA and lacks block transfer
capabilities, regmap silently falls back to regmap_smbus_byte:

drivers/base/regmap/regmap-i2c.c:regmap_get_i2c_bus() {
    ...
    else if (config->val_bits == 8 && config->reg_bits == 8 &&
             i2c_check_functionality(i2c->adapter,
                                     I2C_FUNC_SMBUS_BYTE_DATA))
        bus = &regmap_smbus_byte;
    ...
}

Because of this fallback, when pcf8563_rtc_read_time() reads the time:

drivers/rtc/rtc-pcf8563.c:pcf8563_rtc_read_time() {
    ...
    err = regmap_bulk_read(pcf8563->regmap, PCF8563_REG_ST1, buf,
                           sizeof(buf));
    ...
}

The bulk read executes as a loop of separate single-byte SMBus reads instead
of an atomic transaction. Since each SMBus byte transaction issues its own
STOP condition, the RTC's internal register latch is released between reads.

Does this allow the hardware time to roll over mid-read and result in
corrupt time data?

>  	pcf8563 = devm_kzalloc(&client->dev, sizeof(struct pcf8563),
>  				GFP_KERNEL);
>  	if (!pcf8563)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930113126.720555-1-mwalle@kernel.org?part=1