Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9CE7C6038E
Subject: Re: [Patch v15 4/5] dt-bindings: arm-smmu: Add bindings for
 qcom,smmu-v2
To:     Rob Herring <robh@kernel.org>
Cc:     Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org,
        alex.williamson@redhat.com,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Stephen Boyd <sboyd@kernel.org>,
        freedreno <freedreno@lists.freedesktop.org>,
        "open list:THERMAL" <linux-pm@vger.kernel.org>,
        Will Deacon <will.deacon@arm.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Linux IOMMU <iommu@lists.linux-foundation.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Andy Gross <andy.gross@linaro.org>,
        Robin Murphy <robin.murphy@arm.com>
References: <20180827105551.16346-1-vivek.gautam@codeaurora.org>
 <20180827105551.16346-5-vivek.gautam@codeaurora.org>
 <20180828203418.GA18366@bogus>
 <a1e31d1e-aaa4-5951-2261-3169033c636a@codeaurora.org>
 <CAFp+6iGHus-kZv+Zr5aiYRLct07NCFBL3k44bX=c4eW7J24kZg@mail.gmail.com>
 <CAL_JsqKGHC9wCy583CTvAz8-A_M6zyrAX69yN5RUjv7Kbs1wFQ@mail.gmail.com>
From:   Vivek Gautam <vivek.gautam@codeaurora.org>
Message-ID: <e5c72d2d-a1a1-40dd-103b-da7147b4e7ae@codeaurora.org>
Date:   Fri, 31 Aug 2018 15:21:49 +0530
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <CAL_JsqKGHC9wCy583CTvAz8-A_M6zyrAX69yN5RUjv7Kbs1wFQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Rob,


On 8/30/2018 6:13 AM, Rob Herring wrote:
> On Wed, Aug 29, 2018 at 6:23 AM Vivek Gautam
> <vivek.gautam@codeaurora.org> wrote:
>> On Wed, Aug 29, 2018 at 2:05 PM Vivek Gautam
>> <vivek.gautam@codeaurora.org> wrote:
>>> Hi Rob,
>>>
>>>
>>> On 8/29/2018 2:04 AM, Rob Herring wrote:
>>>> On Mon, Aug 27, 2018 at 04:25:50PM +0530, Vivek Gautam wrote:
>>>>> Add bindings doc for Qcom's smmu-v2 implementation.
>>>>>
>>>>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>>>>> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
>>>>> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>>> ---
>>>>>
>>>>> Changes since v14:
>>>>>    - This is a new patch added in v15 after noticing the new
>>>>>      checkpatch warning for separate dt-bindings doc.
>>>>>    - This patch also addresses comments given by Rob and Robin to add
>>>>>      a list of valid values of '<soc>' in "qcom,<soc>-smmu-v2"
>>>>>      compatible string.
>>>>>
>>>>>    .../devicetree/bindings/iommu/arm,smmu.txt         | 47 ++++++++++++++++++++++
>>>>>    1 file changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>>>> index 8a6ffce12af5..52198a539606 100644
>>>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>>>> @@ -17,10 +17,24 @@ conditions.
>>>>>                            "arm,mmu-401"
>>>>>                            "arm,mmu-500"
>>>>>                            "cavium,smmu-v2"
>>>>> +                        "qcom,<soc>-smmu-v2", "qcom,smmu-v2"
>>>> The v2 in the compatible string is kind of redundant unless the SoC has
>>>> other SMMU types.
>>> sdm845 has smmu-v2, and smmu-500 [1].
>>>
>>>>>                      depending on the particular implementation and/or the
>>>>>                      version of the architecture implemented.
>>>>>
>>>>> +                  A number of Qcom SoCs use qcom,smmu-v2 version of the IP.
>>>>> +                  "qcom,<soc>-smmu-v2" represents a soc specific compatible
>>>>> +                  string that should be present along with the "qcom,smmu-v2"
>>>>> +                  to facilitate SoC specific clocks/power connections and to
>>>>> +                  address specific bug fixes.
>>>>> +                  '<soc>' string in "qcom,<soc>-smmu-v2" should be one of the
>>>>> +                  following:
>>>>> +                  msm8996 - for msm8996 Qcom SoC.
>>>>> +                  sdm845 - for sdm845 Qcom Soc.
>>>> Rather than all this prose, it would be simpler to just add 2 lines with
>>>> the full compatibles rather than <soc>. The <soc> thing is not going to
>>>> work when/if we move bindings to json-schema also.
>>> then we keep adding
>>>            "qcom,msm8996-smmu-v2", "qcom,smmu-v2"
>>>            "qcom,msm8998-smmu-v2", "qcom,smmu-v2"
>>>            "qcom,sdm845-smmu-v2", "qcom,smmu-v2",
>>> and from [1]
>>>            "qcom,sdm845-smmu-500", "arm,mmu-500", etc.
>>> for each SoCs?
>> How about following diff on top of this patch?
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 52198a539606..5e6c04876533 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -17,23 +17,18 @@ conditions.
>>                           "arm,mmu-401"
>>                           "arm,mmu-500"
>>                           "cavium,smmu-v2"
>> -                        "qcom,<soc>-smmu-v2", "qcom,smmu-v2"
>> +                        "qcom,smmu-v2"
>>
>>                     depending on the particular implementation and/or the
>>                     version of the architecture implemented.
>>
>> -                  A number of Qcom SoCs use qcom,smmu-v2 version of the IP.
>> -                  "qcom,<soc>-smmu-v2" represents a soc specific compatible
>> -                  string that should be present along with the "qcom,smmu-v2"
>> -                  to facilitate SoC specific clocks/power connections and to
>> -                  address specific bug fixes.
>> -                  '<soc>' string in "qcom,<soc>-smmu-v2" should be one of the
>> -                  following:
>> -                  msm8996 - for msm8996 Qcom SoC.
>> -                  sdm845 - for sdm845 Qcom Soc.
>> -
>> -                  An example string would be -
>> -                  "qcom,msm8996-smmu-v2", "qcom,smmu-v2".
>> +                  Qcom SoCs using qcom,smmu-v2 must have soc specific
>> +                  compatible string attached to "qcom,smmu-v2" to take care
>> +                  of SoC specific clocks/power connections and to address
>> +                  specific bug fixes.
>> +                  Precisely, it should be one of the following:
>> +                  "qcom,msm8996-smmu-v2", "qcom,smmu-v2",
>> +                  "qcom,sdm845-smmu-v2", "qcom,smmu-v2".
> We don't need an explanation of why we need specific compatibles in
> each binding document (though maybe we need a better explanation
> somewhere). We just need to know what are valid values for compatibles
> and this includes any combinations. Generally, this is just a list of
> combinations.
[snip]

Fixed this in v16. Thanks.

Best regards
Vivek