| Message ID | 20220726212354.1.I5b9006878bdabd6493b866b46dbd6149968d545b@changeid |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: arm: qcom: document zoglin board | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
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)
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>
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 > >
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
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
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
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>
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