Messages in this thread | | | From | Doug Anderson <> | Date | Wed, 19 Aug 2020 10:02:20 -0700 | Subject | Re: [PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU |
| |
Hi,
On Mon, Aug 17, 2020 at 3:03 PM Rob Clark <robdclark@gmail.com> wrote: > > From: Jordan Crouse <jcrouse@codeaurora.org> > > 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@codeaurora.org> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > 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@gmail.com
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...
-Doug
> - description: Marvell SoCs implementing "arm,mmu-500" > items: > - const: marvell,ap806-smmu-500 > -- > 2.26.2 >
| |