Skip to contentSkip to conversation

arm: socfpga: use clk-phase-sd-hs

PATCH Superseded Dinh Nguyen 3 participants 12 messages Oct 19, 2022 – Oct 21, 2022

Subsystems (6)

v5 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Dinh Nguyen

author maintainer
#0 lore raw

v5 cover arm: socfpga: use clk-phase-sd-hs

Hi,

Just wanted to address the comments regarding the dt-bindings document of "altr,sysmgr-syscon". I ran the 'make dt_binding_check'
after: pip3 install dtschema --upgrade and I checked to make sure I have yamllint installed and I still don't see any warnings. I'm also confused about whether "altr,socfpga-dw-mshc" should be a const. I see the same usage in:

Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml

allOf:
- $ref: "mmc-controller.yaml#"
- if:

      properties:
        compatible:
          contains:
            const: arasan,sdhci-5.1

Please advise on how to address this comment!

Thanks,
Dinh

Dinh Nguyen (6):
dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"
arm64: dts: socfpga: Add clk-phase-sd-hs property to the sdmmc node
arm: dts: socfpga: Add clk-phase-sd-hs property to the sdmmc node
mmc: dw_mmc-pltfm: socfpga: add method to configure clk-phase
clk: socfpga: remove the setting of clk-phase for sdmmc_clk
arm: dts: socfpga: remove "clk-phase" in sdmmc_clk

 .../bindings/mmc/synopsys-dw-mshc.yaml        | 32 ++++++++-
 arch/arm/boot/dts/socfpga.dtsi                |  2 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi        |  2 +-
 .../boot/dts/socfpga_arria10_mercury_aa1.dtsi |  1 +
 .../boot/dts/socfpga_arria10_socdk_sdmmc.dts  |  1 +
 arch/arm/boot/dts/socfpga_arria5.dtsi         |  1 +
 arch/arm/boot/dts/socfpga_cyclone5.dtsi       |  1 +
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi   |  1 +
 .../boot/dts/altera/socfpga_stratix10.dtsi    |  1 +
 .../dts/altera/socfpga_stratix10_socdk.dts    |  1 +
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi |  1 +
 .../boot/dts/intel/socfpga_agilex_socdk.dts   |  1 +
 .../boot/dts/intel/socfpga_n5x_socdk.dts      |  1 +
 drivers/clk/socfpga/clk-gate-a10.c            | 68 -------------------
 drivers/clk/socfpga/clk-gate.c                | 60 ----------------
 drivers/clk/socfpga/clk.h                     |  1 -
 drivers/mmc/host/dw_mmc-pltfm.c               | 43 +++++++++++-
 17 files changed, 83 insertions(+), 135 deletions(-)
Signature · 1 line
2.25.1
author maintainer
#1 lore raw

PATCH v5 1/6 dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

Document the optional "altr,sysmgr-syscon" binding that is used to access the System Manager register that controls the SDMMC clock phase.

---
v5: document reg shift
v4: add else statement
v3: document that the "altr,sysmgr-syscon" binding is only applicable to
"altr,socfpga-dw-mshc"
v2: document "altr,sysmgr-syscon" in the MMC section

---
 .../bindings/mmc/synopsys-dw-mshc.yaml        | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml +29 -3
index ae6d6fca79e2..950fa6bd11fd 100644
@@ -6,9 +6,6 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Synopsys Designware Mobile Storage Host Controller Binding
 
-allOf:
-  - $ref: "synopsys-dw-mshc-common.yaml#"
-
 maintainers:
   - Ulf Hansson <[email protected]>
 
@@ -38,6 +35,35 @@ properties:
       - const: biu
       - const: ciu
 
+  altr,sysmgr-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the sysmgr node
+          - description: register offset that controls the SDMMC clock phase
+          - description: register shift for the smplsel(drive in) setting
+    description:
+      Contains the phandle to System Manager block that contains
+      the SDMMC clock-phase control register. The first value is the pointer
+      to the sysmgr, the 2nd value is the register offset for the SDMMC
+      clock phase register, and the 3rd value is the bit shift for the
+      smplsel(drive in) setting.
+
+allOf:
+  - $ref: "synopsys-dw-mshc-common.yaml#"
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: altr,socfpga-dw-mshc
+    then:
+      required:
+        - altr,sysmgr-syscon
+    else:
+      properties:
+        altr,sysmgr-syscon: false
+
 required:
   - compatible
   - reg
Signature · 1 line
2.25.1
maintainer
#2 lore raw

Re: [PATCHv5 1/6] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

Quoted 11 lines · Dinh Nguyen

Document the optional "altr,sysmgr-syscon" binding that is used to access the System Manager register that controls the SDMMC clock phase.

---
v5: document reg shift
v4: add else statement
v3: document that the "altr,sysmgr-syscon" binding is only applicable to
"altr,socfpga-dw-mshc"
v2: document "altr,sysmgr-syscon" in the MMC section

Quoted 4 lines
---
 .../bindings/mmc/synopsys-dw-mshc.yaml        | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)

Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/

dwmmc0@ff704000: $nodename:0: 'dwmmc0@ff704000' does not match '^mmc(@.*)?$'
arch/arm/boot/dts/socfpga_arria5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_chameleon96.dtb arch/arm/boot/dts/socfpga_cyclone5_de0_nano_soc.dtb arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dtb arch/arm/boot/dts/socfpga_cyclone5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_sockit.dtb arch/arm/boot/dts/socfpga_cyclone5_socrates.dtb arch/arm/boot/dts/socfpga_cyclone5_sodia.dtb arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dtb arch/arm/boot/dts/socfpga_vt.dtb

dwmmc0@ff704000: 'altr,sysmgr-syscon' is a required property
arch/arm/boot/dts/socfpga_arria5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_chameleon96.dtb arch/arm/boot/dts/socfpga_cyclone5_de0_nano_soc.dtb arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dtb arch/arm/boot/dts/socfpga_cyclone5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_sockit.dtb arch/arm/boot/dts/socfpga_cyclone5_socrates.dtb arch/arm/boot/dts/socfpga_cyclone5_sodia.dtb

dwmmc0@ff704000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'broken-cd', 'bus-width', 'cap-mmc-highspeed', 'cap-sd-highspeed', 'cd-gpios', 'fifo-depth', 'resets', 'vmmc-supply', 'vqmmc-supply' were unexpected)
arch/arm/boot/dts/socfpga_cyclone5_socdk.dtb
arch/arm/boot/dts/socfpga_cyclone5_sodia.dtb

dwmmc0@ff704000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'broken-cd', 'bus-width', 'cap-mmc-highspeed', 'cap-sd-highspeed', 'fifo-depth', 'resets', 'vmmc-supply', 'vqmmc-supply' were unexpected)
arch/arm/boot/dts/socfpga_arria5_socdk.dtb
arch/arm/boot/dts/socfpga_cyclone5_chameleon96.dtb
arch/arm/boot/dts/socfpga_cyclone5_de0_nano_soc.dtb
arch/arm/boot/dts/socfpga_cyclone5_sockit.dtb

dwmmc0@ff704000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'broken-cd', 'bus-width', 'cap-mmc-highspeed', 'cap-sd-highspeed', 'fifo-depth', 'resets' were unexpected)
arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dtb
arch/arm/boot/dts/socfpga_cyclone5_socrates.dtb
arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dtb
arch/arm/boot/dts/socfpga_vt.dtb

dwmmc0@ff808000: $nodename:0: 'dwmmc0@ff808000' does not match '^mmc(@.*)?$'
arch/arm/boot/dts/socfpga_arria10_chameleonv3.dtb arch/arm/boot/dts/socfpga_arria10_socdk_nand.dtb arch/arm/boot/dts/socfpga_arria10_socdk_qspi.dtb arch/arm/boot/dts/socfpga_arria10_socdk_sdmmc.dtb

dwmmc0@ff808000: 'altr,sysmgr-syscon' is a required property
arch/arm/boot/dts/socfpga_arria10_chameleonv3.dtb arch/arm/boot/dts/socfpga_arria10_socdk_sdmmc.dtb

dwmmc0@ff808000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'broken-cd', 'bus-width', 'cap-mmc-highspeed', 'cap-sd-highspeed', 'fifo-depth', 'resets' were unexpected)
arch/arm/boot/dts/socfpga_arria10_socdk_sdmmc.dtb

dwmmc0@ff808000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'broken-cd', 'bus-width', 'cap-sd-highspeed', 'fifo-depth', 'resets' were unexpected)
arch/arm/boot/dts/socfpga_arria10_chameleonv3.dtb

dwmmc0@ff808000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'fifo-depth', 'resets' were unexpected)
arch/arm/boot/dts/socfpga_arria10_socdk_nand.dtb
arch/arm/boot/dts/socfpga_arria10_socdk_qspi.dtb

mmc@ff808000: 'altr,sysmgr-syscon' is a required property
arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dtb arch/arm64/boot/dts/altera/socfpga_stratix10_swvp.dtb arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dtb arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dtb

mmc@ff808000: Unevaluated properties are not allowed ('altr,dw-mshc-ciu-div', 'altr,dw-mshc-sdr-timing', 'iommus' were unexpected)
arch/arm64/boot/dts/altera/socfpga_stratix10_swvp.dtb

mmc@ff808000: Unevaluated properties are not allowed ('iommus' was unexpected)

	arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dtb
	arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dtb
	arch/arm64/boot/dts/intel/socfpga_agilex_n6000.dtb
	arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dtb
	arch/arm64/boot/dts/intel/socfpga_agilex_socdk_nand.dtb
	arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dtb

mmcsd@40004000: $nodename:0: 'mmcsd@40004000' does not match '^mmc(@.*)?$'

	arch/arm/boot/dts/lpc4337-ciaa.dtb
	arch/arm/boot/dts/lpc4350-hitex-eval.dtb
	arch/arm/boot/dts/lpc4357-ea4357-devkit.dtb
	arch/arm/boot/dts/lpc4357-myd-lpc4357.dtb

mmcsd@40004000: clock-names:0: 'biu' was expected

	arch/arm/boot/dts/lpc4337-ciaa.dtb
	arch/arm/boot/dts/lpc4350-hitex-eval.dtb
	arch/arm/boot/dts/lpc4357-ea4357-devkit.dtb
	arch/arm/boot/dts/lpc4357-myd-lpc4357.dtb

mmcsd@40004000: clock-names:1: 'ciu' was expected

	arch/arm/boot/dts/lpc4337-ciaa.dtb
	arch/arm/boot/dts/lpc4350-hitex-eval.dtb
	arch/arm/boot/dts/lpc4357-ea4357-devkit.dtb
	arch/arm/boot/dts/lpc4357-myd-lpc4357.dtb

mmcsd@40004000: Unevaluated properties are not allowed ('bus-width', 'clock-names', 'resets', 'vmmc-supply' were unexpected)

	arch/arm/boot/dts/lpc4357-ea4357-devkit.dtb
	arch/arm/boot/dts/lpc4357-myd-lpc4357.dtb

mmcsd@40004000: Unevaluated properties are not allowed ('clock-names', 'resets' were unexpected)

	arch/arm/boot/dts/lpc4337-ciaa.dtb
	arch/arm/boot/dts/lpc4350-hitex-eval.dtb
maintainer
#3 lore raw

Re: [PATCHv5 1/6] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

Quoted 21 lines · Rob Herring
Quoted 11 lines · Dinh Nguyen

Document the optional "altr,sysmgr-syscon" binding that is used to access the System Manager register that controls the SDMMC clock phase.

---
v5: document reg shift
v4: add else statement
v3: document that the "altr,sysmgr-syscon" binding is only applicable to
"altr,socfpga-dw-mshc"
v2: document "altr,sysmgr-syscon" in the MMC section

Quoted 4 lines
---
 .../bindings/mmc/synopsys-dw-mshc.yaml        | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)

Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings.

Quoted 7 lines

Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/

dwmmc0@ff704000: $nodename:0: 'dwmmc0@ff704000' does not match '^mmc(@.*)?$'

Not necessary for this series, but it would be nice if existing warnings were fixed before adding new things. Especially since most of the warnings on this common bindings are all socfpga. It may become required at some point, not just nice.

The node name is the cause of most/all the unevaluated property warnings.

Quoted 11 lines

arch/arm/boot/dts/socfpga_arria5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_chameleon96.dtb arch/arm/boot/dts/socfpga_cyclone5_de0_nano_soc.dtb arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dtb arch/arm/boot/dts/socfpga_cyclone5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_sockit.dtb arch/arm/boot/dts/socfpga_cyclone5_socrates.dtb arch/arm/boot/dts/socfpga_cyclone5_sodia.dtb arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dtb arch/arm/boot/dts/socfpga_vt.dtb

Quoted 9 lines

dwmmc0@ff704000: 'altr,sysmgr-syscon' is a required property
arch/arm/boot/dts/socfpga_arria5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_chameleon96.dtb arch/arm/boot/dts/socfpga_cyclone5_de0_nano_soc.dtb arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dtb arch/arm/boot/dts/socfpga_cyclone5_socdk.dtb arch/arm/boot/dts/socfpga_cyclone5_sockit.dtb arch/arm/boot/dts/socfpga_cyclone5_socrates.dtb arch/arm/boot/dts/socfpga_cyclone5_sodia.dtb

I thought it was optional? New required properties are a possible ABI break.

Rob

Reviewed-by on v5 1/6
#4 lore raw

Re: [PATCHv5 1/6] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

In reply to Dinh Nguyen

On 19/10/2022 13:06, Dinh Nguyen wrote:

Thank you for your patch. There is something to discuss/improve.

Quoted 16 lines
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml +6 -3
-allOf:
-  - $ref: "synopsys-dw-mshc-common.yaml#"
-
 maintainers:
   - Ulf Hansson <[email protected]>
@@ -38,6 +35,35 @@ properties:
       - const: biu
       - const: ciu
 
+  altr,sysmgr-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the sysmgr node
+          - description: register offset that controls the SDMMC clock phase

Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml properties: · line 53

Quoted 10 lines
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml +10 -0
+          - description: register shift for the smplsel(drive in) setting
+    description:
+      Contains the phandle to System Manager block that contains
+      the SDMMC clock-phase control register. The first value is the pointer
+      to the sysmgr, the 2nd value is the register offset for the SDMMC
+      clock phase register, and the 3rd value is the bit shift for the
+      smplsel(drive in) setting.
+
+allOf:
+  - $ref: "synopsys-dw-mshc-common.yaml#"

If there is going to be resend, please drop quotes here.

Best regards,
Krzysztof

author maintainer
#5 lore raw

PATCH v5 2/6 arm64: dts: socfpga: Add clk-phase-sd-hs property to the sdmmc node

In reply to Dinh Nguyen

The sdmmc controller's CIU(Card Interface Unit) clock's phase can be adjusted through the register in the system manager. Add the binding "altr,sysmgr-syscon" to the SDMMC node for the driver to access the system manager. Add the "clk-phase-sd-hs" property in the SDMMC node to designate the smpsel and drvsel properties for the CIU clock.

---
v5: add back reg_shift
v4: no change
v3: removed unnecessary property in "altr,sysmgr-syscon"

---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi      | 1 +
 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts | 1 +
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi          | 1 +
 arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts     | 1 +
 arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dts        | 1 +
 5 files changed, 5 insertions(+)
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi +1 -0
index 14c220d87807..55c5e1fdddc7 100644
@@ -309,6 +309,7 @@ mmc: mmc@ff808000 {
 				 <&clkmgr STRATIX10_SDMMC_CLK>;
 			clock-names = "biu", "ciu";
 			iommus = <&smmu 5>;
+			altr,sysmgr-syscon = <&sysmgr 0x28 4>;
 			status = "disabled";
 		};
 
arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts +1 -0
index 48424e459f12..19e7284b4cd5 100644
@@ -105,6 +105,7 @@ &mmc {
 	cap-mmc-highspeed;
 	broken-cd;
 	bus-width = <4>;
+	clk-phase-sd-hs = <0>, <135>;
 };
 
 &osc1 {
arch/arm64/boot/dts/intel/socfpga_agilex.dtsi +1 -0
index 7bbec8aafa62..849b46dd8098 100644
@@ -313,6 +313,7 @@ mmc: mmc@ff808000 {
 				 <&clkmgr AGILEX_SDMMC_CLK>;
 			clock-names = "biu", "ciu";
 			iommus = <&smmu 5>;
+			altr,sysmgr-syscon = <&sysmgr 0x28 4>;
 			status = "disabled";
 		};
 
arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts +1 -0
index 26cd3c121757..07c3f8876613 100644
@@ -83,6 +83,7 @@ &mmc {
 	cap-sd-highspeed;
 	broken-cd;
 	bus-width = <4>;
+	clk-phase-sd-hs = <0>, <135>;
 };
 
 &osc1 {
arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dts +1 -0
index 62c66e52b656..08c088571270 100644
@@ -74,6 +74,7 @@ &mmc {
 	cap-sd-highspeed;
 	broken-cd;
 	bus-width = <4>;
+	clk-phase-sd-hs = <0>, <135>;
 };
 
 &osc1 {
Signature · 1 line
2.25.1
author maintainer
#6 lore raw

PATCH v5 3/6 arm: dts: socfpga: Add clk-phase-sd-hs property to the sdmmc node

In reply to Dinh Nguyen

The sdmmc controller's CIU(Card Interface Unit) clock's phase can be adjusted through the register in the system manager. Add the binding "altr,sysmgr-syscon" to the SDMMC node for the driver to access the system manager. Add the "clk-phase-sd-hs" property in the SDMMC node to designate the smpsel and drvsel properties for the CIU clock.

---
v5: new

---
 arch/arm/boot/dts/socfpga.dtsi                     | 1 +
 arch/arm/boot/dts/socfpga_arria10.dtsi             | 1 +
 arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dtsi | 1 +
 arch/arm/boot/dts/socfpga_arria10_socdk_sdmmc.dts  | 1 +
 arch/arm/boot/dts/socfpga_arria5.dtsi              | 1 +
 arch/arm/boot/dts/socfpga_cyclone5.dtsi            | 1 +
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi        | 1 +
 7 files changed, 7 insertions(+)
arch/arm/boot/dts/socfpga.dtsi +1 -0
index 2459f3cd7dd9..604fc6e0c4ad 100644
@@ -765,6 +765,7 @@ mmc: dwmmc0@ff704000 {
 			clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>;
 			clock-names = "biu", "ciu";
 			resets = <&rst SDMMC_RESET>;
+			altr,sysmgr-syscon = <&sysmgr 0x108 3>;
 			status = "disabled";
 		};
 
arch/arm/boot/dts/socfpga_arria10.dtsi +1 -0
index 4370e3cbbb4b..b6ebe207e2bc 100644
@@ -666,6 +666,7 @@ mmc: dwmmc0@ff808000 {
 			clocks = <&l4_mp_clk>, <&sdmmc_clk>;
 			clock-names = "biu", "ciu";
 			resets = <&rst SDMMC_RESET>;
+			altr,sysmgr-syscon = <&sysmgr 0x28 4>;
 			status = "disabled";
 		};
 
arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dtsi +1 -0
index ad7cd14de6b6..41f865c8c098 100644
@@ -73,6 +73,7 @@ &mmc {
 	cap-sd-highspeed;
 	broken-cd;
 	bus-width = <4>;
+	clk-phase-sd-hs = <0>, <135>;
 };
 
 &osc1 {
arch/arm/boot/dts/socfpga_arria10_socdk_sdmmc.dts +1 -0
index 64dc0799f3d7..d3969367f4b5 100644
@@ -12,6 +12,7 @@ &mmc {
 	cap-mmc-highspeed;
 	broken-cd;
 	bus-width = <4>;
+	clk-phase-sd-hs = <0>, <135>;
 };
 
 &eccmgr {
arch/arm/boot/dts/socfpga_arria5.dtsi +1 -0
index 22dbf07afcff..b531639ce7dc 100644
@@ -23,6 +23,7 @@ mmc0: dwmmc0@ff704000 {
 			bus-width = <4>;
 			cap-mmc-highspeed;
 			cap-sd-highspeed;
+			clk-phase-sd-hs = <0>, <135>;
 		};
 
 		sysmgr@ffd08000 {
arch/arm/boot/dts/socfpga_cyclone5.dtsi +1 -0
index 319a71e41ea4..a9d1ba66f1ff 100644
@@ -23,6 +23,7 @@ mmc0: dwmmc0@ff704000 {
 			bus-width = <4>;
 			cap-mmc-highspeed;
 			cap-sd-highspeed;
+			clk-phase-sd-hs = <0>, <135>;
 		};
 
 		sysmgr@ffd08000 {
arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi +1 -0
index bd92806ffc12..3b9daddf91cd 100644
@@ -18,5 +18,6 @@ memory@0 {
 
 &mmc0 {	/* On-SoM eMMC */
 	bus-width = <8>;
+	clk-phase-sd-hs = <0>, <135>;
 	status = "okay";
 };
Signature · 1 line
2.25.1
author maintainer
#7 lore raw

PATCH v5 4/6 mmc: dw_mmc-pltfm: socfpga: add method to configure clk-phase

In reply to Dinh Nguyen

The clock-phase settings for the SDMMC controller in the SoCFPGA platforms reside in a register in the System Manager. Add a method to access that register through the syscon interface.

---
v5: change error handling from of_property_read_variable_u32_array()
support arm32 by reading the reg_shift
v4: no change
v3: add space before &socfpga_drv_data
v2: simplify clk-phase calculations

---
 drivers/mmc/host/dw_mmc-pltfm.c | 43 ++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc-pltfm.c +42 -1
index 9901208be797..74421d13f466 100644
@@ -17,10 +17,16 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/of.h>
+#include <linux/mfd/altera-sysmgr.h>
+#include <linux/regmap.h>
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
+#define SOCFPGA_DW_MMC_CLK_PHASE_STEP	45
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel, reg_shift) \
+	((((smplsel) & 0x7) << reg_shift) | (((drvsel) & 0x7) << 0))
+
 int dw_mci_pltfm_register(struct platform_device *pdev,
 			  const struct dw_mci_drv_data *drv_data)
 {
@@ -62,9 +68,44 @@ const struct dev_pm_ops dw_mci_pltfm_pmops = {
 };
 EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
+static int dw_mci_socfpga_priv_init(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct regmap *sys_mgr_base_addr;
+	u32 clk_phase[2] = {0}, reg_offset, reg_shift;
+	int i, rc, hs_timing;
+
+	rc = of_property_read_variable_u32_array(np, "clk-phase-sd-hs", &clk_phase[0], 2, 0);
+	if (rc < 0) {
+		dev_err(host->dev, "clk-phase-sd-hs not found!\n");
+		return rc;
+	}
+
+	sys_mgr_base_addr = altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
+	if (IS_ERR(sys_mgr_base_addr)) {
+		dev_err(host->dev, "failed to find altr,sys-mgr regmap!\n");
+		return -ENODEV;
+	}
+
+	of_property_read_u32_index(np, "altr,sysmgr-syscon", 1, &reg_offset);
+	of_property_read_u32_index(np, "altr,sysmgr-syscon", 2, &reg_shift);
+
+	for (i = 0; i < ARRAY_SIZE(clk_phase); i++)
+		clk_phase[i] /= SOCFPGA_DW_MMC_CLK_PHASE_STEP;
+
+	hs_timing = SYSMGR_SDMMC_CTRL_SET(clk_phase[0], clk_phase[1], reg_shift);
+	regmap_write(sys_mgr_base_addr, reg_offset, hs_timing);
+
+	return 0;
+}
+
+static const struct dw_mci_drv_data socfpga_drv_data = {
+	.init		= dw_mci_socfpga_priv_init,
+};
+
 static const struct of_device_id dw_mci_pltfm_match[] = {
 	{ .compatible = "snps,dw-mshc", },
-	{ .compatible = "altr,socfpga-dw-mshc", },
+	{ .compatible = "altr,socfpga-dw-mshc", .data = &socfpga_drv_data, },
 	{ .compatible = "img,pistachio-dw-mshc", },
 	{},
 };
Signature · 1 line
2.25.1
#8 lore raw

Re: [PATCHv5 4/6] mmc: dw_mmc-pltfm: socfpga: add method to configure clk-phase

Quoted 47 lines · Dinh Nguyen

The clock-phase settings for the SDMMC controller in the SoCFPGA platforms reside in a register in the System Manager. Add a method to access that register through the syscon interface.

---
v5: change error handling from of_property_read_variable_u32_array()
support arm32 by reading the reg_shift
v4: no change
v3: add space before &socfpga_drv_data
v2: simplify clk-phase calculations

---
 drivers/mmc/host/dw_mmc-pltfm.c | 43 ++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc-pltfm.c +13 -0
index 9901208be797..74421d13f466 100644
@@ -17,10 +17,16 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/of.h>
+#include <linux/mfd/altera-sysmgr.h>
+#include <linux/regmap.h>
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
+#define SOCFPGA_DW_MMC_CLK_PHASE_STEP	45
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel, reg_shift) \
+	((((smplsel) & 0x7) << reg_shift) | (((drvsel) & 0x7) << 0))
+
 int dw_mci_pltfm_register(struct platform_device *pdev,
 			  const struct dw_mci_drv_data *drv_data)
 {
@@ -62,9 +68,44 @@ const struct dev_pm_ops dw_mci_pltfm_pmops = {
 };
 EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
+static int dw_mci_socfpga_priv_init(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct regmap *sys_mgr_base_addr;
+	u32 clk_phase[2] = {0}, reg_offset, reg_shift;
+	int i, rc, hs_timing;
+

drivers/mmc/host/dw_mmc-pltfm.c const struct dev_pm_ops dw_mci_pltfm_pmops = { · line 87

Quoted 10 lines
drivers/mmc/host/dw_mmc-pltfm.c +10 -0
+	rc = of_property_read_variable_u32_array(np, "clk-phase-sd-hs", &clk_phase[0], 2, 0);
+	if (rc < 0) {
+		dev_err(host->dev, "clk-phase-sd-hs not found!\n");
+		return rc;
+	}
+
+	sys_mgr_base_addr = altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
+	if (IS_ERR(sys_mgr_base_addr)) {
+		dev_err(host->dev, "failed to find altr,sys-mgr regmap!\n");
+		return -ENODEV;

Isn't this now an ABI break? I have an impression we talked about this...

Best regards,
Krzysztof

author maintainer
#9 lore raw

Re: [PATCHv5 4/6] mmc: dw_mmc-pltfm: socfpga: add method to configure clk-phase

Quoted 52 lines · Krzysztof Kozlowski
Quoted 51 lines · Dinh Nguyen

The clock-phase settings for the SDMMC controller in the SoCFPGA platforms reside in a register in the System Manager. Add a method to access that register through the syscon interface.

---
v5: change error handling from of_property_read_variable_u32_array()
support arm32 by reading the reg_shift
v4: no change
v3: add space before &socfpga_drv_data
v2: simplify clk-phase calculations

---
  drivers/mmc/host/dw_mmc-pltfm.c | 43 ++++++++++++++++++++++++++++++++-
  1 file changed, 42 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc-pltfm.c +17 -0
index 9901208be797..74421d13f466 100644
@@ -17,10 +17,16 @@
  #include <linux/mmc/host.h>
  #include <linux/mmc/mmc.h>
  #include <linux/of.h>
+#include <linux/mfd/altera-sysmgr.h>
+#include <linux/regmap.h>
 
  #include "dw_mmc.h"
  #include "dw_mmc-pltfm.h"
 
+#define SOCFPGA_DW_MMC_CLK_PHASE_STEP	45
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel, reg_shift) \
+	((((smplsel) & 0x7) << reg_shift) | (((drvsel) & 0x7) << 0))
+
  int dw_mci_pltfm_register(struct platform_device *pdev,
  			  const struct dw_mci_drv_data *drv_data)
  {
@@ -62,9 +68,44 @@ const struct dev_pm_ops dw_mci_pltfm_pmops = {
  };
  EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
+static int dw_mci_socfpga_priv_init(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct regmap *sys_mgr_base_addr;
+	u32 clk_phase[2] = {0}, reg_offset, reg_shift;
+	int i, rc, hs_timing;
+
+	rc = of_property_read_variable_u32_array(np, "clk-phase-sd-hs", &clk_phase[0], 2, 0);
+	if (rc < 0) {
+		dev_err(host->dev, "clk-phase-sd-hs not found!\n");
+		return rc;
Quoted 9 lines

drivers/mmc/host/dw_mmc-pltfm.c const struct dev_pm_ops dw_mci_pltfm_pmops = { · line 87

Quoted 6 lines
drivers/mmc/host/dw_mmc-pltfm.c +6 -0
+	}
+
+	sys_mgr_base_addr = altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
+	if (IS_ERR(sys_mgr_base_addr)) {
+		dev_err(host->dev, "failed to find altr,sys-mgr regmap!\n");
+		return -ENODEV;

Isn't this now an ABI break? I have an impression we talked about this...

My fault, I'll make this optional.

Dinh

author maintainer
#10 lore raw

PATCH v5 5/6 clk: socfpga: remove the setting of clk-phase for sdmmc_clk

In reply to Dinh Nguyen

Now that the SDMMC driver supports setting the clk-phase, we can remove the need to do it in the clock driver.

---
v5: new

---
 drivers/clk/socfpga/clk-gate-a10.c | 68 ------------------------------
 drivers/clk/socfpga/clk-gate.c     | 60 --------------------------
 drivers/clk/socfpga/clk.h          |  1 -
 3 files changed, 129 deletions(-)
drivers/clk/socfpga/clk-gate-a10.c +0 -68
index 738c53391e39..7cdf2f07c79b 100644
@@ -35,59 +35,7 @@ static unsigned long socfpga_gate_clk_recalc_rate(struct clk_hw *hwclk,
 	return parent_rate / div;
 }
 
-static int socfpga_clk_prepare(struct clk_hw *hwclk)
-{
-	struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
-	int i;
-	u32 hs_timing;
-	u32 clk_phase[2];
-
-	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
-		for (i = 0; i < ARRAY_SIZE(clk_phase); i++) {
-			switch (socfpgaclk->clk_phase[i]) {
-			case 0:
-				clk_phase[i] = 0;
-				break;
-			case 45:
-				clk_phase[i] = 1;
-				break;
-			case 90:
-				clk_phase[i] = 2;
-				break;
-			case 135:
-				clk_phase[i] = 3;
-				break;
-			case 180:
-				clk_phase[i] = 4;
-				break;
-			case 225:
-				clk_phase[i] = 5;
-				break;
-			case 270:
-				clk_phase[i] = 6;
-				break;
-			case 315:
-				clk_phase[i] = 7;
-				break;
-			default:
-				clk_phase[i] = 0;
-				break;
-			}
-		}
-
-		hs_timing = SYSMGR_SDMMC_CTRL_SET_AS10(clk_phase[0], clk_phase[1]);
-		if (!IS_ERR(socfpgaclk->sys_mgr_base_addr))
-			regmap_write(socfpgaclk->sys_mgr_base_addr,
-				     SYSMGR_SDMMCGRP_CTRL_OFFSET, hs_timing);
-		else
-			pr_err("%s: cannot set clk_phase because sys_mgr_base_addr is not available!\n",
-					__func__);
-	}
-	return 0;
-}
-
 static struct clk_ops gateclk_ops = {
-	.prepare = socfpga_clk_prepare,
 	.recalc_rate = socfpga_gate_clk_recalc_rate,
 };
 
@@ -96,7 +44,6 @@ static void __init __socfpga_gate_init(struct device_node *node,
 {
 	u32 clk_gate[2];
 	u32 div_reg[3];
-	u32 clk_phase[2];
 	u32 fixed_div;
 	struct clk_hw *hw_clk;
 	struct socfpga_gate_clk *socfpga_clk;
@@ -136,21 +83,6 @@ static void __init __socfpga_gate_init(struct device_node *node,
 		socfpga_clk->div_reg = NULL;
 	}
 
-	rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
-	if (!rc) {
-		socfpga_clk->clk_phase[0] = clk_phase[0];
-		socfpga_clk->clk_phase[1] = clk_phase[1];
-
-		socfpga_clk->sys_mgr_base_addr =
-			syscon_regmap_lookup_by_compatible("altr,sys-mgr");
-		if (IS_ERR(socfpga_clk->sys_mgr_base_addr)) {
-			pr_err("%s: failed to find altr,sys-mgr regmap!\n",
-					__func__);
-			kfree(socfpga_clk);
-			return;
-		}
-	}
-
 	of_property_read_string(node, "clock-output-names", &clk_name);
 
 	init.name = clk_name;
drivers/clk/socfpga/clk-gate.c +0 -60

73 lines not on this page · raw message

drivers/clk/socfpga/clk.h +0 -1
index d80115fbdd6a..9a2fb2dde5b8 100644
@@ -50,7 +50,6 @@ struct socfpga_gate_clk {
 	u32 width;	/* only valid if div_reg != 0 */
 	u32 shift;	/* only valid if div_reg != 0 */
 	u32 bypass_shift;      /* only valid if bypass_reg != 0 */
-	u32 clk_phase[2];
 };
 
 struct socfpga_periph_clk {
Signature · 1 line
2.25.1
author maintainer
#11 lore raw

PATCH v5 6/6 arm: dts: socfpga: remove "clk-phase" in sdmmc_clk

In reply to Dinh Nguyen

Now that the SDMMC driver can use the "clk-phase-sd-hs" binding, we don't need the clk-phase in the sdmmc_clk anymore.

---
v5: new

---
 arch/arm/boot/dts/socfpga.dtsi         | 1 -
 arch/arm/boot/dts/socfpga_arria10.dtsi | 1 -
 2 files changed, 2 deletions(-)
arch/arm/boot/dts/socfpga.dtsi +0 -1
index 604fc6e0c4ad..a2419a5c6c26 100644
@@ -453,7 +453,6 @@ sdmmc_clk: sdmmc_clk {
 						compatible = "altr,socfpga-gate-clk";
 						clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
 						clk-gate = <0xa0 8>;
-						clk-phase = <0 135>;
 					};
 
 					sdmmc_clk_divided: sdmmc_clk_divided {
arch/arm/boot/dts/socfpga_arria10.dtsi +0 -1
index b6ebe207e2bc..eb528c103d70 100644
@@ -365,7 +365,6 @@ sdmmc_clk: sdmmc_clk {
 						compatible = "altr,socfpga-a10-gate-clk";
 						clocks = <&sdmmc_free_clk>;
 						clk-gate = <0xC8 5>;
-						clk-phase = <0 135>;
 					};
 
 					qspi_clk: qspi_clk {
Signature · 1 line
2.25.1