Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1603274 > unrolled thread

[v3 1/3] mmc: sdhci-cadence: Fix writing PHY delay

Back to article view | Back to linux.kernel

Contents

  [v3 1/3] mmc: sdhci-cadence: Fix writing PHY delay Piotr Sroka <piotrs@cadence.com> - 2017-03-17 13:40 +0100
    [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Piotr Sroka <piotrs@cadence.com> - 2017-03-17 14:20 +0100
      Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-17 18:30 +0100
        RE: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Piotr Sroka <piotrs@cadence.com> - 2017-03-20 10:00 +0100
          Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-21 03:00 +0100
            RE: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Piotr Sroka <piotrs@cadence.com> - 2017-03-21 09:10 +0100
      Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-17 18:40 +0100
    [v3 2/3] Documentation: bindings: add description of PHY delays for sdhci-cadence Piotr Sroka <piotrs@cadence.com> - 2017-03-17 14:20 +0100
      Re: [v3 2/3] Documentation: bindings: add description of PHY delays  for sdhci-cadence Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-17 18:40 +0100
        RE: [v3 2/3] Documentation: bindings: add description of PHY delays  for sdhci-cadence Piotr Sroka <piotrs@cadence.com> - 2017-03-20 10:10 +0100

#1603274 — [v3 1/3] mmc: sdhci-cadence: Fix writing PHY delay

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

[toc] | [next] | [standalone]

#1603299 — [v3 3/3] 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 <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 
---
 drivers/mmc/host/sdhci-cadence.c | 57 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index b2334ec..7728ce6 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
@@ -62,10 +66,33 @@
  */
 #define SDHCI_CDNS_MAX_TUNING_LOOP	40
 
+static const struct of_device_id sdhci_cdns_match[];
+
 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 +117,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 +267,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 +295,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

[toc] | [prev] | [next] | [standalone]

#1603475 — Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration

Hi Piotr,

2017-03-17 21:41 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>
> ---
> 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


I am OK with this approach
because I do not have a better idea.

Only two minor comments below.





> @@ -62,10 +66,33 @@
>   */
>  #define SDHCI_CDNS_MAX_TUNING_LOOP     40
>
> +static const struct of_device_id sdhci_cdns_match[];
> +

You forgot to remove this.



> @@ -227,6 +267,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 you add "dev", please use it for devm_clk_get, too.




-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1604255 — RE: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration

> -----Original Message-----
> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> Sent: 17 March, 2017 6:24 PM
> Subject: Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration
> 
> 
> 
> > @@ -62,10 +66,33 @@
> >   */
> >  #define SDHCI_CDNS_MAX_TUNING_LOOP     40
> >
> > +static const struct of_device_id sdhci_cdns_match[];
> > +
> 
> You forgot to remove this.
> 
You are right. Thanks.
 
> 
> > @@ -227,6 +267,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 you add "dev", please use it for devm_clk_get, too.
> 
I did on the first patch version but Ulf pointed that this is unrelated change and should be removed. 
Therefore I remove it. 

> 
> 
> --
> Best Regards
> Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1605250 — Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration

Hi Piotr,

2017-03-20 17:47 GMT+09:00 Piotr Sroka <piotrs@cadence.com>:
>
>>
>> > @@ -227,6 +267,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 you add "dev", please use it for devm_clk_get, too.
>>
> I did on the first patch version but Ulf pointed that this is unrelated change and should be removed.
> Therefore I remove it.

Ah, OK.

Could you consider to send a separate patch for this?




-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1605385 — RE: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration

Hi Masahiro,

> Hi Piotr,
> 
> 2017-03-20 17:47 GMT+09:00 Piotr Sroka <piotrs@cadence.com>:
> >
> >>
> >> > @@ -227,6 +267,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 you add "dev", please use it for devm_clk_get, too.
> >>
> > I did on the first patch version but Ulf pointed that this is unrelated change and should be removed.
> > Therefore I remove it.
> 
> Ah, OK.
> 
> Could you consider to send a separate patch for this?
> 
Ok. If v5 is needed I will add additional patch making this modification. If not I will prepare a spearate patch.


Best Regards
Piotr Sroka

[toc] | [prev] | [next] | [standalone]

#1603484 — Re: [v3 3/3] mmc: sdhci-cadence: Update PHY delay configuration

Hi Piotr,



Sorry, one more nit.


2017-03-17 21:41 GMT+09:00 Piotr Sroka <piotrs@cadence.com>:

> +
> +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, },
> +};
> +
> +


Only one blank line, please.




-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1603301 — [v3 2/3] Documentation: bindings: 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 <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
---
 .../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

[toc] | [prev] | [next] | [standalone]

#1603490 — Re: [v3 2/3] Documentation: bindings: add description of PHY delays for sdhci-cadence

Hi Piotr,

2017-03-17 21:40 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>


I think you have a chance for v4.

Next time, please follow Rob's suggestion

> For the subject: "dt-bindings: mmc: ..."





-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1604261 — RE: [v3 2/3] Documentation: bindings: add description of PHY delays for sdhci-cadence

> -----Original Message-----
> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> Sent: 17 March, 2017 6:30 PM
> Subject: Re: [v3 2/3] Documentation: bindings: add description of PHY delays for sdhci-cadence
> 
> Hi Piotr,
> 
> 2017-03-17 21:40 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>
> 
> 
> I think you have a chance for v4.
> 
> Next time, please follow Rob's suggestion
> 
> > For the subject: "dt-bindings: mmc: ..."
> 
> 
I missed it. Thanks. I will use proper prefix in the next version.

Best Regards
Piotr Sroka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web