[2/3] dt-bindings: arm: qcom: Add MSM8926 and Samsung Galaxy Tab 4 10.1 LTE

  MSM8926 (also known as Snapdragon 400) is very similar to MSM8226 and
APQ8026 with the primary difference being that it features an LTE modem
unlike the former two which feature a 3G modem and a GPS-only modem,
respectively.

This also documents Samsung Galaxy Tab 4 10.1 LTE (samsung,matisselte)
which is a tablet by Samsung based on the MSM8926 SoC.

Signed-off-by: Stefan Hansson <newbyte@postmarketos.org>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
  

On 22/01/2023 15:47, Stefan Hansson wrote:
> MSM8926 (also known as Snapdragon 400) is very similar to MSM8226 and
> APQ8026 with the primary difference being that it features an LTE modem
> unlike the former two which feature a 3G modem and a GPS-only modem,
> respectively.
> 
> This also documents Samsung Galaxy Tab 4 10.1 LTE (samsung,matisselte)
> which is a tablet by Samsung based on the MSM8926 SoC.
> 
> Signed-off-by: Stefan Hansson <newbyte@postmarketos.org>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 47913a8e3eea..7a0b2088ead9 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -35,6 +35,7 @@ description: |
>          mdm9615
>          msm8226
>          msm8916
> +        msm8926
>          msm8953
>          msm8956
>          msm8974
> @@ -219,6 +220,11 @@ properties:
>            - const: qcom,msm8916-v1-qrd/9-v1
>            - const: qcom,msm8916
>  
> +      - items:
> +          - enum:
> +              - samsung,matisselte

1. matisse is the code name, lte is version/suffix. I don't think they
should be together, because then it looks like "matisselte" is a name.
It actually sounds like one word.

2. You base on other SoC but you do not include its compatibles. Why? Is
it intended? None of the properties applicable to other SoC will match
here, thus I actually wonder if you run dtbs_check...

Best regards,
Krzysztof
  

On 23/01/2023 18:10, Krzysztof Kozlowski wrote:
> On 22/01/2023 15:47, Stefan Hansson wrote:
>> MSM8926 (also known as Snapdragon 400) is very similar to MSM8226 and
>> APQ8026 with the primary difference being that it features an LTE modem
>> unlike the former two which feature a 3G modem and a GPS-only modem,
>> respectively.
>>
>> This also documents Samsung Galaxy Tab 4 10.1 LTE (samsung,matisselte)
>> which is a tablet by Samsung based on the MSM8926 SoC.
>>
>> Signed-off-by: Stefan Hansson <newbyte@postmarketos.org>
>> ---
>>  Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>> index 47913a8e3eea..7a0b2088ead9 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>> @@ -35,6 +35,7 @@ description: |
>>          mdm9615
>>          msm8226
>>          msm8916
>> +        msm8926
>>          msm8953
>>          msm8956
>>          msm8974
>> @@ -219,6 +220,11 @@ properties:
>>            - const: qcom,msm8916-v1-qrd/9-v1
>>            - const: qcom,msm8916
>>  
>> +      - items:
>> +          - enum:
>> +              - samsung,matisselte
> 
> 1. matisse is the code name, lte is version/suffix. I don't think they
> should be together, because then it looks like "matisselte" is a name.
> It actually sounds like one word.

Update: there is already matisse-wifi, so please follow the same naming
convention. Version suffix should be separated with hyphen.

> 
> 2. You base on other SoC but you do not include its compatibles. Why? Is
> it intended? None of the properties applicable to other SoC will match
> here, thus I actually wonder if you run dtbs_check...

Best regards,
Krzysztof
  

On 2023-01-23 18:11, Krzysztof Kozlowski wrote:
> On 23/01/2023 18:10, Krzysztof Kozlowski wrote:
>> On 22/01/2023 15:47, Stefan Hansson wrote:
>>> MSM8926 (also known as Snapdragon 400) is very similar to MSM8226 and
>>> APQ8026 with the primary difference being that it features an LTE modem
>>> unlike the former two which feature a 3G modem and a GPS-only modem,
>>> respectively.
>>>
>>> This also documents Samsung Galaxy Tab 4 10.1 LTE (samsung,matisselte)
>>> which is a tablet by Samsung based on the MSM8926 SoC.
>>>
>>> Signed-off-by: Stefan Hansson <newbyte@postmarketos.org>
>>> ---
>>>   Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> index 47913a8e3eea..7a0b2088ead9 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> @@ -35,6 +35,7 @@ description: |
>>>           mdm9615
>>>           msm8226
>>>           msm8916
>>> +        msm8926
>>>           msm8953
>>>           msm8956
>>>           msm8974
>>> @@ -219,6 +220,11 @@ properties:
>>>             - const: qcom,msm8916-v1-qrd/9-v1
>>>             - const: qcom,msm8916
>>>   
>>> +      - items:
>>> +          - enum:
>>> +              - samsung,matisselte
>>
>> 1. matisse is the code name, lte is version/suffix. I don't think they
>> should be together, because then it looks like "matisselte" is a name.
>> It actually sounds like one word.
> 
> Update: there is already matisse-wifi, so please follow the same naming
> convention. Version suffix should be separated with hyphen.
> 

I'm aware, and I've been in contact with the matisse-wifi dts author who 
told me that he went with this name because you suggested it (he had 
originally sent it in as matissewifi). However I don't think diverging 
from how the rest of the community refers to it is a good idea. 
Codenames often sound nonsensical, but they have effectively become the 
de-facto universal identifier for devices in the community and so I 
think retaining that consistency is more beneficial than making it sound 
nice.

Additionally, while matisse-wifi has the hyphen added before the suffix, 
many other Samsung devices do not (klte, jackpotlte, s3ve3g). As such, I 
think the name matisse-wifi is the outlier here rather than matisselte 
(but yes, I do understand that they are more related to each other than 
the other devices mentioned).

Does that sound sensible?

>>
>> 2. You base on other SoC but you do not include its compatibles. Why? Is
>> it intended? None of the properties applicable to other SoC will match
>> here, thus I actually wonder if you run dtbs_check...

Sorry, I forgot about running dtbs_check. However, I'm not sure I 
understand the question. What do you mean by that I don't include its 
compatibles?

I ran `$ make dtbs_check DT_SCHEMA_FILES=qcom.yaml` locally just now, 
and it only gave me errors from the qcom-msm8974pro-oneplus-bacon dtb. 
Maybe I'm running it wrong?

> 
> Best regards,
> Krzysztof
> 

Thanks for the review,
Stefan Hansson
  

On 23/01/2023 18:41, Stefan Hansson wrote:
>>>
>>> 2. You base on other SoC but you do not include its compatibles. Why? Is
>>> it intended? None of the properties applicable to other SoC will match
>>> here, thus I actually wonder if you run dtbs_check...
> 
> Sorry, I forgot about running dtbs_check. However, I'm not sure I 
> understand the question. What do you mean by that I don't include its 
> compatibles?

I understood you include the msm8226.dtsi which is a different SoC. If
you include it, you get all of its content. We do it only for compatible
devices, but your device does not indicate compatibility with msm8226.

> 
> I ran `$ make dtbs_check DT_SCHEMA_FILES=qcom.yaml` locally just now, 
> and it only gave me errors from the qcom-msm8974pro-oneplus-bacon dtb. 
> Maybe I'm running it wrong?

No clue, I cannot test because your patches do not apply cleanly.

Best regards,
Krzysztof
  

On Montag, 23. Jänner 2023 19:08:03 CET Krzysztof Kozlowski wrote:
> On 23/01/2023 18:41, Stefan Hansson wrote:
> >>> 2. You base on other SoC but you do not include its compatibles. Why? Is
> >>> it intended? None of the properties applicable to other SoC will match
> >>> here, thus I actually wonder if you run dtbs_check...
> > 
> > Sorry, I forgot about running dtbs_check. However, I'm not sure I
> > understand the question. What do you mean by that I don't include its
> > compatibles?
> 
> I understood you include the msm8226.dtsi which is a different SoC. If
> you include it, you get all of its content. We do it only for compatible
> devices, but your device does not indicate compatibility with msm8226.

Hi Krzysztof,

the way the earlier Qualcomm SoCs work, especially regarding naming scheme is 
the following.

There's for example the msm8x74 family which includes msm8974, msm8674, 
msm8274, and the a bit differently named apq8074 where the significant 
different are the RF capabilities, I think with those only 8974 had LTE, 8674 
and 8274 only 3G but different band support, and the apq8074 has no mobile 
radio.

The same exists for sure also for 8x16 and 8x26, probably a bunch of other 
SoCs as well.

So from software side (apart from modem firmware of course) it can be treated 
in practise as the same SoC so that's why we included the dtsi in this case in 
msm8226 but also msm8926 and apq8026.

But the compatible on board-level is in practise (to my knowledge) not really 
used for anything important other than having a nice string in the dts file. I 
know some software uses compatible from user space but there for 
differentiating between different devices and ignoring the SoC compatibles.

But while they are software-compatible for the most part, they *are* distinct 
SoCs with different capabilities and I just don't see the point in trying to 
establish some kinds of relationships between different SoCs that are somewhat 
or very similar (msm8226 and msm8974 also share many components but are 
obviously different SoCs).

And also e.g. (nearly) all apq* dts files we already have in mainline only 
have apq compatible and not the corresponding msm* compatible. And I think 
that's totally legitimate.

Regards
Luca

> 
> > I ran `$ make dtbs_check DT_SCHEMA_FILES=qcom.yaml` locally just now,
> > and it only gave me errors from the qcom-msm8974pro-oneplus-bacon dtb.
> > Maybe I'm running it wrong?
> 
> No clue, I cannot test because your patches do not apply cleanly.
> 
> Best regards,
> Krzysztof
  

On 28/01/2023 22:43, Luca Weiss wrote:
> On Montag, 23. Jänner 2023 19:08:03 CET Krzysztof Kozlowski wrote:
>> On 23/01/2023 18:41, Stefan Hansson wrote:
>>>>> 2. You base on other SoC but you do not include its compatibles. Why? Is
>>>>> it intended? None of the properties applicable to other SoC will match
>>>>> here, thus I actually wonder if you run dtbs_check...
>>>
>>> Sorry, I forgot about running dtbs_check. However, I'm not sure I
>>> understand the question. What do you mean by that I don't include its
>>> compatibles?
>>
>> I understood you include the msm8226.dtsi which is a different SoC. If
>> you include it, you get all of its content. We do it only for compatible
>> devices, but your device does not indicate compatibility with msm8226.
> 
> Hi Krzysztof,
> 
> the way the earlier Qualcomm SoCs work, especially regarding naming scheme is 
> the following.
> 
> There's for example the msm8x74 family which includes msm8974, msm8674, 
> msm8274, and the a bit differently named apq8074 where the significant 
> different are the RF capabilities, I think with those only 8974 had LTE, 8674 
> and 8274 only 3G but different band support, and the apq8074 has no mobile 
> radio.
> 
> The same exists for sure also for 8x16 and 8x26, probably a bunch of other 
> SoCs as well.
> 
> So from software side (apart from modem firmware of course) it can be treated 
> in practise as the same SoC so that's why we included the dtsi in this case in 
> msm8226 but also msm8926 and apq8026.

First, there is distinction between SoC having and not having modem. I
guess small enough that we just include the DTSI.

Second, there is distinction between different families, even if they
share a lot. All of the SoCs here share something, because Qualcomm has
versionable IP blocks which they re-use.

> 
> But the compatible on board-level is in practise (to my knowledge) not really 
> used for anything important other than having a nice string in the dts file. I 
> know some software uses compatible from user space but there for 
> differentiating between different devices and ignoring the SoC compatibles.

It's not only about board, but about all devices in the SoC.

> 
> But while they are software-compatible for the most part, they *are* distinct 
> SoCs with different capabilities and I just don't see the point in trying to 
> establish some kinds of relationships between different SoCs that are somewhat 
> or very similar (msm8226 and msm8974 also share many components but are 
> obviously different SoCs).

You don't have to create such relationships. You don't have to include
other DTSI, either. What yo have to is - quoting Linux docs:
"DO make 'compatible' properties specific. DON'T use wildcards in
compatible strings. DO use fallback compatibles when devices are the
same as or a subset of prior implementations. DO add new compatibles in
case there are new features or bugs."

> 
> And also e.g. (nearly) all apq* dts files we already have in mainline only 
> have apq compatible and not the corresponding msm* compatible. And I think 
> that's totally legitimate.

We do not talk here about apq, actually, at all. We talk about one
msm8xxx including other msm8xxx...


Best regards,
Krzysztof