Hi Krzysztof,
On Sun, 2024-12-22 at 12:42 +0100, Krzysztof Kozlowski wrote:
> On 20/12/2024 12:27, André Draszik wrote:
> > In order to support Pixel 6 (Oriole), Pixel 6 Pro (Raven), and Pixel 6a
> > (Bluejay) correctly, we have to be able to distinguish them properly as
> > we add support for more features.
> >
> > For example, Raven has a larger display. There are other differences,
> > like battery design capacity, etc.
> >
> > To facilitate this, we create a generic gs101-based Pixel DT that can
> > work on any such gs101-based device. At the same time, we move the
>
> No, whatever insanity Android has there, please don't populate it to
> upstream.
>
> There is no such thing as "generic board" thus cannot be a
> "generic DTS".
I'll rephrase to gs101-based Pixel base board. Unless you have a better
suggestion.
>
> > Oriole specific parts that we have at the moment (display) into an
> > overlay, making it easy to add support for Raven and Bluejay in a
> > similar way.
> >
> > Note1:
> > Despite being an overlay, we instruct kbuild to create a merged
> > gs101-oriole.dtb and a gs101-oriole.dtbo. This way existing scripts can
> > keep working, but it also gives the option to just apply the overlay
> > before boot (e.g. by the bootloader).
> >
> > Note2:
> > I've changed the simple-framebuffer node to specify the memory via
> > memory-region instead of reg, as that avoids unnecessary duplication
> > (of the size), and it avoids having to specify #address-cells
> > and #size-cells in the chosen node (and duplicating this in the DTSO),
> > which is otherwise necessary to keep dt_binding_check happy and DT
> > validation working in general.
> >
> > Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx>
> > ---
> > Note: MAINTAINERS doesn't need updating, it covers this whole directory
> > ---
> > arch/arm64/boot/dts/exynos/google/Makefile | 6 ++--
> > .../arm64/boot/dts/exynos/google/gs101-oriole.dtso | 33 ++++++++++++++++++++++
> > .../{gs101-oriole.dts => gs101-pixel-generic.dts} | 24 +++++++---------
> > 3 files changed, 47 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/google/Makefile b/arch/arm64/boot/dts/exynos/google/Makefile
> > index 0a6d5e1fe4ee..6e6b5319212a 100644
> > --- a/arch/arm64/boot/dts/exynos/google/Makefile
> > +++ b/arch/arm64/boot/dts/exynos/google/Makefile
> > @@ -1,4 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > -dtb-$(CONFIG_ARCH_EXYNOS) += \
> > - gs101-oriole.dtb \
> > +dtb-$(CONFIG_ARCH_EXYNOS) += gs101-pixel-generic.dtb
> > +
> > +gs101-oriole-dtbs := gs101-pixel-generic.dtb gs101-oriole.dtbo
> > +dtb-$(CONFIG_ARCH_EXYNOS) += gs101-oriole.dtb
> > diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso
> > new file mode 100644
> > index 000000000000..43572039cd07
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso
> > @@ -0,0 +1,33 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Oriole Device Tree
> > + *
> > + * Copyright 2021-2023 Google LLC
> > + * Copyright 2023-2024 Linaro Ltd
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&{/} {
> > + model = "Oriole";
> > + compatible = "google,gs101-oriole", "google,gs101-pixel", "google,gs101";
>
> Boards are not overlays. Board equals DTB.
You're saying this should move into a dts instead of dtso?
There are numerous boards upstream which use this same dtso
approach.
There is a base board, and also different versions of it,
oriole being one of them.
[...]
> > };
> >
> > chosen {
> > - #address-cells = <2>;
> > - #size-cells = <1>;
> > - ranges;
> > -
> > /* Bootloader expects bootargs specified otherwise it crashes */
> > bootargs = "";
> > stdout-path = &serial_0;
> >
> > /* Use display framebuffer as setup by bootloader */
> > - framebuffer0: framebuffer@fac00000 {
>
> I don't think this exists in current source. It does exist in thing I
> was applying, but then this does not make much sense: add a framebuffer
> which has to be changed, because it is not correct.
>
> I'll drop the framebuffer patch.
The framebuffer makes sense for the oriole version. I can fully remove this
node from this base board here, and just leave it in the versions.
That said, can you please expand how having a 'status=disabled' framebuffer
node is different from any other nodes that are enabled or modified by
overlays?
Cheers,
Andre'
