mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Abdurrahman Hussain" <abdurrahman@nexthop.ai>
To: "Rob Herring" <robh@kernel.org>,
	"Abdurrahman Hussain" <abdurrahman@nexthop.ai>
Cc: "Saravana Kannan" <saravanak@kernel.org>,
	"Frank Rowand" <frowand.list@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Grant Likely" <grant.likely@secretlab.ca>,
	"Grant Likely" <grant.likely@linaro.org>,
	"Pantelis Antoniou" <pantelis.antoniou@konsulko.com>,
	"David Daney" <david.daney@cavium.com>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<stable@vger.kernel.org>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	"Sashiko AI" <sashiko-bot@kernel.org>
Subject: Re: [PATCH v7 00/10] of: teach overlay code to keep /aliases in sync
Date: Thu, 17 Sep 2026 19:21:23 -0700	[thread overview]
Message-ID: <DLI30THJZGQ0.2NVBET2OS94K1@nexthop.ai> (raw)
In-Reply-To: <20260917203938.GA3480871-robh@kernel.org>

On Thu Sep 17, 2026 at 1:39 PM PDT, Rob Herring wrote:
> There was another posting in 2024 of Geert's patches and AFAICT my
> comment there[1] still applies. To repeat, what happens if the alias
> number already got used because the subsystems can pick any of the
> numbers without an alias. The only way I see to solve that is make
> possible alias numbers and dynamic numbers non-overlapping or only allow
> alias names that are not present in the base DT to be used/honored in
> overlays.

Thanks, I missed that thread. I went through what the users of
of_alias_get_id() actually do when the index is taken, to see how bad
the collision is in practice:

  i2c: i2c_add_adapter() -> i2c_allocate_adapter_id() does
       idr_alloc(nr, nr + 1). Taken index -> -EBUSY plus
       pr_err("adapter '%s': failed to allocate id"), adapter is not
       registered.

  spi: spi_register_controller() -> spi_controller_id_alloc(bus_num,
       bus_num + 1). Taken index -> WARN() and -EBUSY, controller is
       not registered.

  serial: serial_port.c sets port->line from the alias;
       serial_core_add_one_port() returns -EINVAL if that line already
       has a uart_port.

So a collision is a loud probe failure, never two devices on one
number or a device silently landing somewhere else. What changes with
this series is the failure mode for an overlay that asks for an index
which a dynamic allocation already took: today the alias is ignored and
the device quietly gets a dynamic number (which is exactly the bug the
series is fixing, since the pinned number was the point of the alias);
with the series the probe fails and says why. I think that is the right
contract: an alias is a request for a specific number, and refusing it
is more useful than pretending it was honored. It matches what already
happens in the base DT if two aliases name the same index, or if a
driver calls i2c_add_numbered_adapter() with a taken number.

On the two alternatives you list:

Non-overlapping ranges: the subsystems already try. i2c computes
__i2c_first_dynamic_bus_num from of_alias_get_highest_id("i2c") at
init and spi re-reads the highest id at each dynamic registration, so
dynamic numbers start above every boot-time alias. What they cannot do
is reserve numbers for an overlay that has not been loaded yet. The
OF core could refuse an overlay alias whose index is at or below the
subsystem's dynamic floor, but it does not know that floor; it would
need a per-stem hook from every subsystem, and I do not think the
mechanism is worth it for what it buys. In practice the overlay author
picks an index well above anything the base system can allocate
dynamically, and if they guess wrong they get -EBUSY in dmesg pointing
at the adapter, not a misnumbered bus.

Only honoring names absent from the base DT: that addresses a
different case, an overlay redefining an existing alias to point at
another node. The notifier already handles that one: an
OF_RECONFIG_UPDATE_PROPERTY on /aliases destroys the old entry before
creating the new one, so the old node loses the id and the new node
gains it. Whether the overlay should be allowed to do that at all is a
policy question I am happy to go either way on; rejecting it is a
two-line check in the notifier. But it does not help with the dynamic
id case, because the colliding number there was never an alias in the
first place.

If you would rather see the failure mode documented than argued, I can
add a paragraph to Documentation/devicetree/overlay-notes.rst in v8
stating that an overlay alias index which is already in use causes the
device registration to fail.

> Fixes should come first in a series. Then I can apply them if ready even
> if the feature patches are not ready.

Understood, and thanks for taking 4-7. v8 will be based on dt/linus
with the remaining fixes (find_target absolute lookup, ERR_PTR from
dup_and_fixup_symbol_prop) first, then the /aliases patches.

Abdurrahman


  reply	other threads:[~2026-09-18  2:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  1:29 Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 02/10] of: update /aliases lookup on reconfig notifications Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 03/10] of/overlay: look up absolute target-paths absolutely Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 04/10] of/overlay: put property on deadprops only after changeset add succeeds Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 05/10] of/overlay: only treat a positive changeset id as registered Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 06/10] of/overlay: don't leak fragment references when changeset init fails Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 07/10] of/overlay: don't create "//" paths for fragments targeting the root Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 08/10] of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop() Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 09/10] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
2026-09-01  1:29 ` [PATCH v7 10/10] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
2026-09-17 20:39 ` [PATCH v7 00/10] of: teach overlay code to keep /aliases in sync Rob Herring
2026-09-18  2:21   ` Abdurrahman Hussain [this message]
2026-09-17 21:41 ` Rob Herring

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=DLI30THJZGQ0.2NVBET2OS94K1@nexthop.ai \
    --to=abdurrahman@nexthop.ai \
    --cc=davem@davemloft.net \
    --cc=david.daney@cavium.com \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=geert@linux-m68k.org \
    --cc=grant.likely@linaro.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pantelis.antoniou@konsulko.com \
    --cc=robh@kernel.org \
    --cc=saravanak@kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®