* [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware
@ 2026-07-16 12:51 Cosmo Chou
2026-07-16 12:56 ` Alexandre Belloni
2026-07-16 13:00 ` sashiko-bot
0 siblings, 2 replies; 5+ messages in thread
From: Cosmo Chou @ 2026-07-16 12:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: linux-rtc, linux-kernel, cosmo.chou, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware 2026-07-16 12:51 [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware Cosmo Chou @ 2026-07-16 12:56 ` Alexandre Belloni 2026-07-16 14:56 ` Cosmo Chou 2026-07-16 13:00 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Alexandre Belloni @ 2026-07-16 12:56 UTC (permalink / raw) To: Cosmo Chou; +Cc: linux-rtc, linux-kernel, cosmo.chou 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware 2026-07-16 12:56 ` Alexandre Belloni @ 2026-07-16 14:56 ` Cosmo Chou 2026-07-16 15:22 ` Alexandre Belloni 0 siblings, 1 reply; 5+ messages in thread From: Cosmo Chou @ 2026-07-16 14:56 UTC (permalink / raw) To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware 2026-07-16 14:56 ` Cosmo Chou @ 2026-07-16 15:22 ` Alexandre Belloni 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Belloni @ 2026-07-16 15:22 UTC (permalink / raw) To: Cosmo Chou; +Cc: linux-rtc, linux-kernel, cosmo.chou 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware 2026-07-16 12:51 [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware Cosmo Chou 2026-07-16 12:56 ` Alexandre Belloni @ 2026-07-16 13:00 ` sashiko-bot 1 sibling, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-07-16 13:00 UTC (permalink / raw) To: Cosmo Chou; +Cc: Alexandre Belloni, linux-rtc 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-16 15:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-16 12:51 [PATCH] rtc: pcf85363: Fix phantom device registration on missing hardware Cosmo Chou 2026-07-16 12:56 ` Alexandre Belloni 2026-07-16 14:56 ` Cosmo Chou 2026-07-16 15:22 ` Alexandre Belloni 2026-07-16 13:00 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.