All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Cosmo Chou" <chou.cosmo@gmail.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] ARM: dts: aspeed: bletchley: Add second source ISL1208 RTC
Date: Tue, 14 Jul 2026 12:18:44 +0000	[thread overview]
Message-ID: <20260714121845.EF8A11F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260714120816.1504058-2-chou.cosmo@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new `rtc@6f` node is deliberately inserted out of numerical order (before `rtc@51`) to manipulate probe order. This acts as a brittle workaround for a driver bug in `rtc-pcf85363.c` (which unconditionally registers a phantom RTC even when the hardware is absent).
--

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

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

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

> 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 d1a04b63df9e0..a966be69ff27d 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> @@ -865,6 +865,11 @@ eeprom@56 {
>  		reg = <0x56>;
>  	};
>  
> +	rtc@6f {
> +		compatible = "isil,isl1208";
> +		reg = <0x6f>;
> +	};
> +
>  	rtc@51 {

[Severity: Medium]
Are these device tree nodes intentionally placed out of numerical order?

Typically, device tree nodes are sorted by their unit address, meaning rtc@51
would normally appear before rtc@6f.

If this out-of-order placement is intended to force rtc@6f to probe first
as a workaround for a driver issue in rtc-pcf85363.c (which unconditionally
registers a phantom RTC even when the hardware is absent), could this create
a brittle dependency?

If a developer later sorts these nodes by unit address during a routine
cleanup, the probe order would change. This could allow the phantom PCF85263
device to probe first and claim /dev/rtc0, which might silently break system
timekeeping on ISL1208 boards.

>  		/* in-chip rtc disabled, use external rtc (battery-backed) */
>  		compatible = "nxp,pcf85263";

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

  reply	other threads:[~2026-07-14 12:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 12:08 [PATCH 0/2] ARM: dts: aspeed: bletchley: Add second source devices Cosmo Chou
2026-07-14 12:08 ` [PATCH 1/2] ARM: dts: aspeed: bletchley: Add second source ISL1208 RTC Cosmo Chou
2026-07-14 12:18   ` sashiko-bot [this message]
2026-07-14 12:08 ` [PATCH 2/2] ARM: dts: aspeed: bletchley: Add second source PCA9532 LED controllers Cosmo Chou
2026-07-14 12:28   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260714121845.EF8A11F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=chou.cosmo@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.