diff mbox series

[PATCHv3,1/3] mmc: meson-gx: Fix clk phase tuning for MMC

Message ID 20191226113353.1757-2-linux.amoon@gmail.com
State Accepted, archived
Delegated to: Neil Armstrong
Headers show
Series Odroid n2 using eMMC would fail to boot up | expand

Commit Message

Anand Moon Dec. 26, 2019, 11:33 a.m. UTC 
As per mainline line kernel fix the clk tunnig phase for
mmc, set Core=180, Tx=0, Rx=0 clk phase for mmc initialization.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
Changes from previous
v2: Fix the clk phase macro to support PHASE_180
    drop the wrong CLK_CORE_PHASE_MASK macro.

v1: use the mainline kernel tuning for clk tuning.
Fixed the commmit messages.
Patch v1:
https://patchwork.ozlabs.org/patch/1201208/

Before these changes.
    clock is enabled (380953Hz)
    clock is enabled (25000000Hz)
After these changes
    clock is enabled (380953Hz)
    clock is enabled (25000000Hz)
    clock is enabled (52000000Hz)
Test on Odroid N2 and Odroid C2 with eMMC and microSD cards
---
 arch/arm/include/asm/arch-meson/sd_emmc.h | 14 ++++++--------
 drivers/mmc/meson_gx_mmc.c                |  9 +++++----
 2 files changed, 11 insertions(+), 12 deletions(-)

Comments

Jerome Brunet Jan. 7, 2020, 10 a.m. UTC | #1 
On Thu 26 Dec 2019 at 12:33, Anand Moon <linux.amoon@gmail.com> wrote:

> As per mainline line kernel fix the clk tunnig phase for
> mmc, set Core=180, Tx=0, Rx=0 clk phase for mmc initialization.
>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> Changes from previous
> v2: Fix the clk phase macro to support PHASE_180
>     drop the wrong CLK_CORE_PHASE_MASK macro.
>
> v1: use the mainline kernel tuning for clk tuning.
> Fixed the commmit messages.
> Patch v1:
> https://patchwork.ozlabs.org/patch/1201208/
>
> Before these changes.
>     clock is enabled (380953Hz)
>     clock is enabled (25000000Hz)
> After these changes
>     clock is enabled (380953Hz)
>     clock is enabled (25000000Hz)
>     clock is enabled (52000000Hz)
> Test on Odroid N2 and Odroid C2 with eMMC and microSD cards
> ---
>  arch/arm/include/asm/arch-meson/sd_emmc.h | 14 ++++++--------
>  drivers/mmc/meson_gx_mmc.c                |  9 +++++----
>  2 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
> index e3a72c8b66..ee20c009e2 100644
> --- a/arch/arm/include/asm/arch-meson/sd_emmc.h
> +++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
> @@ -7,6 +7,7 @@
>  #define __SD_EMMC_H__
>  
>  #include <mmc.h>
> +#include <linux/bitops.h>
>  
>  #define SDIO_PORT_A			0
>  #define SDIO_PORT_B			1
> @@ -19,14 +20,11 @@
>  #define   CLK_MAX_DIV			63
>  #define   CLK_SRC_24M			(0 << 6)
>  #define   CLK_SRC_DIV2			(1 << 6)
> -#define   CLK_CO_PHASE_000		(0 << 8)
> -#define   CLK_CO_PHASE_090		(1 << 8)
> -#define   CLK_CO_PHASE_180		(2 << 8)
> -#define   CLK_CO_PHASE_270		(3 << 8)
> -#define   CLK_TX_PHASE_000		(0 << 10)
> -#define   CLK_TX_PHASE_090		(1 << 10)
> -#define   CLK_TX_PHASE_180		(2 << 10)
> -#define   CLK_TX_PHASE_270		(3 << 10)
> +
> +#define   CLK_PHASE_180 		2
> +#define   CLK_TX_PHASE_MASK		GENMASK(11, 10)
> +#define   CLK_RX_PHASE_MASK		GENMASK(13, 12)
> +
>  #define   CLK_ALWAYS_ON			BIT(24)
>  
>  #define MESON_SD_EMMC_CFG		0x44
> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
> index 86c1a7164a..ad697d3a5e 100644
> --- a/drivers/mmc/meson_gx_mmc.c
> +++ b/drivers/mmc/meson_gx_mmc.c
> @@ -52,10 +52,11 @@ static void meson_mmc_config_clock(struct mmc *mmc)
>  	clk_div = DIV_ROUND_UP(clk, mmc->clock);
>  
>  	/* 180 phase core clock */
> -	meson_mmc_clk |= CLK_CO_PHASE_180;
> -
> -	/* 180 phase tx clock */
> -	meson_mmc_clk |= CLK_TX_PHASE_000;
> +	meson_mmc_clk |= CLK_PHASE_180;
> +	/* 000 phase rx clock */
> +	meson_mmc_clk |= CLK_RX_PHASE_MASK;
> +	/* 000 phase tx clock */
> +	meson_mmc_clk |= CLK_TX_PHASE_MASK;

The comment on your previous version seemed correct but I think what you
have implemented here is still not doing what you expect.

>  
>  	/* clock settings */
>  	meson_mmc_clk |= clk_src;
Anand Moon Jan. 11, 2020, 12:23 p.m. UTC | #2 
Hi Jerome,

On Tue, 7 Jan 2020 at 15:31, Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Thu 26 Dec 2019 at 12:33, Anand Moon <linux.amoon@gmail.com> wrote:
>
> > As per mainline line kernel fix the clk tunnig phase for
> > mmc, set Core=180, Tx=0, Rx=0 clk phase for mmc initialization.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > Changes from previous
> > v2: Fix the clk phase macro to support PHASE_180
> >     drop the wrong CLK_CORE_PHASE_MASK macro.
> >
> > v1: use the mainline kernel tuning for clk tuning.
> > Fixed the commmit messages.
> > Patch v1:
> > https://patchwork.ozlabs.org/patch/1201208/
> >
> > Before these changes.
> >     clock is enabled (380953Hz)
> >     clock is enabled (25000000Hz)
> > After these changes
> >     clock is enabled (380953Hz)
> >     clock is enabled (25000000Hz)
> >     clock is enabled (52000000Hz)
> > Test on Odroid N2 and Odroid C2 with eMMC and microSD cards
> > ---
> >  arch/arm/include/asm/arch-meson/sd_emmc.h | 14 ++++++--------
> >  drivers/mmc/meson_gx_mmc.c                |  9 +++++----
> >  2 files changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
> > index e3a72c8b66..ee20c009e2 100644
> > --- a/arch/arm/include/asm/arch-meson/sd_emmc.h
> > +++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
> > @@ -7,6 +7,7 @@
> >  #define __SD_EMMC_H__
> >
> >  #include <mmc.h>
> > +#include <linux/bitops.h>
> >
> >  #define SDIO_PORT_A                  0
> >  #define SDIO_PORT_B                  1
> > @@ -19,14 +20,11 @@
> >  #define   CLK_MAX_DIV                        63
> >  #define   CLK_SRC_24M                        (0 << 6)
> >  #define   CLK_SRC_DIV2                       (1 << 6)
> > -#define   CLK_CO_PHASE_000           (0 << 8)
> > -#define   CLK_CO_PHASE_090           (1 << 8)
> > -#define   CLK_CO_PHASE_180           (2 << 8)
> > -#define   CLK_CO_PHASE_270           (3 << 8)
> > -#define   CLK_TX_PHASE_000           (0 << 10)
> > -#define   CLK_TX_PHASE_090           (1 << 10)
> > -#define   CLK_TX_PHASE_180           (2 << 10)
> > -#define   CLK_TX_PHASE_270           (3 << 10)
> > +
> > +#define   CLK_PHASE_180              2
> > +#define   CLK_TX_PHASE_MASK          GENMASK(11, 10)
> > +#define   CLK_RX_PHASE_MASK          GENMASK(13, 12)
> > +
> >  #define   CLK_ALWAYS_ON                      BIT(24)
> >
> >  #define MESON_SD_EMMC_CFG            0x44
> > diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
> > index 86c1a7164a..ad697d3a5e 100644
> > --- a/drivers/mmc/meson_gx_mmc.c
> > +++ b/drivers/mmc/meson_gx_mmc.c
> > @@ -52,10 +52,11 @@ static void meson_mmc_config_clock(struct mmc *mmc)
> >       clk_div = DIV_ROUND_UP(clk, mmc->clock);
> >
> >       /* 180 phase core clock */
> > -     meson_mmc_clk |= CLK_CO_PHASE_180;
> > -
> > -     /* 180 phase tx clock */
> > -     meson_mmc_clk |= CLK_TX_PHASE_000;
> > +     meson_mmc_clk |= CLK_PHASE_180;
> > +     /* 000 phase rx clock */
> > +     meson_mmc_clk |= CLK_RX_PHASE_MASK;
> > +     /* 000 phase tx clock */
> > +     meson_mmc_clk |= CLK_TX_PHASE_MASK;
>
> The comment on your previous version seemed correct but I think what you
> have implemented here is still not doing what you expect.

Thanks for your review. Ok I admit, I did not study the code correctly and
it's wrong to set this way. Plz discard this changes it's wrong.
It did not worked for some sdcards and emmc.

-Anand
diff mbox series

Patch

diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
index e3a72c8b66..ee20c009e2 100644
--- a/arch/arm/include/asm/arch-meson/sd_emmc.h
+++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
@@ -7,6 +7,7 @@ 
 #define __SD_EMMC_H__
 
 #include <mmc.h>
+#include <linux/bitops.h>
 
 #define SDIO_PORT_A			0
 #define SDIO_PORT_B			1
@@ -19,14 +20,11 @@ 
 #define   CLK_MAX_DIV			63
 #define   CLK_SRC_24M			(0 << 6)
 #define   CLK_SRC_DIV2			(1 << 6)
-#define   CLK_CO_PHASE_000		(0 << 8)
-#define   CLK_CO_PHASE_090		(1 << 8)
-#define   CLK_CO_PHASE_180		(2 << 8)
-#define   CLK_CO_PHASE_270		(3 << 8)
-#define   CLK_TX_PHASE_000		(0 << 10)
-#define   CLK_TX_PHASE_090		(1 << 10)
-#define   CLK_TX_PHASE_180		(2 << 10)
-#define   CLK_TX_PHASE_270		(3 << 10)
+
+#define   CLK_PHASE_180 		2
+#define   CLK_TX_PHASE_MASK		GENMASK(11, 10)
+#define   CLK_RX_PHASE_MASK		GENMASK(13, 12)
+
 #define   CLK_ALWAYS_ON			BIT(24)
 
 #define MESON_SD_EMMC_CFG		0x44
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
index 86c1a7164a..ad697d3a5e 100644
--- a/drivers/mmc/meson_gx_mmc.c
+++ b/drivers/mmc/meson_gx_mmc.c
@@ -52,10 +52,11 @@  static void meson_mmc_config_clock(struct mmc *mmc)
 	clk_div = DIV_ROUND_UP(clk, mmc->clock);
 
 	/* 180 phase core clock */
-	meson_mmc_clk |= CLK_CO_PHASE_180;
-
-	/* 180 phase tx clock */
-	meson_mmc_clk |= CLK_TX_PHASE_000;
+	meson_mmc_clk |= CLK_PHASE_180;
+	/* 000 phase rx clock */
+	meson_mmc_clk |= CLK_RX_PHASE_MASK;
+	/* 000 phase tx clock */
+	meson_mmc_clk |= CLK_TX_PHASE_MASK;
 
 	/* clock settings */
 	meson_mmc_clk |= clk_src;