LinuxLists.cc - [v5 1/4] mmc: sdhci-cadence: Fix writing PHY delay

[permalink] [raw]

Subject: [v5 3/4] mmc: sdhci-cadence: Update PHY delay configuration

DTS properties are used instead of fixed data
because PHY settings can be different for different chips/boards.

Signed-off-by: Piotr Sroka <[email protected]>
---
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)
---
Changes for v5:
- use driver version from next branch, with applied enhanced strobe feature support.
- change name of property to be consistent with timing modes
available in Linux
---
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 83c3b55..c3c7090 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"

@@ -55,6 +56,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
@@ -68,6 +72,25 @@ struct sdhci_cdns_priv {
bool enhanced_strobe;
};

+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-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)
{
@@ -92,13 +115,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)
@@ -267,6 +303,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))
@@ -297,7 +334,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

2017-03-21 14:33:35

[permalink] [raw]

Subject: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

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 <[email protected]>
---
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
---
Changes for v5:
- change name of property to be consistent with timing modes
available in Linux
---
.../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..fa423c2 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 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

2017-03-21 14:33:53

[permalink] [raw]

Subject: [v5 4/4] mmc: sdhci-cadence: refactor probe function

Use added dev variable for devm_clk_get.

Signed-off-by: Piotr Sroka <[email protected]>
---
Changes for v5:
- patch created in v5
---
drivers/mmc/host/sdhci-cadence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index c3c7090..5aa238d 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -305,7 +305,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
int ret;
struct device *dev = &pdev->dev;

- clk = devm_clk_get(&pdev->dev, NULL);
+ clk = devm_clk_get(dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);

--
2.2.2

2017-03-22 07:30:22

[permalink] [raw]

Subject: Re: [v5 1/4] mmc: sdhci-cadence: Fix writing PHY delay

2017-03-21 23:32 GMT+09:00 Piotr Sroka <[email protected]>:
> Add polling for ACK to be sure that data are written to PHY register.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Reviewed-by: Masahiro Yamada <[email protected]>

--
Best Regards
Masahiro Yamada

2017-03-22 07:31:59

[permalink] [raw]

Subject: Re: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

2017-03-21 23:33 GMT+09:00 Piotr Sroka <[email protected]>:
> 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 <[email protected]>
> ---
> 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
> ---
> Changes for v5:
> - change name of property to be consistent with timing modes
> available in Linux

As I gave Reviewed-by in v4 already, this looks good to me.

Reviewed-by: Masahiro Yamada <[email protected]>

As I said before,
once you get Reviewed/Acked tags,
please include them in your later version.

--
Best Regards
Masahiro Yamada

2017-03-22 07:32:29

[permalink] [raw]

Subject: Re: [v5 4/4] mmc: sdhci-cadence: refactor probe function

2017-03-21 23:33 GMT+09:00 Piotr Sroka <[email protected]>:
> Use added dev variable for devm_clk_get.
>
> Signed-off-by: Piotr Sroka <[email protected]>
> ---
> Changes for v5:
> - patch created in v5
> ---

Reviewed-by: Masahiro Yamada <[email protected]>

--
Best Regards
Masahiro Yamada

2017-03-22 07:32:39

[permalink] [raw]

Subject: Re: [v5 3/4] mmc: sdhci-cadence: Update PHY delay configuration

2017-03-21 23:33 GMT+09:00 Piotr Sroka <[email protected]>:
> DTS properties are used instead of fixed data
> because PHY settings can be different for different chips/boards.
>
> Signed-off-by: Piotr Sroka <[email protected]>
> ---
> 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)
> ---
> Changes for v5:
> - use driver version from next branch, with applied enhanced strobe feature support.
> - change name of property to be consistent with timing modes
> available in Linux
> ---

Reviewed-by: Masahiro Yamada <[email protected]>

--
Best Regards
Masahiro Yamada

2017-03-24 16:26:31

by Rob Herring (Arm)

[permalink] [raw]

Subject: Re: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

On Tue, Mar 21, 2017 at 02:33:01PM +0000, Piotr Sroka wrote:
> 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 <[email protected]>
> ---
> 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
> ---
> Changes for v5:
> - change name of property to be consistent with timing modes
> available in Linux

I don't see any change here...

> ---
> .../devicetree/bindings/mmc/sdhci-cadence.txt | 48 ++++++++++++++++++++++
> 1 file changed, 48 insertions(+)

Acked-by: Rob Herring <[email protected]>

2017-03-29 13:08:38

by Adrian Hunter

[permalink] [raw]

Subject: Re: [v5 1/4] mmc: sdhci-cadence: Fix writing PHY delay

On 21/03/17 16:32, Piotr Sroka wrote:
> Add polling for ACK to be sure that data are written to PHY register.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> Changes for v2:
> - fix indent
> ---
> Changes for v3:
> - none
> ---
> Changes for v4:
> - none
> ---
> Changes for v5:
> - use driver version from next branch, with applied enhanced strobe feature support.
> ---
> 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 48f6419..83c3b55 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -68,11 +68,12 @@ struct sdhci_cdns_priv {
> bool enhanced_strobe;
> };
>
> -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);
> @@ -81,8 +82,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)
>

2017-03-29 13:09:26

by Adrian Hunter

[permalink] [raw]

Subject: Re: [v5 3/4] mmc: sdhci-cadence: Update PHY delay configuration

On 21/03/17 16:33, Piotr Sroka wrote:
> DTS properties are used instead of fixed data
> because PHY settings can be different for different chips/boards.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> 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)
> ---
> Changes for v5:
> - use driver version from next branch, with applied enhanced strobe feature support.
> - change name of property to be consistent with timing modes
> available in Linux
> ---
> 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 83c3b55..c3c7090 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"
>
> @@ -55,6 +56,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
> @@ -68,6 +72,25 @@ struct sdhci_cdns_priv {
> bool enhanced_strobe;
> };
>
> +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-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)
> {
> @@ -92,13 +115,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)
> @@ -267,6 +303,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))
> @@ -297,7 +334,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)
>

2017-03-29 13:11:31

by Adrian Hunter

[permalink] [raw]

Subject: Re: [v5 4/4] mmc: sdhci-cadence: refactor probe function

On 21/03/17 16:33, Piotr Sroka wrote:
> Use added dev variable for devm_clk_get.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> Changes for v5:
> - patch created in v5
> ---
> drivers/mmc/host/sdhci-cadence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index c3c7090..5aa238d 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -305,7 +305,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
> int ret;
> struct device *dev = &pdev->dev;
>
> - clk = devm_clk_get(&pdev->dev, NULL);
> + clk = devm_clk_get(dev, NULL);
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
>

2017-03-30 11:06:55

[permalink] [raw]

Subject: RE: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

Hi Masahiro

2017-03-22 16:30 Masahiro Yamada <[email protected]>:
> 2017-03-21 23:33 GMT+09:00 Piotr Sroka <[email protected]>:
> > 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 <[email protected]>
> > ---
> > 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
> > ---
> > Changes for v5:
> > - change name of property to be consistent with timing modes
> > available in Linux
>
>
> As I gave Reviewed-by in v4 already, this looks good to me.
>
>
> Reviewed-by: Masahiro Yamada <[email protected]>
>
>
> As I said before,
> once you get Reviewed/Acked tags,
> please include them in your later version.
>

Ok. I will remember next time.

--
Best Regards
Piotr Sroka

2017-03-30 19:30:46

[permalink] [raw]

Subject: Re: [v5 1/4] mmc: sdhci-cadence: Fix writing PHY delay

On 21 March 2017 at 15:32, Piotr Sroka <[email protected]> wrote:
> Add polling for ACK to be sure that data are written to PHY register.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Thanks, applied for next!

Kind regards
Uffe

> ---
> Changes for v2:
> - fix indent
> ---
> Changes for v3:
> - none
> ---
> Changes for v4:
> - none
> ---
> Changes for v5:
> - use driver version from next branch, with applied enhanced strobe feature support.
> ---
> 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 48f6419..83c3b55 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -68,11 +68,12 @@ struct sdhci_cdns_priv {
> bool enhanced_strobe;
> };
>
> -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);
> @@ -81,8 +82,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
>

2017-03-30 19:30:59

[permalink] [raw]

Subject: Re: [v5 4/4] mmc: sdhci-cadence: refactor probe function

On 21 March 2017 at 15:33, Piotr Sroka <[email protected]> wrote:
> Use added dev variable for devm_clk_get.
>
> Signed-off-by: Piotr Sroka <[email protected]>

Thanks, applied for next!

Kind regards
Uffe

> ---
> Changes for v5:
> - patch created in v5
> ---
> drivers/mmc/host/sdhci-cadence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index c3c7090..5aa238d 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -305,7 +305,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
> int ret;
> struct device *dev = &pdev->dev;
>
> - clk = devm_clk_get(&pdev->dev, NULL);
> + clk = devm_clk_get(dev, NULL);
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
> --
> 2.2.2
>

2017-03-30 19:31:21

[permalink] [raw]

Subject: Re: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

On 21 March 2017 at 15:33, Piotr Sroka <[email protected]> wrote:
> 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 <[email protected]>

Thanks, applied for next!

Kind regards
Uffe

> ---
> 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
> ---
> Changes for v5:
> - change name of property to be consistent with timing modes
> available in Linux
> ---
> .../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..fa423c2 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 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
>

2017-03-30 19:32:31