* [v4 1/3] mmc: sdhci-cadence: Fix writing PHY delay @ 2017-03-20 11:12 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:12 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Piotr Sroka Add polling for ACK to be sure that data are written to PHY register. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - fix indent --- Changes for v3: - none --- Changes for v4: - none --- drivers/mmc/host/sdhci-cadence.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index 316cfec..b2334ec 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -66,11 +66,12 @@ struct sdhci_cdns_priv { void __iomem *hrs_addr; }; -static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, - u8 addr, u8 data) +static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, + u8 addr, u8 data) { void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS04; u32 tmp; + int ret; tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) | (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT); @@ -79,8 +80,14 @@ static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, tmp |= SDHCI_CDNS_HRS04_WR; writel(tmp, reg); + ret = readl_poll_timeout(reg, tmp, tmp & SDHCI_CDNS_HRS04_ACK, 0, 10); + if (ret) + return ret; + tmp &= ~SDHCI_CDNS_HRS04_WR; writel(tmp, reg); + + return 0; } static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [v4 1/3] mmc: sdhci-cadence: Fix writing PHY delay @ 2017-03-20 11:12 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:12 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Piotr Sroka Add polling for ACK to be sure that data are written to PHY register. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - fix indent --- Changes for v3: - none --- Changes for v4: - none --- drivers/mmc/host/sdhci-cadence.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index 316cfec..b2334ec 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -66,11 +66,12 @@ struct sdhci_cdns_priv { void __iomem *hrs_addr; }; -static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, - u8 addr, u8 data) +static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, + u8 addr, u8 data) { void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS04; u32 tmp; + int ret; tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) | (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT); @@ -79,8 +80,14 @@ static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, tmp |= SDHCI_CDNS_HRS04_WR; writel(tmp, reg); + ret = readl_poll_timeout(reg, tmp, tmp & SDHCI_CDNS_HRS04_ACK, 0, 10); + if (ret) + return ret; + tmp &= ~SDHCI_CDNS_HRS04_WR; writel(tmp, reg); + + return 0; } static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [v4 2/3] dt-bindings: mmc: add description of PHY delays for sdhci-cadence 2017-03-20 11:12 ` Piotr Sroka @ 2017-03-20 11:20 ` Piotr Sroka -1 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:20 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Rob Herring, Mark Rutland, devicetree, Piotr Sroka DTS properties are used instead of fixed data because PHY settings can be different for different chips/boards. Add description of new DLL PHY delays. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - file was created in v2. It was a part of driver source file patch. - most delays were moved from dts file to data associated with an SoC specific compatible - description of delays was updated to be more clearly --- Changes for v3: - move all delays back to dts because they are also boards dependent - prefix all of the Cadence-specific properties with cdns prefix --- Changes for v4: - change the beginning of the commit subject --- .../devicetree/bindings/mmc/sdhci-cadence.txt | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt index c0f37cb..c341820 100644 --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt @@ -19,6 +19,53 @@ if supported. See mmc.txt for details. - mmc-hs400-1_8v - mmc-hs400-1_2v +Some PHY delays can be configured by following properties. +PHY DLL input delays: +They are used to delay the data valid window, and align the window +to sampling clock. The delay starts from 5ns (for delay parameter equal to 0) +and it is increased by 2.5ns in each step. +- cdns,phy-input-delay-sd-highspeed: + Value of the delay in the input path for SD high-speed timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-legacy: + Value of the delay in the input path for SD legacy timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr12: + Value of the delay in the input path for SD UHS SDR12 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr25: + Value of the delay in the input path for SD UHS SDR25 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr50: + Value of the delay in the input path for SD UHS SDR50 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-ddr50: + Value of the delay in the input path for SD UHS DDR50 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-mmc-highspeed: + Value of the delay in the input path for MMC high-speed timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-mmc-ddr: + Value of the delay in the input path for eMMC high-speed DDR timing + Valid range = [0:0x1F]. + +PHY DLL clock delays: +Each delay property represents the fraction of the clock period. +The approximate delay value will be +(<delay property value>/128)*sdmclk_clock_period. +- cdns,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]. +- cdns,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]. +- cdns,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]. + Example: emmc: sdhci@5a000000 { compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc"; @@ -29,4 +76,5 @@ Example: mmc-ddr-1_8v; mmc-hs200-1_8v; mmc-hs400-1_8v; + cdns,phy-dll-delay-sdclk = <0>; }; -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [v4 2/3] dt-bindings: mmc: add description of PHY delays for sdhci-cadence @ 2017-03-20 11:20 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:20 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Rob Herring, Mark Rutland, devicetree, Piotr Sroka DTS properties are used instead of fixed data because PHY settings can be different for different chips/boards. Add description of new DLL PHY delays. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - file was created in v2. It was a part of driver source file patch. - most delays were moved from dts file to data associated with an SoC specific compatible - description of delays was updated to be more clearly --- Changes for v3: - move all delays back to dts because they are also boards dependent - prefix all of the Cadence-specific properties with cdns prefix --- Changes for v4: - change the beginning of the commit subject --- .../devicetree/bindings/mmc/sdhci-cadence.txt | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt index c0f37cb..c341820 100644 --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt @@ -19,6 +19,53 @@ if supported. See mmc.txt for details. - mmc-hs400-1_8v - mmc-hs400-1_2v +Some PHY delays can be configured by following properties. +PHY DLL input delays: +They are used to delay the data valid window, and align the window +to sampling clock. The delay starts from 5ns (for delay parameter equal to 0) +and it is increased by 2.5ns in each step. +- cdns,phy-input-delay-sd-highspeed: + Value of the delay in the input path for SD high-speed timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-legacy: + Value of the delay in the input path for SD legacy timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr12: + Value of the delay in the input path for SD UHS SDR12 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr25: + Value of the delay in the input path for SD UHS SDR25 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-sdr50: + Value of the delay in the input path for SD UHS SDR50 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-sd-uhs-ddr50: + Value of the delay in the input path for SD UHS DDR50 timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-mmc-highspeed: + Value of the delay in the input path for MMC high-speed timing + Valid range = [0:0x1F]. +- cdns,phy-input-delay-mmc-ddr: + Value of the delay in the input path for eMMC high-speed DDR timing + Valid range = [0:0x1F]. + +PHY DLL clock delays: +Each delay property represents the fraction of the clock period. +The approximate delay value will be +(<delay property value>/128)*sdmclk_clock_period. +- cdns,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]. +- cdns,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]. +- cdns,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]. + Example: emmc: sdhci@5a000000 { compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc"; @@ -29,4 +76,5 @@ Example: mmc-ddr-1_8v; mmc-hs200-1_8v; mmc-hs400-1_8v; + cdns,phy-dll-delay-sdclk = <0>; }; -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v4 2/3] dt-bindings: mmc: add description of PHY delays for sdhci-cadence 2017-03-20 11:20 ` Piotr Sroka (?) @ 2017-03-21 7:40 ` Masahiro Yamada -1 siblings, 0 replies; 14+ messages in thread From: Masahiro Yamada @ 2017-03-21 7:40 UTC (permalink / raw) To: Piotr Sroka Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List, Rob Herring, Mark Rutland, devicetree 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > DTS properties are used instead of fixed data > because PHY settings can be different for different chips/boards. > Add description of new DLL PHY delays. > > Signed-off-by: Piotr Sroka <piotrs@cadence.com> > --- > Changes for v2: > - file was created in v2. It was a part of driver source file patch. > - most delays were moved from dts file > to data associated with an SoC specific compatible > - description of delays was updated to be more clearly > --- > Changes for v3: > - move all delays back to dts because they are also boards dependent > - prefix all of the Cadence-specific properties with cdns prefix > --- > Changes for v4: > - change the beginning of the commit subject > --- Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com> -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-20 11:12 ` Piotr Sroka @ 2017-03-20 11:20 ` Piotr Sroka -1 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:20 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Piotr Sroka DTS properties are used instead of fixed data because PHY settings can be different for different chips/boards. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - dts part was removed from this patch - most delays were moved from dts file to data associated with an SoC specific compatible - remove unrelated changes --- Changes for v3: - move all delays back to dts because they are also boards dependent - prefix all of the Cadence-specific properties with cdns prefix - put checking delay properties inside the for loop instead of using a lot of single if expressions --- Changes for v4: - remove unecessary declaration of sdhci_cdns_match - format fix (blank line removed) --- drivers/mmc/host/sdhci-cadence.c | 53 ++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index b2334ec..308a372 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -18,6 +18,7 @@ #include <linux/module.h> #include <linux/mmc/host.h> #include <linux/mmc/mmc.h> +#include <linux/of.h> #include "sdhci-pltfm.h" @@ -54,6 +55,9 @@ #define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY 0x06 #define SDHCI_CDNS_PHY_DLY_EMMC_SDR 0x07 #define SDHCI_CDNS_PHY_DLY_EMMC_DDR 0x08 +#define SDHCI_CDNS_PHY_DLY_SDCLK 0x0b +#define SDHCI_CDNS_PHY_DLY_HSMMC 0x0c +#define SDHCI_CDNS_PHY_DLY_STROBE 0x0d /* * The tuned val register is 6 bit-wide, but not the whole of the range is @@ -66,6 +70,25 @@ struct sdhci_cdns_priv { void __iomem *hrs_addr; }; +struct sdhci_cdns_phy_cfg { + const char *property; + u8 addr; +}; + +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, +}; + static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, u8 addr, u8 data) { @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, return 0; } -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) +static int sdhci_cdns_phy_init(struct device_node *np, + struct sdhci_cdns_priv *priv) { - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); + u32 val; + int ret, i; + + for (i = 0; i < ARRAY_SIZE(sdhci_cdns_phy_cfgs); i++) { + ret = of_property_read_u32(np, sdhci_cdns_phy_cfgs[i].property, + &val); + if (ret) + continue; + + ret = sdhci_cdns_write_phy_reg(priv, + sdhci_cdns_phy_cfgs[i].addr, + val); + if (ret) + return ret; + } + + return 0; } static inline void *sdhci_cdns_priv(struct sdhci_host *host) @@ -227,6 +263,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev) struct sdhci_cdns_priv *priv; struct clk *clk; int ret; + struct device *dev = &pdev->dev; clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(clk)) @@ -254,7 +291,9 @@ static int sdhci_cdns_probe(struct platform_device *pdev) if (ret) goto free; - sdhci_cdns_phy_init(priv); + ret = sdhci_cdns_phy_init(dev->of_node, priv); + if (ret) + goto free; ret = sdhci_add_host(host); if (ret) -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration @ 2017-03-20 11:20 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-20 11:20 UTC (permalink / raw) To: linux-mmc Cc: Adrian Hunter, Ulf Hansson, linux-kernel, Masahiro Yamada, Piotr Sroka DTS properties are used instead of fixed data because PHY settings can be different for different chips/boards. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- Changes for v2: - dts part was removed from this patch - most delays were moved from dts file to data associated with an SoC specific compatible - remove unrelated changes --- Changes for v3: - move all delays back to dts because they are also boards dependent - prefix all of the Cadence-specific properties with cdns prefix - put checking delay properties inside the for loop instead of using a lot of single if expressions --- Changes for v4: - remove unecessary declaration of sdhci_cdns_match - format fix (blank line removed) --- drivers/mmc/host/sdhci-cadence.c | 53 ++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index b2334ec..308a372 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -18,6 +18,7 @@ #include <linux/module.h> #include <linux/mmc/host.h> #include <linux/mmc/mmc.h> +#include <linux/of.h> #include "sdhci-pltfm.h" @@ -54,6 +55,9 @@ #define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY 0x06 #define SDHCI_CDNS_PHY_DLY_EMMC_SDR 0x07 #define SDHCI_CDNS_PHY_DLY_EMMC_DDR 0x08 +#define SDHCI_CDNS_PHY_DLY_SDCLK 0x0b +#define SDHCI_CDNS_PHY_DLY_HSMMC 0x0c +#define SDHCI_CDNS_PHY_DLY_STROBE 0x0d /* * The tuned val register is 6 bit-wide, but not the whole of the range is @@ -66,6 +70,25 @@ struct sdhci_cdns_priv { void __iomem *hrs_addr; }; +struct sdhci_cdns_phy_cfg { + const char *property; + u8 addr; +}; + +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, +}; + static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, u8 addr, u8 data) { @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, return 0; } -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) +static int sdhci_cdns_phy_init(struct device_node *np, + struct sdhci_cdns_priv *priv) { - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); + u32 val; + int ret, i; + + for (i = 0; i < ARRAY_SIZE(sdhci_cdns_phy_cfgs); i++) { + ret = of_property_read_u32(np, sdhci_cdns_phy_cfgs[i].property, + &val); + if (ret) + continue; + + ret = sdhci_cdns_write_phy_reg(priv, + sdhci_cdns_phy_cfgs[i].addr, + val); + if (ret) + return ret; + } + + return 0; } static inline void *sdhci_cdns_priv(struct sdhci_host *host) @@ -227,6 +263,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev) struct sdhci_cdns_priv *priv; struct clk *clk; int ret; + struct device *dev = &pdev->dev; clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(clk)) @@ -254,7 +291,9 @@ static int sdhci_cdns_probe(struct platform_device *pdev) if (ret) goto free; - sdhci_cdns_phy_init(priv); + ret = sdhci_cdns_phy_init(dev->of_node, priv); + if (ret) + goto free; ret = sdhci_add_host(host); if (ret) -- 2.2.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-20 11:20 ` Piotr Sroka (?) @ 2017-03-21 1:45 ` Masahiro Yamada 2017-03-21 7:01 ` Piotr Sroka -1 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2017-03-21 1:45 UTC (permalink / raw) To: Piotr Sroka Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List Hi Piotr, 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > DTS properties are used instead of fixed data > because PHY settings can be different for different chips/boards. > > Signed-off-by: Piotr Sroka <piotrs@cadence.com> I found this version is a problem for me. > + > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, > +}; > + I see mmc-legacy property in v1, but it is missing now. > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > u8 addr, u8 data) > { > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > return 0; > } > > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) > +static int sdhci_cdns_phy_init(struct device_node *np, > + struct sdhci_cdns_priv *priv) > { > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. Maybe, do we need a DT property for this, too? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-21 1:45 ` Masahiro Yamada @ 2017-03-21 7:01 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-21 7:01 UTC (permalink / raw) To: Piotr Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List, Piotr Hi Masahiro 2017-03-21 02:46 AM Masahiro Yamada <yamada.masahiro@socionext.com>: > Hi Piotr, > > > 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > > DTS properties are used instead of fixed data > > because PHY settings can be different for different chips/boards. > > > > Signed-off-by: Piotr Sroka <piotrs@cadence.com> > > > I found this version is a problem for me. > > > > + > > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { > > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, > > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, > > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, > > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, > > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, > > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, > > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, > > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, > > +}; > > + > > > I see mmc-legacy property in v1, > but it is missing now. > > > > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > > u8 addr, u8 data) > > { > > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > > return 0; > > } > > > > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) > > +static int sdhci_cdns_phy_init(struct device_node *np, > > + struct sdhci_cdns_priv *priv) > > { > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); > > > I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. > > Maybe, do we need a DT property for this, too? > I can add it but could you check if you realy need it? There is no selection MMC legacy mode in this driver. Best Regards Piotr Sroka ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration @ 2017-03-21 7:01 ` Piotr Sroka 0 siblings, 0 replies; 14+ messages in thread From: Piotr Sroka @ 2017-03-21 7:01 UTC (permalink / raw) Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List, Piotr Hi Masahiro 2017-03-21 02:46 AM Masahiro Yamada <yamada.masahiro@socionext.com>: > Hi Piotr, > > > 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > > DTS properties are used instead of fixed data > > because PHY settings can be different for different chips/boards. > > > > Signed-off-by: Piotr Sroka <piotrs@cadence.com> > > > I found this version is a problem for me. > > > > + > > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { > > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, > > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, > > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, > > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, > > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, > > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, > > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, > > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, > > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, > > +}; > > + > > > I see mmc-legacy property in v1, > but it is missing now. > > > > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > > u8 addr, u8 data) > > { > > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > > return 0; > > } > > > > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) > > +static int sdhci_cdns_phy_init(struct device_node *np, > > + struct sdhci_cdns_priv *priv) > > { > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); > > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); > > > I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. > > Maybe, do we need a DT property for this, too? > I can add it but could you check if you realy need it? There is no selection MMC legacy mode in this driver. Best Regards Piotr Sroka ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-21 7:01 ` Piotr Sroka (?) @ 2017-03-21 7:32 ` Masahiro Yamada 2017-03-22 7:08 ` Piotr Sroka -1 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2017-03-21 7:32 UTC (permalink / raw) To: Piotr Sroka Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List Hi Piotr, 2017-03-21 16:01 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > > Hi Masahiro > > 2017-03-21 02:46 AM Masahiro Yamada <yamada.masahiro@socionext.com>: >> Hi Piotr, >> >> >> 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: >> > DTS properties are used instead of fixed data >> > because PHY settings can be different for different chips/boards. >> > >> > Signed-off-by: Piotr Sroka <piotrs@cadence.com> >> >> >> I found this version is a problem for me. >> >> >> > + >> > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { >> > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, >> > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, >> > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, >> > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, >> > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, >> > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, >> > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, >> > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, >> > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, >> > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, >> > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, >> > +}; >> > + >> >> >> I see mmc-legacy property in v1, >> but it is missing now. >> >> >> > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, >> > u8 addr, u8 data) >> > { >> > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, >> > return 0; >> > } >> > >> > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) >> > +static int sdhci_cdns_phy_init(struct device_node *np, >> > + struct sdhci_cdns_priv *priv) >> > { >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); >> >> >> I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. >> >> Maybe, do we need a DT property for this, too? >> > I can add it but could you check if you realy need it? There is no selection MMC legacy mode > in this driver. > Ah, you are right. For Linux, SD-Legacy and MMC-Legacy share the same timing flag. The legacy mode is covered by SDHCI_CDNS_PHY_DLY_SD_DEFAULT, so we need not to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY. So, I have no problem with patch. Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com> >From here, just my minor comments. The binding should not be too Linux-oriented. Device Tree is hardware description language, and project-neutral from its concept. I wonder if there is a project that handles SD-Legacy and MMC-Legacy separately. I am not sure about this, and I do not have a strong opinion, either. I leave this to you (and other developers). -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-21 7:32 ` Masahiro Yamada @ 2017-03-22 7:08 ` Piotr Sroka 2017-03-22 7:24 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Piotr Sroka @ 2017-03-22 7:08 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List, Piotr Hi Masahiro 2017-03-21 08:3 AM Masahiro Yamada <yamada.masahiro@socionext.com>: > Hi Piotr, > > 2017-03-21 16:01 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > > > > Hi Masahiro > > > > 2017-03-21 02:46 AM Masahiro Yamada <yamada.masahiro@socionext.com>: > >> Hi Piotr, > >> > >> > >> 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > >> > DTS properties are used instead of fixed data > >> > because PHY settings can be different for different chips/boards. > >> > > >> > Signed-off-by: Piotr Sroka <piotrs@cadence.com> > >> > >> > >> I found this version is a problem for me. > >> > >> > >> > + > >> > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { > >> > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, > >> > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, > >> > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, > >> > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, > >> > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, > >> > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, > >> > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, > >> > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, > >> > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, > >> > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, > >> > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, > >> > +}; > >> > + > >> > >> > >> I see mmc-legacy property in v1, > >> but it is missing now. > >> > >> > >> > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > >> > u8 addr, u8 data) > >> > { > >> > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > >> > return 0; > >> > } > >> > > >> > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) > >> > +static int sdhci_cdns_phy_init(struct device_node *np, > >> > + struct sdhci_cdns_priv *priv) > >> > { > >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); > >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); > >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); > >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); > >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); > >> > >> > >> I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. > >> > >> Maybe, do we need a DT property for this, too? > >> > > I can add it but could you check if you realy need it? There is no selection MMC legacy mode > > in this driver. > > > > Ah, you are right. > > For Linux, SD-Legacy and MMC-Legacy share the same timing flag. > The legacy mode is covered by SDHCI_CDNS_PHY_DLY_SD_DEFAULT, > so we need not to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY. > > So, I have no problem with patch. > > Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > > > > From here, just my minor comments. > The binding should not be too Linux-oriented. > Device Tree is hardware description language, and project-neutral from > its concept. > I wonder if there is a project that handles SD-Legacy and MMC-Legacy separately. > I am not sure about this, and I do not have a strong opinion, either. > > I leave this to you (and other developers). > Thanks for your comments. Because patch adding HS400 enhanced strobe support was applied for next branch I needed to create v5. BTW I added one extra patch modifing probe function as you suggested. I also modify binding to be consistent with linux MMC timing names. The driver does not use MMC legacy at all so I think there is no need to distinguish between SD Legacy and MMC Legacy. So there are no contraindications for being consistent with Linux. Best Regards Piotr Sroka ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration 2017-03-22 7:08 ` Piotr Sroka @ 2017-03-22 7:24 ` Masahiro Yamada 0 siblings, 0 replies; 14+ messages in thread From: Masahiro Yamada @ 2017-03-22 7:24 UTC (permalink / raw) To: Piotr Sroka Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List 2017-03-22 16:08 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > Hi Masahiro > > 2017-03-21 08:3 AM Masahiro Yamada <yamada.masahiro@socionext.com>: >> Hi Piotr, >> >> 2017-03-21 16:01 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: >> > >> > Hi Masahiro >> > >> > 2017-03-21 02:46 AM Masahiro Yamada <yamada.masahiro@socionext.com>: >> >> Hi Piotr, >> >> >> >> >> >> 2017-03-20 20:20 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: >> >> > DTS properties are used instead of fixed data >> >> > because PHY settings can be different for different chips/boards. >> >> > >> >> > Signed-off-by: Piotr Sroka <piotrs@cadence.com> >> >> >> >> >> >> I found this version is a problem for me. >> >> >> >> >> >> > + >> >> > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = { >> >> > + { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, }, >> >> > + { "cdns,phy-input-delay-sd-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, }, >> >> > + { "cdns,phy-input-delay-sd-uhs-sdr12", SDHCI_CDNS_PHY_DLY_UHS_SDR12, }, >> >> > + { "cdns,phy-input-delay-sd-uhs-sdr25", SDHCI_CDNS_PHY_DLY_UHS_SDR25, }, >> >> > + { "cdns,phy-input-delay-sd-uhs-sdr50", SDHCI_CDNS_PHY_DLY_UHS_SDR50, }, >> >> > + { "cdns,phy-input-delay-sd-uhs-ddr50", SDHCI_CDNS_PHY_DLY_UHS_DDR50, }, >> >> > + { "cdns,phy-input-delay-mmc-highspeed", SDHCI_CDNS_PHY_DLY_EMMC_SDR, }, >> >> > + { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, }, >> >> > + { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, }, >> >> > + { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, }, >> >> > + { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, }, >> >> > +}; >> >> > + >> >> >> >> >> >> I see mmc-legacy property in v1, >> >> but it is missing now. >> >> >> >> >> >> > static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, >> >> > u8 addr, u8 data) >> >> > { >> >> > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, >> >> > return 0; >> >> > } >> >> > >> >> > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) >> >> > +static int sdhci_cdns_phy_init(struct device_node *np, >> >> > + struct sdhci_cdns_priv *priv) >> >> > { >> >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4); >> >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4); >> >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9); >> >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2); >> >> > - sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3); >> >> >> >> >> >> I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC. >> >> >> >> Maybe, do we need a DT property for this, too? >> >> >> > I can add it but could you check if you realy need it? There is no selection MMC legacy mode >> > in this driver. >> > >> >> Ah, you are right. >> >> For Linux, SD-Legacy and MMC-Legacy share the same timing flag. >> The legacy mode is covered by SDHCI_CDNS_PHY_DLY_SD_DEFAULT, >> so we need not to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY. >> >> So, I have no problem with patch. >> >> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> >> >> >> >> From here, just my minor comments. >> The binding should not be too Linux-oriented. >> Device Tree is hardware description language, and project-neutral from >> its concept. >> I wonder if there is a project that handles SD-Legacy and MMC-Legacy separately. >> I am not sure about this, and I do not have a strong opinion, either. >> >> I leave this to you (and other developers). >> > > Thanks for your comments. > > Because patch adding HS400 enhanced strobe support was applied > for next branch I needed to create v5. BTW I added one extra patch modifing > probe function as you suggested. > I also modify binding to be consistent with linux MMC timing names. > The driver does not use MMC legacy at all so I think there is no need > to distinguish between SD Legacy and MMC Legacy. So there are no contraindications > for being consistent with Linux. OK. Make sense. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4 1/3] mmc: sdhci-cadence: Fix writing PHY delay 2017-03-20 11:12 ` Piotr Sroka ` (2 preceding siblings ...) (?) @ 2017-03-21 7:39 ` Masahiro Yamada -1 siblings, 0 replies; 14+ messages in thread From: Masahiro Yamada @ 2017-03-21 7:39 UTC (permalink / raw) To: Piotr Sroka Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Linux Kernel Mailing List Hi Piotr, 2017-03-20 20:12 GMT+09:00 Piotr Sroka <piotrs@cadence.com>: > Add polling for ACK to be sure that data are written to PHY register. > > Signed-off-by: Piotr Sroka <piotrs@cadence.com> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com> One you get Acked-by or Reviewed-by, please make sure to add it in the next version. I issued my Reviewed-by multiple times (because maintainers usually pick up the latest version). -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-03-22 7:25 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-20 11:12 [v4 1/3] mmc: sdhci-cadence: Fix writing PHY delay Piotr Sroka 2017-03-20 11:12 ` Piotr Sroka 2017-03-20 11:20 ` [v4 2/3] dt-bindings: mmc: add description of PHY delays for sdhci-cadence Piotr Sroka 2017-03-20 11:20 ` Piotr Sroka 2017-03-21 7:40 ` Masahiro Yamada 2017-03-20 11:20 ` [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration Piotr Sroka 2017-03-20 11:20 ` Piotr Sroka 2017-03-21 1:45 ` Masahiro Yamada 2017-03-21 7:01 ` Piotr Sroka 2017-03-21 7:01 ` Piotr Sroka 2017-03-21 7:32 ` Masahiro Yamada 2017-03-22 7:08 ` Piotr Sroka 2017-03-22 7:24 ` Masahiro Yamada 2017-03-21 7:39 ` [v4 1/3] mmc: sdhci-cadence: Fix writing PHY delay Masahiro Yamada
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.