[PATCH] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[PATCH] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Yunlong Jia]

The difference between sku6 and sku4 is that there is no esim

Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.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 4dd18fbf20b6..aebeefdab27f 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -410,6 +410,7 @@ properties:
       - description: Google Pazquel with LTE and Parade (newest rev)
         items:
           - const: google,pazquel-sku4
+          - const: google,pazquel-sku6
           - const: qcom,sc7180
 
       - description: Google Pazquel with LTE and TI (newest rev)
-- 
2.17.1

Re: [PATCH] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 20/07/2022 09:38, Yunlong Jia wrote:
> The difference between sku6 and sku4 is that there is no esim
> 
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.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 4dd18fbf20b6..aebeefdab27f 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -410,6 +410,7 @@ properties:
>        - description: Google Pazquel with LTE and Parade (newest rev)
>          items:
>            - const: google,pazquel-sku4
> +          - const: google,pazquel-sku6

This looks wrong, did you test it?

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Wed, Jul 20, 2022 at 12:46 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/07/2022 09:38, Yunlong Jia wrote:
> > The difference between sku6 and sku4 is that there is no esim
> >
> > Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.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 4dd18fbf20b6..aebeefdab27f 100644
> > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > @@ -410,6 +410,7 @@ properties:
> >        - description: Google Pazquel with LTE and Parade (newest rev)
> >          items:
> >            - const: google,pazquel-sku4
> > +          - const: google,pazquel-sku6
>
> This looks wrong, did you test it?

Why do you think it's wrong? This patch is adding a second compatible
string to an existing dts. The only difference between SKU4 and SKU6
is that one of them has the eSIM component stuffed and the other one
doesn't. This need not be represented in the dts since the eSIM is
automatically detected, but it was still stuffed as a SKU strapping so
the factory could tell whether the missing eSIM was an error or
intentional.

This is just like the SKU0 vs. SKU2 difference.

Other than the fact that this should be together in one series with
the dts patch:

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

Re: [PATCH] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 20/07/2022 17:07, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jul 20, 2022 at 12:46 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/07/2022 09:38, Yunlong Jia wrote:
>>> The difference between sku6 and sku4 is that there is no esim
>>>
>>> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.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 4dd18fbf20b6..aebeefdab27f 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> @@ -410,6 +410,7 @@ properties:
>>>        - description: Google Pazquel with LTE and Parade (newest rev)
>>>          items:
>>>            - const: google,pazquel-sku4
>>> +          - const: google,pazquel-sku6
>>
>> This looks wrong, did you test it?
> 
> Why do you think it's wrong? This patch is adding a second compatible
> string to an existing dts. 

Because it was sent after a patch adding sku6+sku4+sc7180, but the order
here is different.

However for some reason it was not part of a patchset which makes it
total mess and even less possible to understand.

> The only difference between SKU4 and SKU6
> is that one of them has the eSIM component stuffed and the other one
> doesn't. This need not be represented in the dts since the eSIM is
> automatically detected, but it was still stuffed as a SKU strapping so
> the factory could tell whether the missing eSIM was an error or
> intentional.
> 
> This is just like the SKU0 vs. SKU2 difference.
> 
> Other than the fact that this should be together in one series with
> the dts patch:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

I don't agree. With last DTS patch this is a wrong binding (or wrong
DTS) therefore I must NAK it.

Please do not upstream stuff in some different way, like sending
bindings and DTS separate.

Best regards,
Krzysztof