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

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

 





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@xxxxxxxxxxxxxxxx>
---
  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



[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