Hi, On Wed, Aug 19, 2020 at 10:36 AM Rob Clark <robdclark@xxxxxxxxx> wrote: > > On Wed, Aug 19, 2020 at 10:03 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > > > Hi, > > > > On Mon, Aug 17, 2020 at 3:03 PM Rob Clark <robdclark@xxxxxxxxx> wrote: > > > > > > From: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> > > > > > > Every Qcom Adreno GPU has an embedded SMMU for its own use. These > > > devices depend on unique features such as split pagetables, > > > different stall/halt requirements and other settings. Identify them > > > with a compatible string so that they can be identified in the > > > arm-smmu implementation specific code. > > > > > > Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> > > > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> > > > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx> > > > --- > > > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > index 503160a7b9a0..5ec5d0d691f6 100644 > > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > > > @@ -40,6 +40,10 @@ properties: > > > - qcom,sm8150-smmu-500 > > > - qcom,sm8250-smmu-500 > > > - const: arm,mmu-500 > > > + - description: Qcom Adreno GPUs implementing "arm,smmu-v2" > > > + items: > > > + - const: qcom,adreno-smmu > > > + - const: qcom,smmu-v2 > > > > I know I'm kinda late to the game, but this seems weird to me, > > especially given the later patches in the series like: > > > > https://lore.kernel.org/r/20200817220238.603465-19-robdclark@xxxxxxxxx > > > > Specifically in that patch you can see that this IOMMU already had a > > compatible string and we're changing it and throwing away the > > model-specific string? I'm guessing that you're just trying to make > > it easier for code to identify the adreno iommu, but it seems like a > > better way would have been to just add the adreno compatible in the > > middle, like: > > > > - description: Qcom Adreno GPUs implementing "arm,smmu-v2" > > items: > > - enum: > > - qcom,msm8996-smmu-v2 > > - qcom,msm8998-smmu-v2 > > - qcom,sc7180-smmu-v2 > > - qcom,sdm845-smmu-v2 > > - const: qcom,adreno-smmu > > - const: qcom,smmu-v2 > > > > Then we still have the SoC-specific compatible string in case we need > > it but we also have the generic one? It also means that we're not > > deleting the old compatible string... > > I did bring up the thing about removing the compat string in an > earlier revision of the series.. but then we realized that > qcom,sc7180-smmu-v2 was never actually used anywhere. Right, so at least there's not going to be weird issues where landing the dts before the code change will break anything. > But I guess we could: compatible = "qcom,sc7180-smmu-v2", > "qcom,adreno-smmu", "qcom,smmu-v2"; Yeah, that was what I was suggesting. -Doug
