Re: [PATCH 1/3] dt-bindings: arm: qcom: Add sc7180 Chromebook board bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, May 15, 2022 at 11:40 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 13/05/2022 19:00, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, May 13, 2022 at 2:01 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>
> >> On 13/05/2022 09:57, Krzysztof Kozlowski wrote:
> >>> On 12/05/2022 18:04, Douglas Anderson wrote:
> >>>> This copy-pastes compatibles from sc7180-based boards from the device
> >>>> trees to the yaml file so that `make dtbs_check` will be happy.
> >>>>
> >>>> NOTES:
> >>>> - I make no attempt to try to share an "item" for all sc7180 based
> >>>>   Chromebooks. Because of the revision matching scheme used by the
> >>>>   Chromebook bootloader, at times we need a different number of
> >>>>   revisions listed.
> >>>> - Some of the odd entries in here (like google,homestar-rev23 or the
> >>>>   fact that "Google Lazor Limozeen without Touchscreen" changed from
> >>>>   sku5 to sku6) are not typos but simply reflect reality.
> >>>> - Many revisions of boards here never actually went to consumers, but
> >>>>   they are still in use within various companies that were involved in
> >>>>   Chromebook development. Since Chromebooks are developed with an
> >>>>   "upstream first" methodology, having these revisions supported with
> >>>>   upstream Linux is important. Making it easy for Chromebooks to be
> >>>>   developed with an "upstream first" methodology is valuable to the
> >>>>   upstream community because it improves the quality of upstream and
> >>>>   gets Chromebooks supported with vanilla upstream faster.
> >>>>
> >>>> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >>>> ---
> >>>>
> >>>>  .../devicetree/bindings/arm/qcom.yaml         | 180 ++++++++++++++++++
> >>>>  1 file changed, 180 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> >>>> index 5c06d1bfc046..399be67eb5d2 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> >>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> >>>> @@ -214,11 +214,191 @@ properties:
> >>>>                - qcom,ipq8074-hk10-c2
> >>>>            - const: qcom,ipq8074
> >>>>
> >>>> +      # Qualcomm Technologies, Inc. SC7180 IDP
> >>>>        - items:
> >>>>            - enum:
> >>>>                - qcom,sc7180-idp
> >>>>            - const: qcom,sc7180
> >>>>
> >>>> +      # Google CoachZ (rev1 - 2)
> >>>> +      - items:
> >>>> +          - const: google,coachz-rev1
> >>>> +          - const: google,coachz-rev2
> >>>
> >>> The inverted pattern of old revision being compatible with the new one,
> >>> is done on purpose? You claim here every rev1 is always compatible with
> >>> rev2 ...
> >>>
> >>> I don't think we discussed such patterns in previous talk. I quickly
> >>> went through it and there were only skuX moving around, not rev1 being
> >>> newer then rev2.
> >
> > Isn't this what we just had a whole discussion about?
> >
> > Oh, I see. You're objecting to the fact that the order here lists
> > "rev1" first and "rev2" second.
> >
> > I think the issue here is that for the purposes of booting Chromebooks
> > the order here doesn't matter. Certainly we can pick a fixed order and
> > we can validate that the order in the yaml matches the order in the
> > device tree, but for all intents and purposes it doesn't matter to
> > anything. The same device tree is compatible with _both_ rev1 and rev2
> > coachz devices. Neither of those two devices is inherently better
> > supported by this device tree than the other.
>
> OK, thanks for explanation. Since these were not documented maybe fixing
> existing DTS to more expected order (rev2 being the newest, rev1
> following) would make sense. But certainly please use such new order
> compatibles for new DTSes.

I'm still not sure I understand: if the list of revisions is
effectively unordered, why does it matter which order they are listed
in? Certainly we can change the order, but I'm not sure how I justify
the extra churn in my patch description.

-Doug



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux