| Message ID | 20221021165534.2334329-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | iommy/arm-smmu-qcom: Rework Qualcomm SMMU bindings and implementation | expand |
On Fri, 21 Oct 2022 19:55:27 +0300, Dmitry Baryshkov wrote: > Rework clocks/clock-names properties schema to property describe > possible usage cases. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../devicetree/bindings/iommu/arm,smmu.yaml | 129 ++++++++++++++++-- > 1 file changed, 121 insertions(+), 8 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iommu/arm,smmu.example.dtb: iommu@d00000: 'anyOf' conditional failed, one must be fixed: ['bus', 'iface'] is too long ['bus', 'iface'] is too short 'iface' was expected 'iface-mm' was expected 'mem' was expected 'iface-smmu' was expected [[4294967295, 123], [4294967295, 124]] is too long [[4294967295, 123], [4294967295, 124]] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iommu/arm,smmu.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 21/10/2022 12:55, Dmitry Baryshkov wrote: > Add missing compatibles used for Adreno SMMU on sc7280 and sm8450 > platforms and for the Qualcomm v2 SMMU used on SDM630 platform. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index 9066e6df1ba1..34ee33a62ba5 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -28,6 +28,7 @@ properties: > - enum: > - qcom,msm8996-smmu-v2 > - qcom,msm8998-smmu-v2 > + - qcom,sdm630-smmu-v2 So qcom,adreno-smmu is not compatible with Adreno? See below. > - const: qcom,smmu-v2 > > - description: Qcom SoCs implementing "arm,mmu-500" > @@ -48,10 +49,20 @@ properties: > - qcom,sm8350-smmu-500 > - qcom,sm8450-smmu-500 > - const: arm,mmu-500 > + > + - description: Qcom Adreno GPUs implementing "arm,smmu-500" > + items: > + - enum: > + - qcom,sc7280-smmu-500 > + - qcom,sm8250-smmu-500 > + - const: qcom,adreno-smmu > + - const: arm,mmu-500 > - description: Qcom Adreno GPUs implementing "arm,smmu-v2" > items: > - enum: > + - qcom,msm8996-smmu-v2 > - qcom,sc7180-smmu-v2 > + - qcom,sdm630-smmu-v2 This does not look correct. The same compatible should not be present in two different setups. If qcom,msm8996-smmu-v2 is compatible with qcom,adreno-smmu, then your first hunk is not correct. > - qcom,sdm845-smmu-v2 > - const: qcom,adreno-smmu > - const: qcom,smmu-v2 Best regards, Krzysztof
On 22/10/2022 03:59, Krzysztof Kozlowski wrote: > On 21/10/2022 12:55, Dmitry Baryshkov wrote: >> Add missing compatibles used for Adreno SMMU on sc7280 and sm8450 >> platforms and for the Qualcomm v2 SMMU used on SDM630 platform. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> index 9066e6df1ba1..34ee33a62ba5 100644 >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> @@ -28,6 +28,7 @@ properties: >> - enum: >> - qcom,msm8996-smmu-v2 >> - qcom,msm8998-smmu-v2 >> + - qcom,sdm630-smmu-v2 > > So qcom,adreno-smmu is not compatible with Adreno? See below. > >> - const: qcom,smmu-v2 >> >> - description: Qcom SoCs implementing "arm,mmu-500" >> @@ -48,10 +49,20 @@ properties: >> - qcom,sm8350-smmu-500 >> - qcom,sm8450-smmu-500 >> - const: arm,mmu-500 >> + >> + - description: Qcom Adreno GPUs implementing "arm,smmu-500" >> + items: >> + - enum: >> + - qcom,sc7280-smmu-500 >> + - qcom,sm8250-smmu-500 >> + - const: qcom,adreno-smmu >> + - const: arm,mmu-500 >> - description: Qcom Adreno GPUs implementing "arm,smmu-v2" >> items: >> - enum: >> + - qcom,msm8996-smmu-v2 >> - qcom,sc7180-smmu-v2 >> + - qcom,sdm630-smmu-v2 > > This does not look correct. The same compatible should not be present in > two different setups. > > If qcom,msm8996-smmu-v2 is compatible with qcom,adreno-smmu, then your > first hunk is not correct. Currently the qcom,adreno-smmu compat string is used as a flag, telling the kernel that this SMMU instance needs some special setup to work with Adreno GPU driver For example, we have the following compat lists in the existing DT files: - "qcom,msm8996-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" - "qcom,msm8996-smmu-v2", "qcom,smmu-v2" // not handled by arm-qcom-smmu - "qcom,sdm630-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" - "qcom,sdm630-smmu-v2", "qcom,smmu-v2" - "qcom,sdm845-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" - "qcom,sdm845-smmu-500", "arm,mmu-500" - "qcom,sdm845-smmu-v2", "qcom,smmu-v2" // special setup used on Cheza - "qcom,sm8250-smmu-500", "qcom,adreno-smmu", "arm,mmu-500" - "qcom,sm8250-smmu-500", "arm,mmu-500" As we are trying to refactor the IOMMU bindings, what would be your recommendation? To introduce minimal changes, I wanted to have the following lists: - "qcom,SOC-smmu-500", "qcom,adreno-smmu", "qcom,smmu-500", "arm,mmu-500" - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" However maybe you would prefer the following model: - "qcom,SOC-adreno-smmu-500", "qcom,adreno-smmu-500", "arm,mmu-500" - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" Or: - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" + 'qcom,adreno-smmu' flag/property? > >> - qcom,sdm845-smmu-v2 >> - const: qcom,adreno-smmu >> - const: qcom,smmu-v2 > > Best regards, > Krzysztof >
On 22/10/2022 05:17, Dmitry Baryshkov wrote: > On 22/10/2022 03:59, Krzysztof Kozlowski wrote: >> On 21/10/2022 12:55, Dmitry Baryshkov wrote: >>> Add missing compatibles used for Adreno SMMU on sc7280 and sm8450 >>> platforms and for the Qualcomm v2 SMMU used on SDM630 platform. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >>> index 9066e6df1ba1..34ee33a62ba5 100644 >>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >>> @@ -28,6 +28,7 @@ properties: >>> - enum: >>> - qcom,msm8996-smmu-v2 >>> - qcom,msm8998-smmu-v2 >>> + - qcom,sdm630-smmu-v2 >> >> So qcom,adreno-smmu is not compatible with Adreno? See below. >> >>> - const: qcom,smmu-v2 >>> >>> - description: Qcom SoCs implementing "arm,mmu-500" >>> @@ -48,10 +49,20 @@ properties: >>> - qcom,sm8350-smmu-500 >>> - qcom,sm8450-smmu-500 >>> - const: arm,mmu-500 >>> + >>> + - description: Qcom Adreno GPUs implementing "arm,smmu-500" >>> + items: >>> + - enum: >>> + - qcom,sc7280-smmu-500 >>> + - qcom,sm8250-smmu-500 >>> + - const: qcom,adreno-smmu >>> + - const: arm,mmu-500 >>> - description: Qcom Adreno GPUs implementing "arm,smmu-v2" >>> items: >>> - enum: >>> + - qcom,msm8996-smmu-v2 >>> - qcom,sc7180-smmu-v2 >>> + - qcom,sdm630-smmu-v2 >> >> This does not look correct. The same compatible should not be present in >> two different setups. >> >> If qcom,msm8996-smmu-v2 is compatible with qcom,adreno-smmu, then your >> first hunk is not correct. > > Currently the qcom,adreno-smmu compat string is used as a flag, telling > the kernel that this SMMU instance needs some special setup to work with > Adreno GPU driver Indeed, I see the usage in DTS, > > For example, we have the following compat lists in the existing DT files: > - "qcom,msm8996-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" > - "qcom,msm8996-smmu-v2", "qcom,smmu-v2" // not handled by arm-qcom-smmu > > - "qcom,sdm630-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" > - "qcom,sdm630-smmu-v2", "qcom,smmu-v2" > > - "qcom,sdm845-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2" > - "qcom,sdm845-smmu-500", "arm,mmu-500" > - "qcom,sdm845-smmu-v2", "qcom,smmu-v2" // special setup used on Cheza > > - "qcom,sm8250-smmu-500", "qcom,adreno-smmu", "arm,mmu-500" > - "qcom,sm8250-smmu-500", "arm,mmu-500" > > > As we are trying to refactor the IOMMU bindings, what would be your > recommendation? > > To introduce minimal changes, I wanted to have the following lists: > - "qcom,SOC-smmu-500", "qcom,adreno-smmu", "qcom,smmu-500", "arm,mmu-500" > > - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" > > However maybe you would prefer the following model: > > - "qcom,SOC-adreno-smmu-500", "qcom,adreno-smmu-500", "arm,mmu-500" > - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" If we started from scratch, I would prefer this one, however as DTSes are already using your previous method, It's fine. It's a bit confusing to have most specific compatible followed by different fallbacks, but we already have few cases for this (e.g. Renesas boards), so I guess it is fine here as well. At the end entire compatible list uniquely describes the hardware. > > > Or: > - "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" + > 'qcom,adreno-smmu' flag/property? > > >> >>> - qcom,sdm845-smmu-v2 >>> - const: qcom,adreno-smmu >>> - const: qcom,smmu-v2 >> >> Best regards, >> Krzysztof >> > Best regards, Krzysztof
On 21/10/2022 12:55, Dmitry Baryshkov wrote: > Add missing compatibles used for Adreno SMMU on sc7280 and sm8450 > platforms and for the Qualcomm v2 SMMU used on SDM630 platform. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 21/10/2022 12:55, Dmitry Baryshkov wrote: > Add generic bindings for the Qualcomm variant of the ARM MMU-500. It is > expected that all future platforms will use the generic qcom,smmu-500 > compat string in addition to SoC-specific and the generic arm,mmu-500 > ones. Older bindings are now described as deprecated. > > Note: I have split the sdx55 and sdx65 from the legacy bindings. They > are not supported by the qcom SMMU implementation. I can suppose that > they are using the generic implementation rather than the > Qualcomm-speicific one. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
The main goal of this patchset is to define a generic qcom,smmu-500 binding to be used by newer Qualcomm platforms instead of defining each and every SoC line with no actual differences between the compats. While preparing this change it was required to cleanup the existing bindings and to rework the way the arm-smmu-qcom implementation handles binding to IOMMU devices. Dmitry Baryshkov (9): dt-bindings: arm-smmu: Add missing Qualcomm SMMU compatibles dt-bindings: arm-smmu: fix clocks/clock-names schema dt-bindings: arm-smmu: Add generic qcom,smmu-500 bindings iommu/arm-smmu-qcom: Move implementation data into match data iommu/arm-smmu-qcom: Move the qcom,adreno-smmu check into qcom_smmu_create iommu/arm-smmu-qcom: provide separate implementation for SDM845-smmu-500 iommu/arm-smmu-qcom: Merge table from arm-smmu-qcom-debug into match data iommu/arm-smmu-qcom: Stop using mmu500 reset for v2 MMUs iommu/arm-smmu-qcom: Add generic qcom,smmu-500 match entry .../devicetree/bindings/iommu/arm,smmu.yaml | 168 +++++++++++++++++- .../iommu/arm/arm-smmu/arm-smmu-qcom-debug.c | 91 ---------- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 156 +++++++++++----- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 21 ++- 4 files changed, 288 insertions(+), 148 deletions(-)