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. But I guess we could: compatible = "qcom,sc7180-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2"; BR, -R > > -Doug > > > > - description: Marvell SoCs implementing "arm,mmu-500" > > items: > > - const: marvell,ap806-smmu-500 > > -- > > 2.26.2 > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
