[PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Bob Moragues]

Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
Zoglin is identical to Hoglin except for the SPI Flash.
The actual SPI Flash is dynamically probed at and not specified in DTS.

Signed-off-by: Bob Moragues <moragues@chromium.org>

Signed-off-by: Bob Moragues <moragues@google.com>
---

 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 581485392404..63091df3cbb3 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -475,6 +475,7 @@ properties:
 
       - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
         items:
+          - const: google,zoglin
           - const: google,hoglin
           - const: qcom,sc7280
 
-- 
2.37.1.359.gd136c6c3e2-goog

[PATCH 2/2] arm64: dts: qcom: sc7280: Add support for zoglin

[Bob Moragues]

Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
Zoglin is identical to Hoglin except for the SPI Flash.
The actual SPI Flash is dynamically probed at and not specified in DTS.

Signed-off-by: Bob Moragues <moragues@chromium.org>
Signed-off-by: Bob Moragues <moragues@google.com>
---

 arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
index 6d324ac2dda1..98d09187fc69 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "Qualcomm Technologies, Inc. sc7280 CRD platform (rev5+)";
-	compatible = "google,hoglin", "qcom,sc7280";
+	compatible = "google,zoglin", "google,hoglin", "qcom,sc7280";
 
 	/* FIXED REGULATORS */
 
-- 
2.37.1.359.gd136c6c3e2-goog

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Doug Anderson]

Hi,

On Tue, Jul 26, 2022 at 9:24 PM Bob Moragues <moragues@chromium.org> wrote:
>
> Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> Zoglin is identical to Hoglin except for the SPI Flash.
> The actual SPI Flash is dynamically probed at and not specified in DTS.
>
> Signed-off-by: Bob Moragues <moragues@chromium.org>
>
> Signed-off-by: Bob Moragues <moragues@google.com>

You need to figure out how to get your system not to add the extra
"@google.com" Signed-off-by. It's probably worth spinning a v2 with
that, if for no other reason than to debug your setup for the next
patch you send.

Other than that, this looks right to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

(you should carry my Reviewed-by tag forward on your v2 unless you
change anything significant)

Re: [PATCH 2/2] arm64: dts: qcom: sc7280: Add support for zoglin

[Doug Anderson]

Hi,

On Tue, Jul 26, 2022 at 9:24 PM Bob Moragues <moragues@chromium.org> wrote:
>
> Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> Zoglin is identical to Hoglin except for the SPI Flash.
> The actual SPI Flash is dynamically probed at and not specified in DTS.
>
> Signed-off-by: Bob Moragues <moragues@chromium.org>
> Signed-off-by: Bob Moragues <moragues@google.com>

Again, other than the extra Signed-off-by this looks right.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Rob Herring]

On Tue, Jul 26, 2022 at 09:24:31PM -0700, Bob Moragues wrote:
> Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> Zoglin is identical to Hoglin except for the SPI Flash.
> The actual SPI Flash is dynamically probed at and not specified in DTS.
> 
> Signed-off-by: Bob Moragues <moragues@chromium.org>
> 
> Signed-off-by: Bob Moragues <moragues@google.com>
> ---
> 
>  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 581485392404..63091df3cbb3 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -475,6 +475,7 @@ properties:
>  
>        - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
>          items:
> +          - const: google,zoglin
>            - const: google,hoglin
>            - const: qcom,sc7280

Is just "google,hoglin", "qcom,sc7280" no longer valid? If it is valid, 
you need another entry.

>  
> -- 
> 2.37.1.359.gd136c6c3e2-goog
> 
> 

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Doug Anderson]

Hi,

On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jul 26, 2022 at 09:24:31PM -0700, Bob Moragues wrote:
> > Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> > Zoglin is identical to Hoglin except for the SPI Flash.
> > The actual SPI Flash is dynamically probed at and not specified in DTS.
> >
> > Signed-off-by: Bob Moragues <moragues@chromium.org>
> >
> > Signed-off-by: Bob Moragues <moragues@google.com>
> > ---
> >
> >  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > index 581485392404..63091df3cbb3 100644
> > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > @@ -475,6 +475,7 @@ properties:
> >
> >        - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
> >          items:
> > +          - const: google,zoglin
> >            - const: google,hoglin
> >            - const: qcom,sc7280
>
> Is just "google,hoglin", "qcom,sc7280" no longer valid? If it is valid,
> you need another entry.

If it makes people happy to have another entry then it wouldn't hurt,
but it has no long term benefit and I would recommend against it. The
next patch in this series changes the existing "hoglin" device tree to
have all 3 compatible strings and thus when both patches land then
make dtbs_check will pass. I assume that is the only goal of
documenting these boards here. Certainly if you had a device tree that
had only "google,zoglin" it would boot fine on zoglin devices and if
you had a device tree that had only "google,hoglin" it would boot fine
on hoglin device. This is true of all of the entries for Chromebooks
that have multiple compatible entries.

-Doug

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Rob Herring]

On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Jul 26, 2022 at 09:24:31PM -0700, Bob Moragues wrote:
> > > Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> > > Zoglin is identical to Hoglin except for the SPI Flash.
> > > The actual SPI Flash is dynamically probed at and not specified in DTS.
> > >
> > > Signed-off-by: Bob Moragues <moragues@chromium.org>
> > >
> > > Signed-off-by: Bob Moragues <moragues@google.com>
> > > ---
> > >
> > >  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > index 581485392404..63091df3cbb3 100644
> > > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > @@ -475,6 +475,7 @@ properties:
> > >
> > >        - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
> > >          items:
> > > +          - const: google,zoglin
> > >            - const: google,hoglin
> > >            - const: qcom,sc7280
> >
> > Is just "google,hoglin", "qcom,sc7280" no longer valid? If it is valid,
> > you need another entry.
>
> If it makes people happy to have another entry then it wouldn't hurt,
> but it has no long term benefit and I would recommend against it. The
> next patch in this series changes the existing "hoglin" device tree to
> have all 3 compatible strings and thus when both patches land then
> make dtbs_check will pass. I assume that is the only goal of
> documenting these boards here. Certainly if you had a device tree that
> had only "google,zoglin" it would boot fine on zoglin devices and if
> you had a device tree that had only "google,hoglin" it would boot fine
> on hoglin device. This is true of all of the entries for Chromebooks
> that have multiple compatible entries.

Why even add the entry? If it is just a different SPI flash, you can
tell that from the SPI flash compatible or device ID.

Rob

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Doug Anderson]

Hi,

On Wed, Jul 27, 2022 at 12:43 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Tue, Jul 26, 2022 at 09:24:31PM -0700, Bob Moragues wrote:
> > > > Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> > > > Zoglin is identical to Hoglin except for the SPI Flash.
> > > > The actual SPI Flash is dynamically probed at and not specified in DTS.
> > > >
> > > > Signed-off-by: Bob Moragues <moragues@chromium.org>
> > > >
> > > > Signed-off-by: Bob Moragues <moragues@google.com>
> > > > ---
> > > >
> > > >  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > index 581485392404..63091df3cbb3 100644
> > > > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > @@ -475,6 +475,7 @@ properties:
> > > >
> > > >        - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
> > > >          items:
> > > > +          - const: google,zoglin
> > > >            - const: google,hoglin
> > > >            - const: qcom,sc7280
> > >
> > > Is just "google,hoglin", "qcom,sc7280" no longer valid? If it is valid,
> > > you need another entry.
> >
> > If it makes people happy to have another entry then it wouldn't hurt,
> > but it has no long term benefit and I would recommend against it. The
> > next patch in this series changes the existing "hoglin" device tree to
> > have all 3 compatible strings and thus when both patches land then
> > make dtbs_check will pass. I assume that is the only goal of
> > documenting these boards here. Certainly if you had a device tree that
> > had only "google,zoglin" it would boot fine on zoglin devices and if
> > you had a device tree that had only "google,hoglin" it would boot fine
> > on hoglin device. This is true of all of the entries for Chromebooks
> > that have multiple compatible entries.
>
> Why even add the entry? If it is just a different SPI flash, you can
> tell that from the SPI flash compatible or device ID.

Yeah, it's really unfortunate. :( The issue is a limitation in the
ChromeOS bootloader infrastructure. The ChromeOS build infrastructure
cannot handle something that it considers the same "board" as having
different SPI flash sizes. This is because the infrastructure always
requires that the bootloader "image" be the exact same size as the SPI
flash and it assumes a universal firmware (single image) per board.
It's unfortunately not very flexible but normally for a given board
the SPI flash size is chosen at the start and never changed. The CRD
board was an exception here. Though it's not beautiful, this means
that the firmware considers this as a different board and looks for a
different compatible string on the kernel command line.

-Doug

Re: [PATCH 1/2] dt-bindings: arm: qcom: document zoglin board

[Rob Herring]

On Wed, Jul 27, 2022 at 3:59 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 27, 2022 at 12:43 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Tue, Jul 26, 2022 at 09:24:31PM -0700, Bob Moragues wrote:
> > > > > Zoglin is a Hoglin Chromebook with SPI Flash reduced from 64MB to 8MB.
> > > > > Zoglin is identical to Hoglin except for the SPI Flash.
> > > > > The actual SPI Flash is dynamically probed at and not specified in DTS.
> > > > >
> > > > > Signed-off-by: Bob Moragues <moragues@chromium.org>
> > > > >
> > > > > Signed-off-by: Bob Moragues <moragues@google.com>
> > > > > ---
> > > > >
> > > > >  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > > index 581485392404..63091df3cbb3 100644
> > > > > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > > > > @@ -475,6 +475,7 @@ properties:
> > > > >
> > > > >        - description: Qualcomm Technologies, Inc. sc7280 CRD platform (newest rev)
> > > > >          items:
> > > > > +          - const: google,zoglin
> > > > >            - const: google,hoglin
> > > > >            - const: qcom,sc7280
> > > >
> > > > Is just "google,hoglin", "qcom,sc7280" no longer valid? If it is valid,
> > > > you need another entry.
> > >
> > > If it makes people happy to have another entry then it wouldn't hurt,
> > > but it has no long term benefit and I would recommend against it. The
> > > next patch in this series changes the existing "hoglin" device tree to
> > > have all 3 compatible strings and thus when both patches land then
> > > make dtbs_check will pass. I assume that is the only goal of
> > > documenting these boards here. Certainly if you had a device tree that
> > > had only "google,zoglin" it would boot fine on zoglin devices and if
> > > you had a device tree that had only "google,hoglin" it would boot fine
> > > on hoglin device. This is true of all of the entries for Chromebooks
> > > that have multiple compatible entries.
> >
> > Why even add the entry? If it is just a different SPI flash, you can
> > tell that from the SPI flash compatible or device ID.
>
> Yeah, it's really unfortunate. :( The issue is a limitation in the
> ChromeOS bootloader infrastructure. The ChromeOS build infrastructure
> cannot handle something that it considers the same "board" as having
> different SPI flash sizes. This is because the infrastructure always
> requires that the bootloader "image" be the exact same size as the SPI
> flash and it assumes a universal firmware (single image) per board.
> It's unfortunately not very flexible but normally for a given board
> the SPI flash size is chosen at the start and never changed. The CRD
> board was an exception here. Though it's not beautiful, this means
> that the firmware considers this as a different board and looks for a
> different compatible string on the kernel command line.

Okay, I guess...

Acked-by: Rob Herring <robh@kernel.org>