Linux Serial subsystem development
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Doug Anderson <dianders@chromium.org>,
	Rob Herring <robh@kernel.org>, Conor Dooley <conor@kernel.org>
Cc: "Peter Griffin" <peter.griffin@linaro.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"André Draszik" <andre.draszik@linaro.org>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Jiri Slaby" <jirislaby@kernel.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>, "Arnd Bergmann" <arnd@arndb.de>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Linus Walleij" <linusw@kernel.org>,
	"Drew Fustini" <fustini@kernel.org>,
	"Kees Cook" <kees@kernel.org>, "Tony Luck" <tony.luck@intel.com>,
	"Guilherme G. Piccoli" <gpiccoli@igalia.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-serial@vger.kernel.org,
	soc@lists.linux.dev, "Juan Yescas" <jyescas@google.com>,
	"RD Babiera" <rdbabiera@google.com>,
	"Brian Norris" <briannorris@google.com>,
	"William McVicker" <willmcvicker@google.com>,
	kernel-team@android.com
Subject: Re: [PATCH v2 4/5] arm64: dts: google: Add initial dts for frankel/blazer/mustang
Date: Wed, 19 Aug 2026 11:02:21 +0200	[thread overview]
Message-ID: <c0657108-1958-4c4c-b5f5-0eb24e81b2b4@kernel.org> (raw)
In-Reply-To: <CAD=FV=V2+D1m5L-0d81W3E_ih2us-SrddeBUAJtMybpQons7Ww@mail.gmail.com>

On 19/08/2026 00:13, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 30, 2026 at 4:32 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>>>> +     /*
>>>> +      * The Pixel bootloader considers it a fatal error if it doesn't find
>>>> +      * a `ufs0` alias so it can add calibration data to the node. Until
>>>
>>> Fake node is ok, but alias won't fly because aliases are not allowed for
>>> ufs. Well, would work 10 years ago, but this is a device from ~2025 (so
>>> SoC maybe a bit earlier), thus Google already knew that they MUST talk
>>> with upstream open source maintainers before they ship such ABI.
>>>
>>> They did not talk, so you reap what you sow.
>>>
>>> There is no more excuse for a vendor to ignore open source and push
>>> whatever-ABI-they-wish into their product, if they ever want to upstream
>>> that product.
>>>
>>> I know it is not your fault, obviously. And I know that not much you can
>>> do, so that is not rant towards you nor towards Doug.
>>>
>>> You will have to keep this part of patch out of tree or fix the Pixel
>>> bootloader.
>>
>> FWIW, it actually _is_ a rant towards me, since I added the "ufs0" alias. :-P
>>
>> When I was originally bringing up Pixel 10 with upstream, the
>> bootloader had a hardcoded path to the UFS node. It looked for it at
>> "/ufs@3c400000". That certainly wasn't going to work. Downstream
>> _still_ hasn't transitioned to having a "soc@0" node to put all the
>> MMIO peripherals under, so the equivalent upstream path would be
>> "/soc@0/ufs@3c400000"
>>
>> Now, I certainly could have made the bootloader search both paths, but
>> that seemed bad to me because:
>>
>> 1. As I understand it, DT paths aren't ABI. While it feels unlikely
>> upstream would change "/soc@0/ufs@3c400000" to something else, I
>> believe upstream would feel free to and not consider it a "breaking"
>> change. This makes it feel unwise to hardcode the path in the
>> bootloader. In the past, upstream has renamed nodes to clean them up
>> and it wasn't considered a violation of the sanctity of the
>> device-tree ABI.
>>
>> 2. If #1 is untrue and we consider DT paths as ABI, it's still a bit
>> awkward. We have one bootloader base that supports multiple SoCs. The
>> unit address differs across SoCs, even though the IP block is nearly
>> the same (bootloader still adds the same type of calibration data to
>> the node). The code I started with had a bunch of #if statements for
>> the paths in various SoC variants, and that went away with the alias.
>> I suppose the bootloader needs to know the UFS base address anyway so
>> I could have probably constructed the node name based on other
>> #defines, but it still was a bit awkward.
>>
>> 3. I certainly could have searched the whole device tree for the UFS
>> node by "compatible" string, but the Pixel 10 (and future) UFS
>> controllers aren't upstream yet. We wouldn't be able to land the Pixel
>> 10 device tree without the UFS bindings landed yet and I think we're a
>> bit far away from getting the Pixel 10 UFS bindings landed...
>>
>> With all that, the "aliases" seemed like a pretty clean way for the
>> bootloader to find the UFS node. It also matched my understanding of
>> an appropriate use of an "alias".
>>
>> Any suggestions for how to resolve this? Do we go back to hardcoding a
>> path in the bootloader and cross our fingers that upstream never
>> cleans up anything that changes the path to the UFS node? Would it
>> really be terrible to allow a "ufs0" alias for this case?
>>
>> As a side note, I did "talk" to upstream shortly after adding the
>> "ufs0" node by sending the Pixel 10 patches upstream, but I guess we
>> were so focused on the overlay topic that nobody thought to comment on
>> the "ufs0" node? At the time, I'm fairly certain my resulting device
>> tree files passed schema validation at the time, too...
> 
> I guess no response / silence == my email was so dumb that it wasn't
> worth responding to? Even despite that, we still need to find a way to
> move forward, so popping back here...
> 
> I did some digging. As far as I can tell:
> 
> * Nothing in the DeviceTree specification 0.4 [1] mentions that
> aliases are deprecated.
> 
> * Nothing in the dt-schema repository [2] causes validation to fail
> when you use new aliases and there is no "allowlist" of old aliases
> that are allowed for historical reasons.
> 
> * There is a single reference in the kernel "Documentation/devicetree"
> about not using aliases to assign an "instance ID" [3].
> 
> Is there some other documentation saying "aliases == evil" that I
> missed? Maybe some email thread we're all supposed to have read?

A lot of rules are implied by other rules and this one, how Linus stated
in other thread, might be implied by no-Linuxisms as you want ordering
or stable naming of Linux /dev entries.

I understand your reason is actually different than above, but your code
does not suggest that.

Anyway, if you wanted to have aliases as ABI, it would have to be
documented. You cannot send post-factum DTS and say "we already use it".
Every ABI must be documented before usage.

And this is what my comment was about: "they MUST talk
with upstream open source maintainers before they ship such ABI."

And no, sending such DTS in your v1 is not documenting ABI. Does not count.

> 
> Is the only issue here the fact that the alias ends with a "0" and
> thus implicitly provides an "instance ID"?  Would it be OK if I
> changed my alias name to "ufs-primary" or "ufs-internal" or "ufs-boot"
> or just "ufs"? We're not using the alias to get an instance ID, but
> when I added the alias I followed the pattern of all the other aliases
> and put an number at the end.
> 
> I'm happy to attempt to fix our bootloader using whatever scheme
> upstream suggests. I'm trying to "talk to upstream" as requested, but
> for it to work I need upstream to talk back. :-)

Make your case - what is the purpose of it? Boot device? Then you have
"chosen" node for stuff between firmware and OS. There is even a
property called "bootsource". If this is not boot device, but some
calibration data for ONE given instance of IP, regardless whether you
boot from it or not, then I find such case as border-base and not worth
implementing, because basically one can come one month later with "I
need 1000 aliases because my bootloader is patching up every device
node". It's called overlays then...

Best regards,
Krzysztof

  parent reply	other threads:[~2026-08-19  9:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22  9:55 [PATCH v2 0/5] Add Laguna/Tensor G5 SoC and Frankel, Blazer & Mustang boards Peter Griffin
2026-07-22  9:55 ` [PATCH v2 1/5] dt-bindings: arm: google: Add dt bindings for frankel/blazer/mustang Peter Griffin
2026-07-24  6:42   ` Krzysztof Kozlowski
2026-07-30 23:33   ` Doug Anderson
2026-07-22  9:55 ` [PATCH v2 2/5] dt-bindings: serial: snps-dw-apb-uart: Add "google,lga-uart" Peter Griffin
2026-07-22  9:55 ` [PATCH v2 3/5] arm64: dts: google: Add dts directory for Google-designed silicon Peter Griffin
2026-07-24  6:46   ` Krzysztof Kozlowski
2026-07-22  9:55 ` [PATCH v2 4/5] arm64: dts: google: Add initial dts for frankel/blazer/mustang Peter Griffin
2026-07-22 23:51   ` Brian Norris
2026-07-24  6:59   ` Krzysztof Kozlowski
2026-07-30 23:32     ` Doug Anderson
2026-08-18 22:13       ` Doug Anderson
2026-08-19  8:28         ` Linus Walleij
2026-08-19  9:02         ` Krzysztof Kozlowski [this message]
2026-08-19  9:26         ` Krzysztof Kozlowski
2026-07-22  9:55 ` [PATCH v2 5/5] arm64: defconfig: enable Tensor G5 SoC family Peter Griffin
2026-07-30 23:34   ` Doug Anderson
2026-07-22 23:58 ` [PATCH v2 0/5] Add Laguna/Tensor G5 SoC and Frankel, Blazer & Mustang boards Brian Norris
2026-07-24  6:40   ` Krzysztof Kozlowski

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=c0657108-1958-4c4c-b5f5-0eb24e81b2b4@kernel.org \
    --to=krzk@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andre.draszik@linaro.org \
    --cc=arnd@arndb.de \
    --cc=briannorris@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=fustini@kernel.org \
    --cc=gpiccoli@igalia.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=jyescas@google.com \
    --cc=kees@kernel.org \
    --cc=kernel-team@android.com \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=peter.griffin@linaro.org \
    --cc=rdbabiera@google.com \
    --cc=robh@kernel.org \
    --cc=soc@lists.linux.dev \
    --cc=tony.luck@intel.com \
    --cc=tudor.ambarus@linaro.org \
    --cc=will@kernel.org \
    --cc=willmcvicker@google.com \
    /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