diff mbox series

[PATCHv4,1/3] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

Message ID 20220928165420.1212284-1-dinguyen@kernel.org
State New
Headers show
Series [PATCHv4,1/3] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon" | expand

Commit Message

Dinh Nguyen Sept. 28, 2022, 4:54 p.m. UTC
Document the optional "altr,sysmgr-syscon" binding that is used to
access the System Manager register that controls the SDMMC clock
phase.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
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        | 31 +++++++++++++++++--
 1 file changed, 28 insertions(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Oct. 1, 2022, 10:06 a.m. UTC | #1
On 29/09/2022 17:18, Dinh Nguyen wrote:
> 
> 
> On 9/29/22 09:38, Krzysztof Kozlowski wrote:
>> On 29/09/2022 16:20, Dinh Nguyen wrote:
>>>>
>>>> So this change will not be backwards compatible with existing DTBs. I
>>>> noticed that patch2 updates the DTS files for the arm64 platforms, but
>>>> there seems to be some arm32 platforms too. Isn't this going to be a
>>>> problem?
>>>>
>>>
>>> The arm32 platforms makes the clk-phase adjustment through the clock
>>> driver. There was a discussion when I originally submitted the support
>>> for the arm32 platforms, and we landed on going through the clock driver
>>> instead of using the MMC driver. The updates to the arm32 platforms can
>>> be done after this patch series.
>>
>> How the update "can be done after"? Didn't you break all boards in- and
>> out-of-tree?
>>
> 
> I don't think so! At least, I don't see how, for the arm32 boards, here 
> are the dts entry for setting the clock-phase:
> 
> sdmmc_clk: sdmmc_clk {
> 	#clock-cells = <0>;
> 	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>;   <-----

It's different node...

> };
> 
> sdmmc_clk_divided: sdmmc_clk_divided {
> 	#clock-cells = <0>;
> 	compatible = "altr,socfpga-gate-clk";
> 	clocks = <&sdmmc_clk>;
> 	clk-gate = <0xa0 8>;
> 	fixed-divider = <4>;
> 	};
> 
> ...
> mmc: dwmmc0@ff704000 {
> 	compatible = "altr,socfpga-dw-mshc";
> 	reg = <0xff704000 0x1000>;
> 	interrupts = <0 139 4>;
> 	fifo-depth = <0x400>;
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>;
> 	clock-names = "biu", "ciu";
> 	resets = <&rst SDMMC_RESET>;
> 	status = "disabled";

And this one does not have clk-phase-sd-hs

> 	};
> 
> 
> So the setting for the clk-phase is done in the clock driver, 
> (drivers/clk/socfpga/clk-gate.c). This has been done many years now, 
> before the clk-phase-hs-sd concept was added to the sdmmc driver.

Yes and the driver now requires clk-phase-sd-hs or altr,sysmgr-syscon
which is not present in DTS.

> 
> When I originally submitted the patches for the ARM64 clock driver 
> support, I forgot to add the clk-phase support for the SD controller. 
> Now that I realized we needed it, the concept to set the clk-phase is in 
> the SD driver, thus I'm just adding the support for arm64.
> 
> The arm32 support does not change in any way, so I don't see how it will 
> break it.

Isn't your driver returning ERRNO for all existing DTS (so without patch
#2) and for all out of tree DTS?

> 
> I can update the arm32 support with the same function in patch3 after 
> this series. Because updating the arm32 will require me to remove the 
> support in the clock driver, thus, I want to break it out.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
index ae6d6fca79e2..b73324273464 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
@@ -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 <ulf.hansson@linaro.org>
 
@@ -38,6 +35,34 @@  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:
+      Contains the phandle to System Manager block that contains
+      the SDMMC clock-phase control register. The first value is the pointer
+      to the sysmgr and the 2nd value is the register offset for the SDMMC
+      clock phase register.
+
+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