Re: [PATCH 1/2] dt-bindings: arm: rockchip: Add gru-scarlet sku{2,4} variants

[Brian Norris]

Hi Krzysztof,

On Thu, Aug 18, 2022 at 3:02 AM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
> On 17/08/2022 22:33, Brian Norris wrote:
> > The Gru-Scarlet family includes a variety of SKU identifiers, using
> > parts of a 3-bit space {0..7}. SKU2 and SKU4 devices (under a few
> > different manufacturer names) also use the Innolux display.
> >
> > For reference, the original vendor tree source:
> >
> > CHROMIUM: arm64: dts: rockchip: add sku{0,2,4} compatibility
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f6ed665c9e2eb37fb2680debbb36ec9fb0e8fb97
> >
> > CHROMIUM: arm64: dts: rockchip: scarlet: add SKU0 device tree
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9987c8776f4b087d135d761e59f7fa6cc83fc7fc
> >
> > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>

> > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml

> > +          - const: google,scarlet-rev15-sku2
> > +          - const: google,scarlet-rev15-sku4
>
> This does not match the sources you linked in commit msg, so I am
> confused what the links are supposed to prove.

It took 2 patches to get there (because SKU0 had some additional
customizations later, which were already upstreamed [0]), but the
result is the same. I'm not sure which part you think doesn't match.

One difference: they're listed in different order, because that seems
to be how the YAML schema is organized. But it doesn't make any
material difference, as long as the -skuX variants are listed before
the non-skuX variants (i.e., "more specific" goes first).

As to what they prove? Well, whoever applies is free to drop them if
they'd like, but I figured more documentation is better. IMO, it shows
that the real product uses those strings, and implies (but not quite
proves) the bootloader is looking for those. That is useful
information, if one expects to use an upstream kernel with the
production bootloader.

[0] https://git.kernel.org/linus/5707e34166f546bf1fcdfd3da600e8187d04d937
arm64: dts: rockchip: Add gru-scarlet-dumo board

> Is this matching at least your DTS (dtbs_check passes)?

Yes. (Well, after patch 2. I didn't try to make this bisectable.)

Brian