lkml.org 
[lkml]   [2023]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 2/3] dt-bindings: arm: qcom: Add MSM8926 and Samsung Galaxy Tab 4 10.1 LTE
    From


    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

    \
     
     \ /
      Last update: 2023-03-26 23:52    [W:6.133 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site