Skip to contentSkip to conversation

dt-bindings: mmc: brcm,iproc-sdhci: allow dma-coherent property

PATCH NAK Lee Yongjun 5 participants 8 messages Mar 5 – Mar 6

Subsystems (3)
Device tree bindings MMC, SD and SDIO Broadcom iproc Arm architecture
lore
Share & tools
lore

Lee Yongjun

author
#0 lore raw

PATCH 1/1 dt-bindings: mmc: brcm,iproc-sdhci: allow dma-coherent property

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

Signed-off-by Lee Yongjun <[email protected]>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +2 -0
index 2f63f2cdeb71..579e44843404 100644
@@ -38,6 +38,8 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+  dma-coherent: true
+
 required:
   - compatible
   - reg
Signature · 1 line
2.34.1
#1 lore raw
Quoted 25 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

Signed-off-by Lee Yongjun <[email protected]>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +0 -0
index 2f63f2cdeb71..579e44843404 100644

Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml properties: · line 43

Quoted 6 lines
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +2 -0
@@ -38,6 +38,8 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+  dma-coherent: true
+

Yeah for Broadcom iProc SDHCI on ARM64 platforms (including NS2), cache coherent DMA is supported (as opposed to other 32-bit based ARM platforms).

Change looks good to me. Thanks!

Quoted 7 lines
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +0 -0
 required:
   - compatible

- reg
--
2.34.1

#2 lore raw
Quoted 30 lines · Ray Jui
Quoted 28 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

Signed-off-by Lee Yongjun <[email protected]>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +0 -0
index 2f63f2cdeb71..579e44843404 100644
@@ -38,6 +38,8 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
Quoted 8 lines

Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml properties:

Quoted 3 lines
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +2 -0
+  dma-coherent: true
+

Yeah for Broadcom iProc SDHCI on ARM64 platforms (including NS2), cache coherent DMA is supported (as opposed to other 32-bit based ARM platforms).

Change looks good to me. Thanks!

Just open the file before giving blanket reviews or how are you exactly reviewing without knowing what is there? This was already fixed and this patch is obviously wrong.

Best regards,
Krzysztof

Reviewed-by on v1 1/1
#3 lore raw
In reply to Lee Yongjun
Quoted 11 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Quoted 4 lines

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

Signed-off-by Lee Yongjun <[email protected]>
Reviewed-by Florian Fainelli <[email protected]>
Signature · 1 line
Florian
maintainer
#4 lore raw
In reply to Lee Yongjun
Quoted 11 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Quoted 8 lines

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

Signed-off-by Lee Yongjun <[email protected]>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 2 ++
 1 file changed, 2 insertions(+)

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml:46:3: [error] duplication of key "dma-coherent" in mapping (key-duplicates)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml: ignoring, error parsing file ./Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml:46:3: found duplicate key "dma-coherent" with value "True" (original value: "True") make[2]: *** Deleting file 'Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dts' Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml:46:3: found duplicate key "dma-coherent" with value "True" (original value: "True") make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1605: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

#5 lore raw
In reply to Lee Yongjun
Quoted 6 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

Quoted 7 lines

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

So is the hardware capable of coherent DMA or not? Entire commit msg is silent about this and says how you fix warning without actually telling us that it is a correct fix.

Best regards,
Krzysztof

NAK
#6 lore raw
Quoted 12 lines · Krzysztof Kozlowski
Quoted 11 lines · Lee Yongjun

The Broadcom iProc SDHCI controller supports DMA, but its binding schema does not allow the 'dma-coherent' property.

As a result, dtbs_check reports the following validation errors on the Northstar2 SoC:

mmc@66420000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected) mmc@66430000 (brcm,sdhci-iproc-cygnus): Unevaluated properties are not allowed ('dma-coherent' was unexpected)

Quoted 7 lines
Quoted 2 lines

Allow the 'dma-coherent' property in the schema to fix the validation errors and accurately reflect the hardware capability.

So is the hardware capable of coherent DMA or not? Entire commit msg is silent about this and says how you fix warning without actually telling us that it is a correct fix.

Heh, why am I reviewing it, it's obviously non-tested. Code is already there!

NAK

Best regards,
Krzysztof

author
#7 lore raw
In reply to Lee Yongjun

Dear Krzysztof, Rob, and Ray,

Thank you for the feedback. I've checked the linux-next tree and confirmed that Rob Herring's patch (commit acb52756e90c) already addresses this issue.

I apologize for the redundant submission. Please disregard this patch.

Best regards,
Lee Yongjun