2022-07-27 04:35:15

by Bob Moragues

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

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 <[email protected]>

Signed-off-by: Bob Moragues <[email protected]>
---

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


2022-07-27 13:50:03

by Doug Anderson

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

Hi,

On Tue, Jul 26, 2022 at 9:24 PM Bob Moragues <[email protected]> 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 <[email protected]>
>
> Signed-off-by: Bob Moragues <[email protected]>

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 <[email protected]>

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

2022-07-27 18:29:11

by Rob Herring (Arm)

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

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 <[email protected]>
>
> Signed-off-by: Bob Moragues <[email protected]>
> ---
>
> 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
>
>

2022-07-27 19:22:58

by Doug Anderson

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

Hi,

On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <[email protected]> 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 <[email protected]>
> >
> > Signed-off-by: Bob Moragues <[email protected]>
> > ---
> >
> > 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

2022-07-27 20:07:52

by Rob Herring (Arm)

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

On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <[email protected]> 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 <[email protected]>
> > >
> > > Signed-off-by: Bob Moragues <[email protected]>
> > > ---
> > >
> > > 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

2022-07-27 23:15:09

by Doug Anderson

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

Hi,

On Wed, Jul 27, 2022 at 12:43 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <[email protected]> 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 <[email protected]>
> > > >
> > > > Signed-off-by: Bob Moragues <[email protected]>
> > > > ---
> > > >
> > > > 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

2022-07-27 23:15:28

by Rob Herring (Arm)

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

On Wed, Jul 27, 2022 at 3:59 PM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, Jul 27, 2022 at 12:43 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Jul 27, 2022 at 11:40 AM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Jul 27, 2022 at 9:03 AM Rob Herring <[email protected]> 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 <[email protected]>
> > > > >
> > > > > Signed-off-by: Bob Moragues <[email protected]>
> > > > > ---
> > > > >
> > > > > 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 <[email protected]>