[19/21] google/gs101: Add dt overlay for oriole board

Message ID	20231005155618.700312-20-peter.griffin@linaro.org
State	New
Headers	show Return-Path: <linux-watchdog-owner@vger.kernel.org> From: Peter Griffin <peter.griffin@linaro.org> To: robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, conor+dt@kernel.org, sboyd@kernel.org, tomasz.figa@gmail.com, s.nawrocki@samsung.com, linus.walleij@linaro.org, wim@linux-watchdog.org, linux@roeck-us.net, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, olof@lixom.net, cw00.choi@samsung.com Cc: peter.griffin@linaro.org, tudor.ambarus@linaro.org, andre.draszik@linaro.org, semen.protsenko@linaro.org, soc@kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org, linux-watchdog@vger.kernel.org Subject: [PATCH 19/21] google/gs101: Add dt overlay for oriole board Date: Thu, 5 Oct 2023 16:56:16 +0100 Message-ID: <20231005155618.700312-20-peter.griffin@linaro.org> In-Reply-To: <20231005155618.700312-1-peter.griffin@linaro.org> References: <20231005155618.700312-1-peter.griffin@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Add minimal Tensor/GS101 SoC support and Oriole/Pixel6 board \| expand [00/21] Add minimal Tensor/GS101 SoC support and Oriole/Pixel6 board [01/21] dt-bindings: interrupt-controller: Add gs101 interrupt controller [02/21] dt-bindings: soc: samsung: exynos-pmu: Add gs101 compatible [03/21] dt-bindings: clock: Add Google gs101 clock management unit bindings [04/21] dt-bindings: soc: google: exynos-sysreg: add dedicated SYSREG compatibles to GS101 [05/21] dt-bindings: watchdog: Document Google gs101 & gs201 watchdog bindings [06/21] dt-bindings: arm: google: Add bindings for Google ARM platforms [07/21] dt-bindings: pinctrl: samsung: add google,gs101-pinctrl compatible [08/21] dt-bindings: pinctrl: samsung: add gs101-wakeup-eint compatible [09/21] dt-bindings: clock: gs101: Add cmu_top clock indices [10/21] dt-bindings: clock: gs101: Add cmu_apm clock indices [11/21] dt-bindings: clock: gs101: Add cmu_misc clock indices [12/21] clk: samsung: clk-pll: Add support for pll_{0516,0517,518} [13/21] clk: samsung: clk-gs101: Add cmu_top registers, plls, mux and gates [14/21] clk: samsung: clk-gs101: add CMU_APM support [15/21] clk: google: gs101: Add support for CMU_MISC clock unit [16/21] pinctrl: samsung: Add gs101 SoC pinctrl configuration [17/21] watchdog: s3c2410_wdt: Add support for Google tensor SoCs [18/21] arm64: dts: google: Add initial Google gs101 SoC support [19/21] google/gs101: Add dt overlay for oriole board [20/21] arm64: defconfig: Enable Google Tensor SoC [21/21] MAINTAINERS: add entry for Google Tensor SoC

Peter Griffin Oct. 5, 2023, 3:56 p.m. UTC

The LK bootloader on Pixel6 searches for a dt overlay in the
dtbo partition with a board_id and board_rev that matches
what is baked into the device. If this overlay is not present
then the phone will bootloop in fastboot and you can't boot
the upstream kernel.

This commit adds a dtbo for the production oriole variant.
The other pre-production board overlays are not included
at this time.

Adding the dtbo here allows for a better experience when
building/booting the upstream kernel on Pixel devices
as all the DT required to boot the device will be created
as part of the kernel build process. Rather than having to
fetch the dtbo from some other repo.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/google/Makefile          |  1 +
 arch/arm64/boot/dts/google/gs101-oriole.dtso | 21 ++++++++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 arch/arm64/boot/dts/google/gs101-oriole.dtso

Krzysztof Kozlowski Oct. 5, 2023, 4:33 p.m. UTC | #1

On 05/10/2023 17:56, Peter Griffin wrote:
> The LK bootloader on Pixel6 searches for a dt overlay in the
> dtbo partition with a board_id and board_rev that matches
> what is baked into the device. If this overlay is not present
> then the phone will bootloop in fastboot and you can't boot
> the upstream kernel.
> 
> This commit adds a dtbo for the production oriole variant.
> The other pre-production board overlays are not included
> at this time.
> 
> Adding the dtbo here allows for a better experience when
> building/booting the upstream kernel on Pixel devices
> as all the DT required to boot the device will be created
> as part of the kernel build process. Rather than having to
> fetch the dtbo from some other repo.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm64/boot/dts/google/Makefile          |  1 +
>  arch/arm64/boot/dts/google/gs101-oriole.dtso | 21 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/google/gs101-oriole.dtso
> 
> diff --git a/arch/arm64/boot/dts/google/Makefile b/arch/arm64/boot/dts/google/Makefile
> index 6d2026a767d4..3f1761f8daa9 100644
> --- a/arch/arm64/boot/dts/google/Makefile
> +++ b/arch/arm64/boot/dts/google/Makefile
> @@ -2,5 +2,6 @@
>  
>  dtb-$(CONFIG_ARCH_GOOGLE_TENSOR) += \
>  	gs101-oriole.dtb \
> +	gs101-oriole.dtbo
>  
>  
> diff --git a/arch/arm64/boot/dts/google/gs101-oriole.dtso b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> new file mode 100644
> index 000000000000..50832fd94204
> --- /dev/null
> +++ b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Oriole DVT Device Tree
> + *
> + * Copyright 2021-2023 Google,LLC
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	board_id = <0x20304>;
> +	board_rev = <0x10000>;

Undocumented properties. Please propose bindings... Also, underscores
are not allowed, so in this form it is a no-go... although I understand
the pain of not being able to change the bootloader.

For reference:
https://lore.kernel.org/all/20220605150747.GA3465286-robh@kernel.org/
https://lore.kernel.org/all/20220610163343.GA1787330-robh@kernel.org/


Best regards,
Krzysztof

Geert Uytterhoeven Oct. 6, 2023, 7:08 a.m. UTC | #2

Hi Peter,

On Thu, Oct 5, 2023 at 5:58 PM Peter Griffin <peter.griffin@linaro.org> wrote:
> The LK bootloader on Pixel6 searches for a dt overlay in the
> dtbo partition with a board_id and board_rev that matches
> what is baked into the device. If this overlay is not present
> then the phone will bootloop in fastboot and you can't boot
> the upstream kernel.
>
> This commit adds a dtbo for the production oriole variant.
> The other pre-production board overlays are not included
> at this time.
>
> Adding the dtbo here allows for a better experience when
> building/booting the upstream kernel on Pixel devices
> as all the DT required to boot the device will be created
> as part of the kernel build process. Rather than having to
> fetch the dtbo from some other repo.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Oriole DVT Device Tree
> + *
> + * Copyright 2021-2023 Google,LLC
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +       board_id = <0x20304>;
> +       board_rev = <0x10000>;
> +       fragment@boardbase {
> +               target-path="/";
> +               __overlay__ {
> +                       model = "Oriole DVT";
> +                       compatible = "google,gs101-oriole";
> +               };
> +       };

Please use sugar-syntax instead of manually defining
fragment/target-path/__overlay__ constructs.
You can override these properties in the root node of the base DTS
using the much simpler:

    &{/} {
            model = "Oriole DVT";
            compatible = "google,gs101-oriole";
    };

The generated DTBO should be identical (modulo naming).

> +};

Gr{oetje,eeting}s,

                        Geert

Rob Herring Oct. 6, 2023, 8:52 p.m. UTC | #3

On Thu, Oct 05, 2023 at 04:56:16PM +0100, Peter Griffin wrote:
> The LK bootloader on Pixel6 searches for a dt overlay in the
> dtbo partition with a board_id and board_rev that matches
> what is baked into the device. If this overlay is not present
> then the phone will bootloop in fastboot and you can't boot
> the upstream kernel.
> 
> This commit adds a dtbo for the production oriole variant.
> The other pre-production board overlays are not included
> at this time.
> 
> Adding the dtbo here allows for a better experience when
> building/booting the upstream kernel on Pixel devices
> as all the DT required to boot the device will be created
> as part of the kernel build process. Rather than having to
> fetch the dtbo from some other repo.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm64/boot/dts/google/Makefile          |  1 +
>  arch/arm64/boot/dts/google/gs101-oriole.dtso | 21 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/google/gs101-oriole.dtso
> 
> diff --git a/arch/arm64/boot/dts/google/Makefile b/arch/arm64/boot/dts/google/Makefile
> index 6d2026a767d4..3f1761f8daa9 100644
> --- a/arch/arm64/boot/dts/google/Makefile
> +++ b/arch/arm64/boot/dts/google/Makefile
> @@ -2,5 +2,6 @@
>  
>  dtb-$(CONFIG_ARCH_GOOGLE_TENSOR) += \
>  	gs101-oriole.dtb \
> +	gs101-oriole.dtbo

Overlays in the kernel must be able to be applied to a base DT in the 
kernel. Add a rule to apply this (hint: a '-dtbs' variable does this 
similar to -objs variables).

> diff --git a/arch/arm64/boot/dts/google/gs101-oriole.dtso b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> new file mode 100644
> index 000000000000..50832fd94204
> --- /dev/null
> +++ b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Oriole DVT Device Tree

Doesn't DVT mean pre-production?

Rob

William McVicker Oct. 9, 2023, 8:03 p.m. UTC | #4

On 10/05/2023, Krzysztof Kozlowski wrote:
> On 05/10/2023 17:56, Peter Griffin wrote:
> > The LK bootloader on Pixel6 searches for a dt overlay in the
> > dtbo partition with a board_id and board_rev that matches
> > what is baked into the device. If this overlay is not present
> > then the phone will bootloop in fastboot and you can't boot
> > the upstream kernel.
> > 
> > This commit adds a dtbo for the production oriole variant.
> > The other pre-production board overlays are not included
> > at this time.
> > 
> > Adding the dtbo here allows for a better experience when
> > building/booting the upstream kernel on Pixel devices
> > as all the DT required to boot the device will be created
> > as part of the kernel build process. Rather than having to
> > fetch the dtbo from some other repo.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  arch/arm64/boot/dts/google/Makefile          |  1 +
> >  arch/arm64/boot/dts/google/gs101-oriole.dtso | 21 ++++++++++++++++++++
> >  2 files changed, 22 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/google/gs101-oriole.dtso
> > 
> > diff --git a/arch/arm64/boot/dts/google/Makefile b/arch/arm64/boot/dts/google/Makefile
> > index 6d2026a767d4..3f1761f8daa9 100644
> > --- a/arch/arm64/boot/dts/google/Makefile
> > +++ b/arch/arm64/boot/dts/google/Makefile
> > @@ -2,5 +2,6 @@
> >  
> >  dtb-$(CONFIG_ARCH_GOOGLE_TENSOR) += \
> >  	gs101-oriole.dtb \
> > +	gs101-oriole.dtbo
> >  
> >  
> > diff --git a/arch/arm64/boot/dts/google/gs101-oriole.dtso b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> > new file mode 100644
> > index 000000000000..50832fd94204
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Oriole DVT Device Tree
> > + *
> > + * Copyright 2021-2023 Google,LLC
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +/ {
> > +	board_id = <0x20304>;
> > +	board_rev = <0x10000>;
> 
> Undocumented properties. Please propose bindings... Also, underscores
> are not allowed, so in this form it is a no-go... although I understand
> the pain of not being able to change the bootloader.
> 
> For reference:
> https://lore.kernel.org/all/20220605150747.GA3465286-robh@kernel.org/
> https://lore.kernel.org/all/20220610163343.GA1787330-robh@kernel.org/

These names are actually arbitrary and don't depend on the bootloader. They are
passed into the mkdtimg tool [1] using --id and --rev and used to create the
dt_table_entries. The bootloader traverses the table and picks the overlay
based on these properties. So we can use whatever property names we want
without changing the bootloader.

[1] https://android.googlesource.com/platform/system/libufdt/+/refs/heads/main/utils/

Thanks,
Will

> 
> 
> Best regards,
> Krzysztof
>

Peter Griffin Oct. 10, 2023, 12:09 p.m. UTC | #5

Hi Rob,

Thanks for your review!

On Fri, 6 Oct 2023 at 21:52, Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Oct 05, 2023 at 04:56:16PM +0100, Peter Griffin wrote:
> > The LK bootloader on Pixel6 searches for a dt overlay in the
> > dtbo partition with a board_id and board_rev that matches
> > what is baked into the device. If this overlay is not present
> > then the phone will bootloop in fastboot and you can't boot
> > the upstream kernel.
> >
> > This commit adds a dtbo for the production oriole variant.
> > The other pre-production board overlays are not included
> > at this time.
> >
> > Adding the dtbo here allows for a better experience when
> > building/booting the upstream kernel on Pixel devices
> > as all the DT required to boot the device will be created
> > as part of the kernel build process. Rather than having to
> > fetch the dtbo from some other repo.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  arch/arm64/boot/dts/google/Makefile          |  1 +
> >  arch/arm64/boot/dts/google/gs101-oriole.dtso | 21 ++++++++++++++++++++
> >  2 files changed, 22 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/google/gs101-oriole.dtso
> >
> > diff --git a/arch/arm64/boot/dts/google/Makefile b/arch/arm64/boot/dts/google/Makefile
> > index 6d2026a767d4..3f1761f8daa9 100644
> > --- a/arch/arm64/boot/dts/google/Makefile
> > +++ b/arch/arm64/boot/dts/google/Makefile
> > @@ -2,5 +2,6 @@
> >
> >  dtb-$(CONFIG_ARCH_GOOGLE_TENSOR) += \
> >       gs101-oriole.dtb \
> > +     gs101-oriole.dtbo
>
> Overlays in the kernel must be able to be applied to a base DT in the
> kernel. Add a rule to apply this (hint: a '-dtbs' variable does this
> similar to -objs variables).

Ok will do, thanks for the hint

>
> > diff --git a/arch/arm64/boot/dts/google/gs101-oriole.dtso b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> > new file mode 100644
> > index 000000000000..50832fd94204
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/google/gs101-oriole.dtso
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Oriole DVT Device Tree
>
> Doesn't DVT mean pre-production?

Yes, DVT stands for Design Verification Testing. I can remove that
DVT suffix for v2.
I suppose that means there were no changes between DVT and production as this
is the overlay used by the production devices.

regards,

Peter

[19/21] google/gs101: Add dt overlay for oriole board

Commit Message

Comments

Patch