| Message ID | 20221026141631.696863-1-dinguyen@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [PATCHv6,1/6] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon" | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, Oct 26, 2022 at 09:16:26AM -0500, Dinh Nguyen wrote: > 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> > --- > v6: make "altr,sysmgr-syscon" optional > 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 | 23 ++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml > index ae6d6fca79e2..0e2024eb9018 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,26 @@ properties: > - const: biu > - const: ciu > > +allOf: > + - $ref: synopsys-dw-mshc-common.yaml# > + > + - if: > + properties: > + compatible: > + contains: > + const: altr,socfpga-dw-mshc > + then: > + properties: > + altr,sysmgr-syscon: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + 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 This goes in the top-level. Use if/then schema to add constraints, not define properties. > + else: Then you'll need to negate the if: if: { not: { properties: ... }} > + properties: > + altr,sysmgr-syscon: false > + > required: > - compatible > - reg > -- > 2.25.1 > >
Quoting Dinh Nguyen (2022-10-26 07:16:30) > Now that the SDMMC driver supports setting the clk-phase, we can remove > the need to do it in the clock driver. > > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> > --- Do you want to take this through mmc tree? Acked-by: Stephen Boyd <sboyd@kernel.org>
On 10/26/22 15:50, Rob Herring wrote: > On Wed, Oct 26, 2022 at 09:16:26AM -0500, Dinh Nguyen wrote: >> 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> >> --- >> v6: make "altr,sysmgr-syscon" optional >> 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 | 23 ++++++++++++++++--- >> 1 file changed, 20 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml >> index ae6d6fca79e2..0e2024eb9018 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,26 @@ properties: >> - const: biu >> - const: ciu >> >> +allOf: >> + - $ref: synopsys-dw-mshc-common.yaml# >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: altr,socfpga-dw-mshc >> + then: >> + properties: >> + altr,sysmgr-syscon: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + 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 > > This goes in the top-level. Use if/then schema to add constraints, not > define properties. > >> + else: > > Then you'll need to negate the if: if: { not: { properties: ... }} > Thanks Rob, is this what you mean? 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: not: <----- add the 'not' here? required: - altr,sysmgr-syscon else: properties: altr,sysmgr-syscon: false
On 10/27/22 16:39, Stephen Boyd wrote: > Quoting Dinh Nguyen (2022-10-26 07:16:30) >> Now that the SDMMC driver supports setting the clk-phase, we can remove >> the need to do it in the clock driver. >> >> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> >> --- > > Do you want to take this through mmc tree? Yes, I'll do that. > > Acked-by: Stephen Boyd <sboyd@kernel.org> Thanks! Dinh
On Wed, 26 Oct 2022 at 16:16, Dinh Nguyen <dinguyen@kernel.org> wrote: > > 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. > > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> > --- > v6: not getting the clk-phase-sd-hs is not a hard failure > 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 > > make property optional in driver > --- > drivers/mmc/host/dw_mmc-pltfm.c | 43 ++++++++++++++++++++++++++++++++- > 1 file changed, 42 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > index 9901208be797..fff6222d58e4 100644 > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -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_info(host->dev, "Optional: clk-phase-sd-hs not found!\n"); Printing things about missing optional features doesn't really make sense. Please drop this. > + return 0; > + } > + > + sys_mgr_base_addr = altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon"); > + if (IS_ERR(sys_mgr_base_addr)) { > + dev_info(host->dev, "Optional: failed to find altr,sys-mgr regmap!\n"); If the clk-phase-sd-hs property is found above, the altr,sysmgr-syscon property is required, isn't it? In that case, perhaps this deserves a dev_warn instead? > + return 0; > + } > + > + of_property_read_u32_index(np, "altr,sysmgr-syscon", 1, ®_offset); > + of_property_read_u32_index(np, "altr,sysmgr-syscon", 2, ®_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", }, > {}, > }; Kind regards Uffe
diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml index ae6d6fca79e2..0e2024eb9018 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,26 @@ properties: - const: biu - const: ciu +allOf: + - $ref: synopsys-dw-mshc-common.yaml# + + - if: + properties: + compatible: + contains: + const: altr,socfpga-dw-mshc + then: + properties: + altr,sysmgr-syscon: + $ref: /schemas/types.yaml#/definitions/phandle-array + 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 + 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> --- v6: make "altr,sysmgr-syscon" optional 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 | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)