Re: [Patch v15 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2

[Vivek Gautam]

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@xxxxxxxxxxxxxx> wrote:
> > On Wed, Aug 29, 2018 at 2:05 PM Vivek Gautam
> > <vivek.gautam@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > > > > Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> > > > > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > > > > ---
> > > > >
> > > > > 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