Re: [PATCH v2 4/4] ARM: dts: aspeed: bletchley: Add second source ISL1208 RTC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Adding the ISL1208 as an active alternative RTC introduces a system-level regression on boards populated with the ISL1208. The unpopulated primary PCF85263 will register a phantom /dev/rtc0 device due to a missing I2C failure check in its driver, causing the ISL1208 to be registered as /dev/rtc1 and breaking userspace time synchronization.
--

--- Patch [4]: [PATCH v2 4/4] ARM: dts: aspeed: bletchley: Add second source ISL1208 RTC ---
commit 02d65705396424e85939c4fb70bef75dc330e54c
Author: Cosmo Chou <chou.cosmo@gmail.com>

ARM: dts: aspeed: bletchley: Add second source ISL1208 RTC

This commit adds an alternative ISL1208 RTC node at address 0x6f to support
boards populated with the ISL1208 instead of the primary PCF85263.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> index 3da1cf9bbb990..01dbbda3e4b6a 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> @@ -1269,6 +1269,11 @@ eeprom@56 {
>  		compatible = "atmel,24c64";
>  		reg = <0x56>;
>  	};
> +
> +	rtc@6f {
> +		compatible = "isil,isl1208";
> +		reg = <0x6f>;
> +	};
[Severity: High]
Will adding this alternative RTC node break userspace time synchronization on
boards populated with the ISL1208?

Looking at the primary RTC's driver probe function:

drivers/rtc/rtc-pcf85363.c:pcf85363_probe() {
    ...
	err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
	if (err < 0)
		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
			 err);

	pcf85363->rtc->ops = &rtc_ops;
    ...
}

The PCF85263 driver ignores I2C communication errors during probe. If a board
is populated with the ISL1208 instead of the PCF85263, the absent PCF85263
will still successfully probe and register a phantom /dev/rtc0 device.

Does this cause the ISL1208 to be registered as /dev/rtc1, resulting in
userspace services like hwclock or systemd-timesyncd targeting the
non-functional phantom device by default?

Should this alternative topology be handled via a bootloader fixup, or
should the rtc-pcf85363 driver be fixed to fail probe when the hardware
is absent before adding this node?

>  };
>  
>  &i2c7 {

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