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

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

[Cosmo Chou]

During probe, pcf85363_load_capacitance() writes the oscillator load
capacitance configuration to the device. However, if the device is
not physically present on the bus (returning -ENXIO), the driver
only emits a warning and continues to execute the probe.

This results in the successful registration of a phantom RTC device
via devm_rtc_register_device() and its associated nvmem regions.
Consequently, userspace may attempt to bind to a non-functional
/dev/rtc node.

Propagate the I2C error back to the driver core using dev_err_probe()
to properly abort the probe and prevent phantom device registration.

Fixes: fd9a6a13949a ("rtc: pcf85363: add support for the quartz-load-femtofarads property")
Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
---
 drivers/rtc/rtc-pcf85363.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8..5612330aff40 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
 
 	err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
 	if (err < 0)
-		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
-			 err);
+		return dev_err_probe(&client->dev, err,
+				     "failed to set xtal load capacitance\n");
 
 	pcf85363->rtc->ops = &rtc_ops;
 	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
-- 
2.43.0

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

[Alexandre Belloni]

On 16/07/2026 20:51:42+0800, Cosmo Chou wrote:
> During probe, pcf85363_load_capacitance() writes the oscillator load
> capacitance configuration to the device. However, if the device is
> not physically present on the bus (returning -ENXIO), the driver
> only emits a warning and continues to execute the probe.
> 
> This results in the successful registration of a phantom RTC device
> via devm_rtc_register_device() and its associated nvmem regions.
> Consequently, userspace may attempt to bind to a non-functional
> /dev/rtc node.
> 
> Propagate the I2C error back to the driver core using dev_err_probe()
> to properly abort the probe and prevent phantom device registration.
> 

No, there is no point in this, simply remove the RTC node from you
device tree.

> Fixes: fd9a6a13949a ("rtc: pcf85363: add support for the quartz-load-femtofarads property")
> Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
> ---
>  drivers/rtc/rtc-pcf85363.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 540042b9eec8..5612330aff40 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
> @@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
>  
>  	err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
>  	if (err < 0)
> -		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
> -			 err);
> +		return dev_err_probe(&client->dev, err,
> +				     "failed to set xtal load capacitance\n");
>  
>  	pcf85363->rtc->ops = &rtc_ops;
>  	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> -- 
> 2.43.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[sashiko-bot]

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

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

[Cosmo Chou]

On Thu, Jul 16, 2026 at 8:56 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 16/07/2026 20:51:42+0800, Cosmo Chou wrote:
> > During probe, pcf85363_load_capacitance() writes the oscillator load
> > capacitance configuration to the device. However, if the device is
> > not physically present on the bus (returning -ENXIO), the driver
> > only emits a warning and continues to execute the probe.
> >
> > This results in the successful registration of a phantom RTC device
> > via devm_rtc_register_device() and its associated nvmem regions.
> > Consequently, userspace may attempt to bind to a non-functional
> > /dev/rtc node.
> >
> > Propagate the I2C error back to the driver core using dev_err_probe()
> > to properly abort the probe and prevent phantom device registration.
> >
>
> No, there is no point in this, simply remove the RTC node from you
> device tree.
>

Hi Alexandre,

You are right, fixing the DT is the correct solution for missing
hardware. My commit message was misleading.

However, the current code ignores actual I2C transport errors (e.g.,
-ENXIO due to physical bus issues) from pcf85363_load_capacitance()
and continues probing.

Should I send a v2 with an updated commit message focusing solely on
properly handling these transport errors?

Thanks,
Cosmo

> > Fixes: fd9a6a13949a ("rtc: pcf85363: add support for the quartz-load-femtofarads property")
> > Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
> > ---
> >  drivers/rtc/rtc-pcf85363.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> > index 540042b9eec8..5612330aff40 100644
> > --- a/drivers/rtc/rtc-pcf85363.c
> > +++ b/drivers/rtc/rtc-pcf85363.c
> > @@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
> >
> >       err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
> >       if (err < 0)
> > -             dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
> > -                      err);
> > +             return dev_err_probe(&client->dev, err,
> > +                                  "failed to set xtal load capacitance\n");
> >
> >       pcf85363->rtc->ops = &rtc_ops;
> >       pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > --
> > 2.43.0
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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

[Alexandre Belloni]

On 16/07/2026 22:56:04+0800, Cosmo Chou wrote:
> On Thu, Jul 16, 2026 at 8:56 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 16/07/2026 20:51:42+0800, Cosmo Chou wrote:
> > > During probe, pcf85363_load_capacitance() writes the oscillator load
> > > capacitance configuration to the device. However, if the device is
> > > not physically present on the bus (returning -ENXIO), the driver
> > > only emits a warning and continues to execute the probe.
> > >
> > > This results in the successful registration of a phantom RTC device
> > > via devm_rtc_register_device() and its associated nvmem regions.
> > > Consequently, userspace may attempt to bind to a non-functional
> > > /dev/rtc node.
> > >
> > > Propagate the I2C error back to the driver core using dev_err_probe()
> > > to properly abort the probe and prevent phantom device registration.
> > >
> >
> > No, there is no point in this, simply remove the RTC node from you
> > device tree.
> >
> 
> Hi Alexandre,
> 
> You are right, fixing the DT is the correct solution for missing
> hardware. My commit message was misleading.
> 
> However, the current code ignores actual I2C transport errors (e.g.,
> -ENXIO due to physical bus issues) from pcf85363_load_capacitance()
> and continues probing.
> 
> Should I send a v2 with an updated commit message focusing solely on
> properly handling these transport errors?

Then you should add error checking to all the regmap calls in probe()

> 
> Thanks,
> Cosmo
> 
> > > Fixes: fd9a6a13949a ("rtc: pcf85363: add support for the quartz-load-femtofarads property")
> > > Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
> > > ---
> > >  drivers/rtc/rtc-pcf85363.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> > > index 540042b9eec8..5612330aff40 100644
> > > --- a/drivers/rtc/rtc-pcf85363.c
> > > +++ b/drivers/rtc/rtc-pcf85363.c
> > > @@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
> > >
> > >       err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
> > >       if (err < 0)
> > > -             dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
> > > -                      err);
> > > +             return dev_err_probe(&client->dev, err,
> > > +                                  "failed to set xtal load capacitance\n");
> > >
> > >       pcf85363->rtc->ops = &rtc_ops;
> > >       pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > > --
> > > 2.43.0
> > >
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com