| 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 |
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 --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
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(-)