[PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Jimmy Chen]

This adds yaml file for new a LTE skus for villager device.

Signed-off-by: Jimmy Chen <jinghung.chen3@hotmail.com>
---

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

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 4dd18fbf20b68..a136b1389c2ac 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -529,11 +529,26 @@ properties:
           - const: google,herobrine
           - const: qcom,sc7280
 
+      - description: Google Villager (rev0)
+        items:
+          - const: google,villager-rev0
+          - const: qcom,sc7280
+
       - description: Google Villager (newest rev)
         items:
           - const: google,villager
           - const: qcom,sc7280
 
+      - description: Google Villager with LTE (rev0)
+        items:
+          - const: google,villager-rev0-sku0
+          - const: qcom,sc7280
+
+      - description: Google Villager with LTE (newest rev)
+        items:
+          - const: google,villager-sku0
+          - const: qcom,sc7280
+
       - items:
           - enum:
               - lenovo,flex-5g
-- 
2.25.1

Re: [PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Krzysztof Kozlowski]

On 04/07/2022 09:09, Jimmy Chen wrote:
> This adds yaml file for new a LTE skus for villager device.
> 
> Signed-off-by: Jimmy Chen <jinghung.chen3@hotmail.com>
> ---
> 
>  Documentation/devicetree/bindings/arm/qcom.yaml | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 4dd18fbf20b68..a136b1389c2ac 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -529,11 +529,26 @@ properties:
>            - const: google,herobrine
>            - const: qcom,sc7280
>  
> +      - description: Google Villager (rev0)
> +        items:
> +          - const: google,villager-rev0
> +          - const: qcom,sc7280
> +
>        - description: Google Villager (newest rev)
>          items:
>            - const: google,villager
>            - const: qcom,sc7280
>  
> +      - description: Google Villager with LTE (rev0)
> +        items:
> +          - const: google,villager-rev0-sku0
> +          - const: qcom,sc7280
> +
> +      - description: Google Villager with LTE (newest rev)
> +        items:
> +          - const: google,villager-sku0
> +          - const: qcom,sc7280
> +

All these should be one entry - one enum. If you really need some
descriptive text, add a comment.

Best regards,
Krzysztof

Re: [PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Jimmy Chen]

To keep the consistency of the format for Chromebook items,
we basically add these items based on the rules discussed in the previous patch series here
https://lore.kernel.org/all/20220520143502.v4.1.I71e42c6174f1cec17da3024c9f73ba373263b9b6@changeid/.

We are little configured with “one entry - one enum “. Do you mean something like below example?
We suggest keep items separated as it is more readable.

      - description: Google Villager
        items:
          - const: google,villager-rev0
          - const: google,villager
          - const: google,villager-rev0-sku0
          - const: google,villager-sku0
          - const: qcom,sc7280

Best regards,
Jimmy Chen

Re: [PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Krzysztof Kozlowski]

On 06/07/2022 11:36, Jimmy Chen wrote:
> To keep the consistency of the format for Chromebook items,
> we basically add these items based on the rules discussed in the previous patch series here
> https://lore.kernel.org/all/20220520143502.v4.1.I71e42c6174f1cec17da3024c9f73ba373263b9b6@changeid/.
> 
> We are little configured with “one entry - one enum “. Do you mean something like below example?
> We suggest keep items separated as it is more readable.
> 
>       - description: Google Villager
>         items:
>           - const: google,villager-rev0
>           - const: google,villager
>           - const: google,villager-rev0-sku0
>           - const: google,villager-sku0
>           - const: qcom,sc7280
> 

Is this a reply to something we discussed? There is no quote here, but I
remember pointing out some issues with one of Google patches recently.
Unfortunately my mailbox receives like 300 mails per day, so this does
not help...

Best regards,
Krzysztof

Re: [PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Doug Anderson]

Hi,

On Wed, Jul 6, 2022 at 3:04 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 06/07/2022 11:36, Jimmy Chen wrote:
> > To keep the consistency of the format for Chromebook items,
> > we basically add these items based on the rules discussed in the previous patch series here
> > https://lore.kernel.org/all/20220520143502.v4.1.I71e42c6174f1cec17da3024c9f73ba373263b9b6@changeid/.
> >
> > We are little configured with “one entry - one enum “. Do you mean something like below example?
> > We suggest keep items separated as it is more readable.
> >
> >       - description: Google Villager
> >         items:
> >           - const: google,villager-rev0
> >           - const: google,villager
> >           - const: google,villager-rev0-sku0
> >           - const: google,villager-sku0
> >           - const: qcom,sc7280
> >
>
> Is this a reply to something we discussed? There is no quote here, but I
> remember pointing out some issues with one of Google patches recently.
> Unfortunately my mailbox receives like 300 mails per day, so this does
> not help...

Jimmy did add a link to the series where the previous discussion was
in his reply, though he provided a link to patch #1 in the series and
not patch #4 (where the discussion was).

Relevant links:

* You saying you liked the enum [1].
* Me saying I liked them separate and that switching from a
"description" to a comment was opposite of what Stephen had previous
voted for [2], but could change if there was overwhelming need to make
them an enum.
* Rob saying he prefers an enum but lets sub-arch maintainers decide [3].
* Bjorn (the sub-arch maintainer) landing the patch without the enum [4].

[1] https://lore.kernel.org/r/a2bcac04-23ad-d1ae-84f1-924c4dbad42b@linaro.org/
[2] https://lore.kernel.org/r/CAD=FV=WgYbD9GN_wiR29ikZMzEjKUSZGH588+nnyd3O-dNgChQ@mail.gmail.com/
[3] https://lore.kernel.org/r/20220601232614.GA504337-robh@kernel.org/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=5069fe941f76c9f37abc98636a7db33a5ac72840

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

-Doug

Re: [PATCH 1/2] [V1,1/2] arm64: dts: qcom: Add LTE SKUs for sc7280-villager family

[Krzysztof Kozlowski]

On 07/07/2022 01:40, Doug Anderson wrote:
> Relevant links:
> 
> * You saying you liked the enum [1].
> * Me saying I liked them separate and that switching from a
> "description" to a comment was opposite of what Stephen had previous
> voted for [2], but could change if there was overwhelming need to make
> them an enum.
> * Rob saying he prefers an enum but lets sub-arch maintainers decide [3].
> * Bjorn (the sub-arch maintainer) landing the patch without the enum [4].

Ah, so the argument to my comment about using enum was that Bjorn
apparently prefers such way. OK.


Best regards,
Krzysztof