Re: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `nvmem_cfg` array in `pcf85363_probe()` is declared `static`, causing all instances of the device to share the same configuration. Because its `.id` field is not explicitly initialized, it defaults to 0 rather than `NVMEM_DEVID_AUTO`. The NVMEM core will assign ID 0 for the first instance (e.g., "pcf85x63-0"), but any subsequent PCF85363 device on the same system will cause a sysfs duplicate name collision and fail to register. Furthermore, the driver mutates this shared static structure during probe by assigning `nvmem_cfg[i].priv = pcf85363`, and the `devm_rtc_nvmem_register()` helper further mutates it by assigning `nvmem_config->dev = dev`. If two PCF85363 devices are probed concurrently, a data race occurs where one thread may overwrite the `priv` or `dev` pointers of the other before registration completes, causing NVMEM operations to access the wrong I2C device.
--

--- Patch [1]: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware ---
commit ad427522054996520ae92d70a7e73f66bf7ac5a8
Author: Cosmo Chou <chou.cosmo@gmail.com>

rtc: pcf85363: Fix phantom device registration on missing hardware

This patch prevents phantom RTC device registration if the PCF85363 device
is missing. It propagates the error back to the driver core using
dev_err_probe() to properly abort the probe instead of continuing.

> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
> @@ -382,21 +382,21 @@ static int pcf85363_probe(struct i2c_client *client)
>  {
>  	struct pcf85363 *pcf85363;
>  	const struct pcf85x63_config *config = &pcf_85363_config;
>  	const void *data = of_device_get_match_data(&client->dev);
>  	static struct nvmem_config nvmem_cfg[] = {

[Severity: High]
This is a pre-existing issue, but since nvmem_cfg is declared static, all
instances of the device share this configuration.

Because its .id field is not explicitly initialized, it defaults to 0
rather than NVMEM_DEVID_AUTO. Doesn't this cause sysfs duplicate name
collisions if multiple PCF85363 devices are present on the system?

Furthermore, doesn't this shared static array cause a data race during
concurrent probes? The driver mutates this array later in pcf85363_probe():

drivers/rtc/rtc-pcf85363.c:pcf85363_probe() {
    ...
    for (i = 0; i < config->num_nvram; i++) {
        nvmem_cfg[i].priv = pcf85363;
        devm_rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
    ...
}

And devm_rtc_nvmem_register() further mutates it by assigning
nvmem_config->dev = dev.

If two PCF85363 devices are probed concurrently, could one thread overwrite
the priv or dev pointers of the other before registration completes, causing
NVMEM operations to access the wrong I2C device? Should this configuration
be dynamically allocated or copied per-device instead of being static?

>  		{
>  			.name = "pcf85x63-",
>  			.word_size = 1,
>  			.stride = 1,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716125142.1801599-1-chou.cosmo@gmail.com?part=1