Message-ID: <CAD=FV=VSxeKgGcsRb9qiXq7nsbOWZjPvzmGEOhFA+pmABWdknQ@mail.gmail.com>
Date: Thu, 13 Nov 2025 08:23:52 -0800
From: Doug Anderson <dianders@...omium.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Peter Griffin <peter.griffin@...aro.org>, 
	André Draszik <andre.draszik@...aro.org>, 
	Tudor Ambarus <tudor.ambarus@...aro.org>, linux-samsung-soc@...r.kernel.org, 
	Roy Luo <royluo@...gle.com>, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, Chen-Yu Tsai <wenst@...omium.org>, 
	Julius Werner <jwerner@...omium.org>, William McVicker <willmcvicker@...gle.com>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] dt-bindings: arm: google: Add bindings for frankel/blazer/mustang

Hi,

On Wed, Nov 12, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> >>> +      # Google Tensor G5 AKA lga (laguna) SoC and boards
> >>> +      - description: Tensor G5 SoC (laguna)
> >>> +        items:
> >>> +          - enum:
> >>> +              - google,soc-id-0005-rev-00  # A0
> >>> +              - google,soc-id-0005-rev-10  # B0
> >>
> >> SoCs cannot be final compatibles.
> >
> > Right. I talked about this at length "after the cut" in my patch. See
> > above. I wish to relitigate this policy and wish to know more details
> > about where it is documented, the reasons for decision, and where the
> > boundary exactly lies between something that's allowed to be a final
> > compatible and something that's not. I made several arguments above
> > for why I think the SoC should be allowed as a final compatible, so it
>
> Because this represents a actual device users run. It is electronically,
> physically impossible to run the SoC alone.

I'm not convinced that this definition is as clear as you're making it
out to be. It's physically impossible to run many "boards" alone.

Want to boot up a Raspberry Pi? Provide it with power. Hook up a
display to it. Hook up a keyboard to it. Plug in an Ethernet cable.
Plug an SD card in it. Without those things it doesn't run.

Want to boot up a lga-B0 SoC? Hook up power to the power pins. Connect
a MIPI panel to the MIPI pins. Connect a UFS chip to the UFS pins.
Without those things it doesn't run.

Yes, the complexity of just "hooking up" the components on an SoC is
an order of magnitude harder than a Raspberry Pi, but it's still just
hooking it up to external components. In both cases, we are modeling
the core "brains" (the part that contains the processor) as the DTB
and everything else just "hooks up" to interfaces.

If I had to make a definition for what the base DTB should be it
should be the component with the boot CPU. _Why_ is that the wrong
definition?


> There are few - one or two - exceptions for the SoMs, but never for SoC.

OK, but the big question: _WHY???_

Where does it say that a DTB has to be something that can run "alone"
and (most importantly) what benefit do we get for making that
requirement (AKA _WHY_)? This is the question you're not answering.
>From Rob's response, he doesn't seem to think that a DTB needs to be
for something that can boot alone and he seems OK with allowing a
fairly flexible split here. Rob's response was focused on making sure
we can do validation and I'd love to continue on with that discussion.


> > would be great if you could respond to them and tell me where I got it
> > wrong.
> >
> >
> >> Your commit msg does not explain what
> >> is 'soc-id' or 'soc_id' in this context.
> >
> > In the commit message I do say: "SoC: an A0 pre-production variant (ID
> > 0x000500) and a B0 variant (ID 0x000510) used in production. The ID is
> > canonicaly broken up into a 16-bit SoC product ID, a 4-bit major rev,
> > and a 4-bit minor rev."
>
> >
> > ...then, I further say "In this patch, put the SoC IDs straight into
>
> That's fine.
>
> > the compatible. Though the bootloader doesn't look at the compatible
> > at the moment, this should be easy to teach the bootloader about."
>
> But nothing explains why this SoC can be run alone without board.
> >
> > The idea here is for the bootloader, which can read the ID of the
> > current SoC, to be able to pick the right device tree from among
> > multiple. I am certainly not married to putting the SoC ID in the
>
> I am not discussing about style of the compatible. I said - you cannot
> have SoC compatible alone. None of above gives any argument for that.
>
> > compatible like this. As I mentioned above, in downstream device trees
> > the SoC is stored in a custom node and I thought upstream would hate
> > that. I also considered giving the `soc@0` node a custom compatible
> > string and adding properties about the SoC ID underneath that and
> > teaching the bootloader how to find this, and I can switch to this if
> > you prefer.
> >
> > If you have an alternate technique for which the bootloader could pick
> > a device tree based on the current SoC ID or you have specific wording
> > that you think I should add to the commit message to explain my
> > current scheme, I'm happy to adjust things.
> >
> >
> >>> +          - const: google,lga
> >>> +      - description: Google Pixel 10 Board (Frankel)
> >>> +        items:
> >>> +          - enum:
> >>> +              - google,pixel-id-070302-rev-000000  # Proto 0
> >>> +              - google,pixel-id-070302-rev-010000  # Proto 1
> >>> +              - google,pixel-id-070302-rev-010100  # Proto 1.1
> >>> +              - google,pixel-id-070303-rev-010000  # EVT 1
> >>> +              - google,pixel-id-070303-rev-010100  # EVT 1.1
> >>> +              - google,pixel-id-070303-rev-010101  # EVT 1.1 Wingboard
> >>> +              - google,pixel-id-070304-rev-010000  # DVT 1
> >>> +              - google,pixel-id-070305-rev-010000  # PVT 1
> >>> +              - google,pixel-id-070306-rev-010000  # MP 1
> >>> +          - const: google,lga-frankel
> >>> +          - const: google,lga
> >>
> >> So what is the lga?
> >
> > "google,lga" is the name of the processor. I was under the impression
> > that the last entry in the top-level compatible string was supposed to
> > be the SoC compatible string. Certainly this was true in every board
>
> google,soc-id-0005-rev-00 is the soc compatible string.
>
> > I've worked with and I seem to even recall it being requested by DT
> > folks. It also seems to match what I see in examples in the kernel
> > docs [1].
>
> Sorry but no. Writing bindings do not request having two compatibles for
> the same soc in two different, independent (orthogonal) lists.
>
> So it is rev-xyz + google,lga-frankel + soc-id + lga, if you need that
> soc-id part.

Probably not worth continuing to discuss until we can agree that the
SoC can be in its own dtb. If we can agree upon that we can talk about
if we need the SoC part in the top-level compatible and how to
accomplish that. I had a few ideas / suggestions in my response to Rob
that roughly break down into:

* Don't model the SoC in the top-level compatible. Maybe put it in the
soc@0 node.

* Come up with a rule for "merging" top-level compatibles if a base
and overlay both define.

Whatever solution we come up with, I want to make sure it handles our
socketed dev boards where the SoC can be removed and replaced with a
different one.


> >>> +allOf:
> >>>    # Bootloader requires empty ect node to be present
> >>> -  ect:
> >>> -    type: object
> >>> -    additionalProperties: false
> >>
> >> Please keep it here
> >
> > "it" being "additionalProperties", I think? I'm not sure I understand,
> > but let's discuss below in the context of full examples and not diffs.
>
> I meant ect node, existing hunk. Properties must be defined in top-level.

OK, much clearer. Thanks!


> > The best my brain can parse your request, I think you're asking for this:
> >
> > --
> >
> > properties:
> >   ...
> >   ...
> >   ect:
> >     type: object
> >     additionalProperties: false
> >
> > allOf:
> >   # Bootloader requires empty ect node to be present
> >   - if:
> >       properties:
> >         compatible:
> >           not:
> >             contains:
> >               const: google,gs101
> >     then:
> >       properties:
> >         ect: false
> >     else:
> >       required:
> >       - ect
>
> Yes, actually now I see you could drop the "not:" and exchange the
> "then:else:" branches.

For the sake of clarity, I'll go with this:

properties:
  ...
  ...
  ect:
    type: object
    additionalProperties: false

allOf:
  # Bootloader requires empty ect node to be present
  - if:
      properties:
        compatible:
          contains:
            const: google,gs101
    then:
      required:
      - ect

additionalProperties: true

Powered by blists - more mailing lists