Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
Hi. 2017-02-17 23:12 GMT+09:00 Piotr Sroka <pio...@cadence.com>: >> -Original Message- >> From: Ulf Hansson [mailto:ulf.hans...@linaro.org] >> Sent: 16 February, 2017 4:10 PM >> Subject: Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay >> configuration >> >> On 16 February 2017 at 14:06, Piotr Sroka <pio...@cadence.com> wrote: >> > DTS properties are used instead of fixed data because PHY settings can >> > be different for different platforms. >> > Configuration of new three PHY delays were added >> > >> > Signed-off-by: Piotr Sroka <pio...@cadence.com> >> > --- >> > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ >> >> Please split this patch. >> >> DT docs should be a separate patch and make sure it precedes the changes >> where the new bindings are being parsed in the driver code. >> > > Ok I will do it in next version of patch. > >> > drivers/mmc/host/sdhci-cadence.c | 83 >> > +++- >> -- >> > 2 files changed, 129 insertions(+), 8 deletions(-) >> > >> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > index c0f37cb..221d3fe 100644 >> > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. >> > - mmc-hs400-1_8v >> > - mmc-hs400-1_2v >> > >> > +- phy-input-delay-sd-hs: >> > + Value of the delay in the input path for High Speed work mode. >> > + Valid range = [0:0x1F]. Instead of bunch of new bindings, a data associated with an SoC specific compatible will do in most cases. static const struct of_device_id sdhci_cdns_match[] = { { .compatible = "socionext,uniphier-sd4hc", .data = sdhci_cdns_uniphier_phy_data, }, { .compatible = "cdns,sd4hc" }, { /* sentinel */ } }; Strictly speaking, the DLL delays will depend on board design as well as SoC. So, DT bindings would be more flexible, though. >> Please specify what unit this in. And then also a suffix, like "-ns" >> to the name of the binding. > > The delay starts from 5ns (for delay parameter equal to 0), and it is > increased by 2.5ns. > 0 - means 5ns, 1 means 7.5 ns etc. In short, all the DT values here are kind of mysterious register values for the PHY. > >> > + if (ret) >> > + return ret; >> > + } >> > + if (!of_property_read_u32(np, "phy-dll-delay-sdclk-hsmmc", )) { >> > + ret = sdhci_cdns_write_phy_reg(priv, >> > + SDHCI_CDNS_PHY_DLY_HSMMC, >> > tmp); >> > + if (ret) >> > + return ret; >> > + } >> > + if (!of_property_read_u32(np, "phy-dll-delay-strobe", )) { >> > + ret = sdhci_cdns_write_phy_reg(priv, >> > + SDHCI_CDNS_PHY_DLY_STROBE, >> > tmp); >> > + if (ret) >> > + return ret; >> > + } >> > + return 0; The repeat of the same pattern, "look up a DT property, then if it exists, set it to a register." Maybe, is it better to describe it with data array + loop, like this? struct sdhci_cdns_phy_cfg { const char *property; u8 addr; }; static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { { "phy-input-delay-sd-hs", SDHCI_CDNS_PHY_DLY_SD_HS, }, { "phy-input-delay-sd-default", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, { "phy-input-delay-sd-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, { "phy-input-delay-sd-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, { "phy-input-delay-sd-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, ... }; static int sdhci_cdns_phy_init(struct device_node *np, struct sdhci_cdns_priv *priv) { u32 val; int i; for (i = 0; i < ARRAY_SIZE(sdhci_cdns_phy_cfgs), i++) { ret = of_property_read_u32(np, sdhci_cdns_phy_cfgs[i].property, ); if (ret) continue; ret = sdhci_cdns_write_phy_reg(priv, sdhci_cdns_phy_cfgs[i].addr, val); if (ret) return ret; } } -- Best Regards Masahiro Yamada
Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
Hi. 2017-02-17 23:12 GMT+09:00 Piotr Sroka : >> -Original Message- >> From: Ulf Hansson [mailto:ulf.hans...@linaro.org] >> Sent: 16 February, 2017 4:10 PM >> Subject: Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay >> configuration >> >> On 16 February 2017 at 14:06, Piotr Sroka wrote: >> > DTS properties are used instead of fixed data because PHY settings can >> > be different for different platforms. >> > Configuration of new three PHY delays were added >> > >> > Signed-off-by: Piotr Sroka >> > --- >> > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ >> >> Please split this patch. >> >> DT docs should be a separate patch and make sure it precedes the changes >> where the new bindings are being parsed in the driver code. >> > > Ok I will do it in next version of patch. > >> > drivers/mmc/host/sdhci-cadence.c | 83 >> > +++- >> -- >> > 2 files changed, 129 insertions(+), 8 deletions(-) >> > >> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > index c0f37cb..221d3fe 100644 >> > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. >> > - mmc-hs400-1_8v >> > - mmc-hs400-1_2v >> > >> > +- phy-input-delay-sd-hs: >> > + Value of the delay in the input path for High Speed work mode. >> > + Valid range = [0:0x1F]. Instead of bunch of new bindings, a data associated with an SoC specific compatible will do in most cases. static const struct of_device_id sdhci_cdns_match[] = { { .compatible = "socionext,uniphier-sd4hc", .data = sdhci_cdns_uniphier_phy_data, }, { .compatible = "cdns,sd4hc" }, { /* sentinel */ } }; Strictly speaking, the DLL delays will depend on board design as well as SoC. So, DT bindings would be more flexible, though. >> Please specify what unit this in. And then also a suffix, like "-ns" >> to the name of the binding. > > The delay starts from 5ns (for delay parameter equal to 0), and it is > increased by 2.5ns. > 0 - means 5ns, 1 means 7.5 ns etc. In short, all the DT values here are kind of mysterious register values for the PHY. > >> > + if (ret) >> > + return ret; >> > + } >> > + if (!of_property_read_u32(np, "phy-dll-delay-sdclk-hsmmc", )) { >> > + ret = sdhci_cdns_write_phy_reg(priv, >> > + SDHCI_CDNS_PHY_DLY_HSMMC, >> > tmp); >> > + if (ret) >> > + return ret; >> > + } >> > + if (!of_property_read_u32(np, "phy-dll-delay-strobe", )) { >> > + ret = sdhci_cdns_write_phy_reg(priv, >> > + SDHCI_CDNS_PHY_DLY_STROBE, >> > tmp); >> > + if (ret) >> > + return ret; >> > + } >> > + return 0; The repeat of the same pattern, "look up a DT property, then if it exists, set it to a register." Maybe, is it better to describe it with data array + loop, like this? struct sdhci_cdns_phy_cfg { const char *property; u8 addr; }; static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { { "phy-input-delay-sd-hs", SDHCI_CDNS_PHY_DLY_SD_HS, }, { "phy-input-delay-sd-default", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, { "phy-input-delay-sd-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, { "phy-input-delay-sd-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, { "phy-input-delay-sd-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, ... }; static int sdhci_cdns_phy_init(struct device_node *np, struct sdhci_cdns_priv *priv) { u32 val; int i; for (i = 0; i < ARRAY_SIZE(sdhci_cdns_phy_cfgs), i++) { ret = of_property_read_u32(np, sdhci_cdns_phy_cfgs[i].property, ); if (ret) continue; ret = sdhci_cdns_write_phy_reg(priv, sdhci_cdns_phy_cfgs[i].addr, val); if (ret) return ret; } } -- Best Regards Masahiro Yamada
RE: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
> -Original Message- > From: Ulf Hansson [mailto:ulf.hans...@linaro.org] > Sent: 16 February, 2017 4:10 PM > Subject: Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay > configuration > > On 16 February 2017 at 14:06, Piotr Sroka <pio...@cadence.com> wrote: > > DTS properties are used instead of fixed data because PHY settings can > > be different for different platforms. > > Configuration of new three PHY delays were added > > > > Signed-off-by: Piotr Sroka <pio...@cadence.com> > > --- > > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ > > Please split this patch. > > DT docs should be a separate patch and make sure it precedes the changes > where the new bindings are being parsed in the driver code. > Ok I will do it in next version of patch. > > drivers/mmc/host/sdhci-cadence.c | 83 > > +++- > -- > > 2 files changed, 129 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > index c0f37cb..221d3fe 100644 > > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. > > - mmc-hs400-1_8v > > - mmc-hs400-1_2v > > > > +- phy-input-delay-sd-hs: > > + Value of the delay in the input path for High Speed work mode. > > + Valid range = [0:0x1F]. > > Please specify what unit this in. And then also a suffix, like "-ns" > to the name of the binding. The delay starts from 5ns (for delay parameter equal to 0), and it is increased by 2.5ns. 0 - means 5ns, 1 means 7.5 ns etc. I will add this description. I think the suffix in this case will not be necessary because the unit is a 2.5ns. What is your opinion? > > Similar comment applies to all new bindings below. > > > + Delay configuration stay unchanged if this property is not provided. > > I would remove this information from the DT doc, it's just confusion when > you refer to something that should remain "unchanged". > Ok I will remove it in next version of patch. > Similar comment applies to all new bindings below. > > > +- phy-input-delay-sd-default: > > + Value of the delay in the input path for Default Speed work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr12: > > + Value of the delay in the input path for SDR12 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr25: > > + Value of the delay in the input path for SDR25 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr50: > > + Value of the delay in the input path for SDR50 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-ddr50: > > + Value of the delay in the input path for DDR50 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-legacy: > > Legacy? As in legacy speed mode? Yes as legacy speed mode. But there are two delays each for one specific mode. One for SD legacy mode and one for MMC legacy mode. > > > + Value of the delay in the input path for eMMC legacy work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-sdr: > > + Value of the delay in the input path for eMMC SDR work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-ddr: > > + Value of the delay in the input path for eMMC DDR work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-dll-delay-sdclk: > > + Value of the delay introduced on the sdclk output > > + for all modes except HS200, HS400 and HS400_ES. > > + Valid range = [0:0x7F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-dll-delay-sdclk-hsmmc: > > + Value of the delay introduced on the sdclk output > > + for HS200, HS400 and HS400_ES speed modes. > > + Valid rang
RE: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
> -Original Message- > From: Ulf Hansson [mailto:ulf.hans...@linaro.org] > Sent: 16 February, 2017 4:10 PM > Subject: Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay > configuration > > On 16 February 2017 at 14:06, Piotr Sroka wrote: > > DTS properties are used instead of fixed data because PHY settings can > > be different for different platforms. > > Configuration of new three PHY delays were added > > > > Signed-off-by: Piotr Sroka > > --- > > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ > > Please split this patch. > > DT docs should be a separate patch and make sure it precedes the changes > where the new bindings are being parsed in the driver code. > Ok I will do it in next version of patch. > > drivers/mmc/host/sdhci-cadence.c | 83 > > +++- > -- > > 2 files changed, 129 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > index c0f37cb..221d3fe 100644 > > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. > > - mmc-hs400-1_8v > > - mmc-hs400-1_2v > > > > +- phy-input-delay-sd-hs: > > + Value of the delay in the input path for High Speed work mode. > > + Valid range = [0:0x1F]. > > Please specify what unit this in. And then also a suffix, like "-ns" > to the name of the binding. The delay starts from 5ns (for delay parameter equal to 0), and it is increased by 2.5ns. 0 - means 5ns, 1 means 7.5 ns etc. I will add this description. I think the suffix in this case will not be necessary because the unit is a 2.5ns. What is your opinion? > > Similar comment applies to all new bindings below. > > > + Delay configuration stay unchanged if this property is not provided. > > I would remove this information from the DT doc, it's just confusion when > you refer to something that should remain "unchanged". > Ok I will remove it in next version of patch. > Similar comment applies to all new bindings below. > > > +- phy-input-delay-sd-default: > > + Value of the delay in the input path for Default Speed work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr12: > > + Value of the delay in the input path for SDR12 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr25: > > + Value of the delay in the input path for SDR25 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-sdr50: > > + Value of the delay in the input path for SDR50 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-sd-ddr50: > > + Value of the delay in the input path for DDR50 work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-legacy: > > Legacy? As in legacy speed mode? Yes as legacy speed mode. But there are two delays each for one specific mode. One for SD legacy mode and one for MMC legacy mode. > > > + Value of the delay in the input path for eMMC legacy work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-sdr: > > + Value of the delay in the input path for eMMC SDR work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-input-delay-emmc-ddr: > > + Value of the delay in the input path for eMMC DDR work mode. > > + Valid range = [0:0x1F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-dll-delay-sdclk: > > + Value of the delay introduced on the sdclk output > > + for all modes except HS200, HS400 and HS400_ES. > > + Valid range = [0:0x7F]. > > + Delay configuration stay unchanged if this property is not provided. > > +- phy-dll-delay-sdclk-hsmmc: > > + Value of the delay introduced on the sdclk output > > + for HS200, HS400 and HS400_ES speed modes. > > + Valid range = [0:0x7F]. > > + Delay configuration s
Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
On 16 February 2017 at 14:06, Piotr Srokawrote: > DTS properties are used instead of fixed data > because PHY settings can be different for different platforms. > Configuration of new three PHY delays were added > > Signed-off-by: Piotr Sroka > --- > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ Please split this patch. DT docs should be a separate patch and make sure it precedes the changes where the new bindings are being parsed in the driver code. > drivers/mmc/host/sdhci-cadence.c | 83 > +++--- > 2 files changed, 129 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > index c0f37cb..221d3fe 100644 > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. > - mmc-hs400-1_8v > - mmc-hs400-1_2v > > +- phy-input-delay-sd-hs: > + Value of the delay in the input path for High Speed work mode. > + Valid range = [0:0x1F]. Please specify what unit this in. And then also a suffix, like "-ns" to the name of the binding. Similar comment applies to all new bindings below. > + Delay configuration stay unchanged if this property is not provided. I would remove this information from the DT doc, it's just confusion when you refer to something that should remain "unchanged". Similar comment applies to all new bindings below. > +- phy-input-delay-sd-default: > + Value of the delay in the input path for Default Speed work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr12: > + Value of the delay in the input path for SDR12 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr25: > + Value of the delay in the input path for SDR25 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr50: > + Value of the delay in the input path for SDR50 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-ddr50: > + Value of the delay in the input path for DDR50 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-legacy: Legacy? As in legacy speed mode? > + Value of the delay in the input path for eMMC legacy work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-sdr: > + Value of the delay in the input path for eMMC SDR work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-ddr: > + Value of the delay in the input path for eMMC DDR work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-sdclk: > + Value of the delay introduced on the sdclk output > + for all modes except HS200, HS400 and HS400_ES. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-sdclk-hsmmc: > + Value of the delay introduced on the sdclk output > + for HS200, HS400 and HS400_ES speed modes. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-strobe: > + Value of the delay introduced on the dat_strobe input > + used in HS400 / HS400_ES speed modes. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. A general comment, is that I suggest you look at the generic mmc DT bindings for the different speedmodes, and align these new names of DT bindings to those. > + > + > Example: > emmc: sdhci@5a00 { > compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc"; > @@ -29,4 +82,5 @@ Example: > mmc-ddr-1_8v; > mmc-hs200-1_8v; > mmc-hs400-1_8v; > + phy-input-delay-sd-hs = <0>; > }; > diff --git a/drivers/mmc/host/sdhci-cadence.c > b/drivers/mmc/host/sdhci-cadence.c > index c946e45..6c338d2 100644 > --- a/drivers/mmc/host/sdhci-cadence.c > +++ b/drivers/mmc/host/sdhci-cadence.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include "sdhci-pltfm.h" > > @@ -53,6 +54,9 @@ > #define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY 0x06 > #define SDHCI_CDNS_PHY_DLY_EMMC_SDR0x07 > #define SDHCI_CDNS_PHY_DLY_EMMC_DDR0x08 > +#define SDHCI_CDNS_PHY_DLY_SDCLK 0x0b > +#define SDHCI_CDNS_PHY_DLY_HSMMC 0x0c > +#define
Re: [PATCH 2/2] [2/2] mmc: sdhci-cadence: Update PHY delay configuration
On 16 February 2017 at 14:06, Piotr Sroka wrote: > DTS properties are used instead of fixed data > because PHY settings can be different for different platforms. > Configuration of new three PHY delays were added > > Signed-off-by: Piotr Sroka > --- > .../devicetree/bindings/mmc/sdhci-cadence.txt | 54 ++ Please split this patch. DT docs should be a separate patch and make sure it precedes the changes where the new bindings are being parsed in the driver code. > drivers/mmc/host/sdhci-cadence.c | 83 > +++--- > 2 files changed, 129 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > index c0f37cb..221d3fe 100644 > --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt > @@ -19,6 +19,59 @@ if supported. See mmc.txt for details. > - mmc-hs400-1_8v > - mmc-hs400-1_2v > > +- phy-input-delay-sd-hs: > + Value of the delay in the input path for High Speed work mode. > + Valid range = [0:0x1F]. Please specify what unit this in. And then also a suffix, like "-ns" to the name of the binding. Similar comment applies to all new bindings below. > + Delay configuration stay unchanged if this property is not provided. I would remove this information from the DT doc, it's just confusion when you refer to something that should remain "unchanged". Similar comment applies to all new bindings below. > +- phy-input-delay-sd-default: > + Value of the delay in the input path for Default Speed work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr12: > + Value of the delay in the input path for SDR12 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr25: > + Value of the delay in the input path for SDR25 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-sdr50: > + Value of the delay in the input path for SDR50 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-sd-ddr50: > + Value of the delay in the input path for DDR50 work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-legacy: Legacy? As in legacy speed mode? > + Value of the delay in the input path for eMMC legacy work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-sdr: > + Value of the delay in the input path for eMMC SDR work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-input-delay-emmc-ddr: > + Value of the delay in the input path for eMMC DDR work mode. > + Valid range = [0:0x1F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-sdclk: > + Value of the delay introduced on the sdclk output > + for all modes except HS200, HS400 and HS400_ES. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-sdclk-hsmmc: > + Value of the delay introduced on the sdclk output > + for HS200, HS400 and HS400_ES speed modes. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. > +- phy-dll-delay-strobe: > + Value of the delay introduced on the dat_strobe input > + used in HS400 / HS400_ES speed modes. > + Valid range = [0:0x7F]. > + Delay configuration stay unchanged if this property is not provided. A general comment, is that I suggest you look at the generic mmc DT bindings for the different speedmodes, and align these new names of DT bindings to those. > + > + > Example: > emmc: sdhci@5a00 { > compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc"; > @@ -29,4 +82,5 @@ Example: > mmc-ddr-1_8v; > mmc-hs200-1_8v; > mmc-hs400-1_8v; > + phy-input-delay-sd-hs = <0>; > }; > diff --git a/drivers/mmc/host/sdhci-cadence.c > b/drivers/mmc/host/sdhci-cadence.c > index c946e45..6c338d2 100644 > --- a/drivers/mmc/host/sdhci-cadence.c > +++ b/drivers/mmc/host/sdhci-cadence.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include "sdhci-pltfm.h" > > @@ -53,6 +54,9 @@ > #define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY 0x06 > #define SDHCI_CDNS_PHY_DLY_EMMC_SDR0x07 > #define SDHCI_CDNS_PHY_DLY_EMMC_DDR0x08 > +#define SDHCI_CDNS_PHY_DLY_SDCLK 0x0b > +#define SDHCI_CDNS_PHY_DLY_HSMMC 0x0c > +#define SDHCI_CDNS_PHY_DLY_STROBE 0x0d > > /* > * The