Re: [U-Boot] [PATCHv2 1/3] mmc: meson-gx: Fix clk phase tuning for MMC

2019-12-26 Thread Jerome Brunet


On Tue 24 Dec 2019 at 14:25, Anand Moon  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 
> ---
> Changes from previous
> 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 (2500Hz)
> After these changes
> clock is enabled (380953Hz)
> clock is enabled (2500Hz)
> clock is enabled (5200Hz)
> 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..d70fe4f03e 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 
> +#include 
>  
>  #define SDIO_PORT_A  0
>  #define SDIO_PORT_B  1
> @@ -19,14 +20,11 @@
>  #define   CLK_MAX_DIV63
>  #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_CORE_PHASE_MASKGENMASK(9, 8)
> +#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_CFG0x44
> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
> index 86c1a7164a..402981c3bb 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_CORE_PHASE_MASK;
> + /* 000 phase rx clock */
> + meson_mmc_clk |= CLK_RX_PHASE_MASK;
> + /* 000 phase tx clock */
> + meson_mmc_clk |= CLK_TX_PHASE_MASK;
>

I'm not sure how this acheive what is descibed in the commit
description.

It looks more that it would set a 270 degree phase on all clocks, which
is not desirable.

>   /* clock settings */
>   meson_mmc_clk |= clk_src;



Re: [U-Boot] [PATCHv2 1/3] mmc: meson-gx: Fix clk phase tuning for MMC

2019-12-26 Thread Anand Moon
Hi Jerome,

On Thu, 26 Dec 2019 at 14:40, Jerome Brunet  wrote:
>
>
> On Tue 24 Dec 2019 at 14:25, Anand Moon  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 
> > ---
> > Changes from previous
> > 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 (2500Hz)
> > After these changes
> > clock is enabled (380953Hz)
> > clock is enabled (2500Hz)
> > clock is enabled (5200Hz)
> > 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..d70fe4f03e 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 
> > +#include 
> >
> >  #define SDIO_PORT_A  0
> >  #define SDIO_PORT_B  1
> > @@ -19,14 +20,11 @@
> >  #define   CLK_MAX_DIV63
> >  #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_CORE_PHASE_MASKGENMASK(9, 8)
> > +#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_CFG0x44
> > diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
> > index 86c1a7164a..402981c3bb 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_CORE_PHASE_MASK;
> > + /* 000 phase rx clock */
> > + meson_mmc_clk |= CLK_RX_PHASE_MASK;
> > + /* 000 phase tx clock */
> > + meson_mmc_clk |= CLK_TX_PHASE_MASK;
> >
>
> I'm not sure how this acheive what is descibed in the commit
> description.
>
> It looks more that it would set a 270 degree phase on all clocks, which
> is not desirable.
>
Oops thanks for spotting my mistake, you are correct, it should be as below.

clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, CLK_PHASE_180);
clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, CLK_PHASE_0);
clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, CLK_PHASE_0);
I will resend with correct configuration.

-Anand