diff mbox series

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

Message ID 20200122120620.8699-2-linux.amoon@gmail.com
State Superseded
Delegated to: Peng Fan
Headers show
Series Odroid n2 using eMMC would fail to boot up | expand

Commit Message

Anand Moon Jan. 22, 2020, 12:06 p.m. UTC
As per mainline line kernel fix the clk tunig phase for
mmc, set Core=180, Tx=180, Rx=0 clk phase for mmc initialization.
As per S905X and S922X datasheet set the default values
for clk tuning.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
Changes from previous
v3  Fix the initialization of core clk tunning phase as per datasheet.
    Fix the commit message.

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 | 11 ++-----
 drivers/mmc/meson_gx_mmc.c                | 38 +++++++++++++++++++----
 2 files changed, 35 insertions(+), 14 deletions(-)

Comments

Jaehoon Chung Jan. 22, 2020, 11:39 p.m. UTC | #1
Dear Anand,

On 1/22/20 9:06 PM, Anand Moon wrote:
> As per mainline line kernel fix the clk tunig phase for
> mmc, set Core=180, Tx=180, Rx=0 clk phase for mmc initialization.
> As per S905X and S922X datasheet set the default values
> for clk tuning.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> Changes from previous
> v3  Fix the initialization of core clk tunning phase as per datasheet.
>     Fix the commit message.
> 
> 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://protect2.fireeye.com/url?k=c4a34ac1-9973420d-c4a2c18e-000babff3793-f192c82d705776ae&u=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 | 11 ++-----
>  drivers/mmc/meson_gx_mmc.c                | 38 +++++++++++++++++++----
>  2 files changed, 35 insertions(+), 14 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..c193547aad 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,8 @@
>  #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	  UPDATE(x, h, l)		(((x) << (l)) & GENMASK((h), (l)))
> +
>  #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..67b1b075ab 100644
> --- a/drivers/mmc/meson_gx_mmc.c
> +++ b/drivers/mmc/meson_gx_mmc.c
> @@ -51,12 +51,38 @@ 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;
> -
> +	/* Clock divider */
> +	meson_mmc_clk = GENMASK(5, 0);
> +	/* Clock source 1: Fix PLL, 1000MHz */
> +	meson_mmc_clk |= UPDATE(1, 7, 6);
> +	/* Core clock phase 2:180 */
> +	meson_mmc_clk |= UPDATE(2, 9, 8);
> +	/* TX clock phase 2:180 */
> +	meson_mmc_clk |= UPDATE(2, 11, 10);
> +	/* RX clock phase 0:180 */
> +	meson_mmc_clk |= UPDATE(0, 13, 12);
> +#ifdef CONFIG_MESON_GX
> +	/* TX clock delay line */
> +	meson_mmc_clk |= GENMASK(19, 16);
> +	/* RX clock delay line */
> +	meson_mmc_clk |= GENMASK(23, 20);
> +	/* clk always on */
> +	meson_mmc_clk |= BIT(20);
> +	/* clk irq sdio sleep */
> +	meson_mmc_clk |= BIT(25);

Could you define the macro instead of BIT(x)?
It's helpful to know what its bit is used. 

e.g) #define CLK_ALWAYS_ON BIT(20)

i didn't know that BIT(20)/(25) are defined as what purpose has.

Best Regards,
Jaehoon Chung

> +#endif
> +#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
> +	/* TX clock delay line */
> +	meson_mmc_clk |=  GENMASK(21, 16);
> +	/* RX clock delay line */
> +	meson_mmc_clk |=  GENMASK(27, 22);
> +	/* clk always on */
> +	meson_mmc_clk |= BIT(28);
> +	/* clk irq sdio sleep */
> +	meson_mmc_clk |= BIT(29);
> +	/* clk irq sdio sleep_ds */
> +	meson_mmc_clk |= BIT(30);
> +#endif
>  	/* clock settings */
>  	meson_mmc_clk |= clk_src;
>  	meson_mmc_clk |= clk_div;
>
Anand Moon Jan. 23, 2020, 5:14 a.m. UTC | #2
Hi Jaehoon,

On Thu, 23 Jan 2020 at 05:09, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Dear Anand,
>
> On 1/22/20 9:06 PM, Anand Moon wrote:
> > As per mainline line kernel fix the clk tunig phase for
> > mmc, set Core=180, Tx=180, Rx=0 clk phase for mmc initialization.
> > As per S905X and S922X datasheet set the default values
> > for clk tuning.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > Changes from previous
> > v3  Fix the initialization of core clk tunning phase as per datasheet.
> >     Fix the commit message.
> >
> > 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://protect2.fireeye.com/url?k=c4a34ac1-9973420d-c4a2c18e-000babff3793-f192c82d705776ae&u=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 | 11 ++-----
> >  drivers/mmc/meson_gx_mmc.c                | 38 +++++++++++++++++++----
> >  2 files changed, 35 insertions(+), 14 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..c193547aad 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,8 @@
> >  #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        UPDATE(x, h, l)               (((x) << (l)) & GENMASK((h), (l)))
> > +
> >  #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..67b1b075ab 100644
> > --- a/drivers/mmc/meson_gx_mmc.c
> > +++ b/drivers/mmc/meson_gx_mmc.c
> > @@ -51,12 +51,38 @@ 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;
> > -
> > +     /* Clock divider */
> > +     meson_mmc_clk = GENMASK(5, 0);
> > +     /* Clock source 1: Fix PLL, 1000MHz */
> > +     meson_mmc_clk |= UPDATE(1, 7, 6);
> > +     /* Core clock phase 2:180 */
> > +     meson_mmc_clk |= UPDATE(2, 9, 8);
> > +     /* TX clock phase 2:180 */
> > +     meson_mmc_clk |= UPDATE(2, 11, 10);
> > +     /* RX clock phase 0:180 */
> > +     meson_mmc_clk |= UPDATE(0, 13, 12);
> > +#ifdef CONFIG_MESON_GX
> > +     /* TX clock delay line */
> > +     meson_mmc_clk |= GENMASK(19, 16);
> > +     /* RX clock delay line */
> > +     meson_mmc_clk |= GENMASK(23, 20);
> > +     /* clk always on */

Opps Typo this should be BIT(24)

> > +     meson_mmc_clk |= BIT(20);
> > +     /* clk irq sdio sleep */
> > +     meson_mmc_clk |= BIT(25);
>
> Could you define the macro instead of BIT(x)?
> It's helpful to know what its bit is used.
>
> e.g) #define CLK_ALWAYS_ON BIT(20)
>
> i didn't know that BIT(20)/(25) are defined as what purpose has.
>
> Best Regards,
> Jaehoon Chung

Thanks for your review.
Their are different bit setting for CLK_ALWAYS_ON on
BIT(24) for (S905. S905X)  and BIT(28) for S922X.
I will change this as per your suggestion in the next version
Lets wait for more comments on this patch.

-Anand

>
> > +#endif
> > +#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
> > +     /* TX clock delay line */
> > +     meson_mmc_clk |=  GENMASK(21, 16);
> > +     /* RX clock delay line */
> > +     meson_mmc_clk |=  GENMASK(27, 22);
> > +     /* clk always on */
> > +     meson_mmc_clk |= BIT(28);
> > +     /* clk irq sdio sleep */
> > +     meson_mmc_clk |= BIT(29);
> > +     /* clk irq sdio sleep_ds */
> > +     meson_mmc_clk |= BIT(30);
> > +#endif
> >       /* clock settings */
> >       meson_mmc_clk |= clk_src;
> >       meson_mmc_clk |= clk_div;
> >
>
Neil Armstrong Feb. 1, 2020, 7:24 a.m. UTC | #3
Le 22/01/2020 à 13:06, Anand Moon a écrit :
> As per mainline line kernel fix the clk tunig phase for
> mmc, set Core=180, Tx=180, Rx=0 clk phase for mmc initialization.
> As per S905X and S922X datasheet set the default values
> for clk tuning.

Please add you also add AXG/G12 setup.

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> Changes from previous
> v3  Fix the initialization of core clk tunning phase as per datasheet.
>     Fix the commit message.
> 
> 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 | 11 ++-----
>  drivers/mmc/meson_gx_mmc.c                | 38 +++++++++++++++++++----
>  2 files changed, 35 insertions(+), 14 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..c193547aad 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,8 @@
>  #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	  UPDATE(x, h, l)		(((x) << (l)) & GENMASK((h), (l)))

A macro already exists for that: FIELD_PREP(), please use it with proper BIT and GENMASK defines.

> +
>  #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..67b1b075ab 100644
> --- a/drivers/mmc/meson_gx_mmc.c
> +++ b/drivers/mmc/meson_gx_mmc.c
> @@ -51,12 +51,38 @@ 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;
> -
> +	/* Clock divider */
> +	meson_mmc_clk = GENMASK(5, 0);
> +	/* Clock source 1: Fix PLL, 1000MHz */
> +	meson_mmc_clk |= UPDATE(1, 7, 6);
> +	/* Core clock phase 2:180 */
> +	meson_mmc_clk |= UPDATE(2, 9, 8);
> +	/* TX clock phase 2:180 */
> +	meson_mmc_clk |= UPDATE(2, 11, 10);
> +	/* RX clock phase 0:180 */
> +	meson_mmc_clk |= UPDATE(0, 13, 12);
> +#ifdef CONFIG_MESON_GX
> +	/* TX clock delay line */
> +	meson_mmc_clk |= GENMASK(19, 16);
> +	/* RX clock delay line */
> +	meson_mmc_clk |= GENMASK(23, 20);
> +	/* clk always on */
> +	meson_mmc_clk |= BIT(20);
> +	/* clk irq sdio sleep */
> +	meson_mmc_clk |= BIT(25);
> +#endif
> +#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
> +	/* TX clock delay line */
> +	meson_mmc_clk |=  GENMASK(21, 16);
> +	/* RX clock delay line */
> +	meson_mmc_clk |=  GENMASK(27, 22);
> +	/* clk always on */
> +	meson_mmc_clk |= BIT(28);
> +	/* clk irq sdio sleep */
> +	meson_mmc_clk |= BIT(29);
> +	/* clk irq sdio sleep_ds */
> +	meson_mmc_clk |= BIT(30);

As Jaehoon wrote, please define these MASKs and BITs

> +#endif

Instead of ifdef, please add a data in the compatible list and use thez
fact we use amlogic,meson-axg-mmc on AXG & G12/SM1.

Example: https://github.com/u-boot/u-boot/blob/master/drivers/video/meson/meson_vpu.c#L98
and https://github.com/u-boot/u-boot/blob/master/drivers/video/meson/meson_vpu.c#L37

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

Thanks,
Neil
Anand Moon Feb. 2, 2020, 3:52 a.m. UTC | #4
Hi Neil,

On Sat, 1 Feb 2020 at 12:54, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
>
>
> Le 22/01/2020 à 13:06, Anand Moon a écrit :
> > As per mainline line kernel fix the clk tunig phase for
> > mmc, set Core=180, Tx=180, Rx=0 clk phase for mmc initialization.
> > As per S905X and S922X datasheet set the default values
> > for clk tuning.
>
> Please add you also add AXG/G12 setup.

Ok.

>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > Changes from previous
> > v3  Fix the initialization of core clk tunning phase as per datasheet.
> >     Fix the commit message.
> >
> > 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 | 11 ++-----
> >  drivers/mmc/meson_gx_mmc.c                | 38 +++++++++++++++++++----
> >  2 files changed, 35 insertions(+), 14 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..c193547aad 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,8 @@
> >  #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        UPDATE(x, h, l)               (((x) << (l)) & GENMASK((h), (l)))
>
> A macro already exists for that: FIELD_PREP(), please use it with proper BIT and GENMASK defines.
>
Thanks for this input.
> > +
> >  #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..67b1b075ab 100644
> > --- a/drivers/mmc/meson_gx_mmc.c
> > +++ b/drivers/mmc/meson_gx_mmc.c
> > @@ -51,12 +51,38 @@ 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;
> > -
> > +     /* Clock divider */
> > +     meson_mmc_clk = GENMASK(5, 0);
> > +     /* Clock source 1: Fix PLL, 1000MHz */
> > +     meson_mmc_clk |= UPDATE(1, 7, 6);
> > +     /* Core clock phase 2:180 */
> > +     meson_mmc_clk |= UPDATE(2, 9, 8);
> > +     /* TX clock phase 2:180 */
> > +     meson_mmc_clk |= UPDATE(2, 11, 10);
> > +     /* RX clock phase 0:180 */
> > +     meson_mmc_clk |= UPDATE(0, 13, 12);
> > +#ifdef CONFIG_MESON_GX
> > +     /* TX clock delay line */
> > +     meson_mmc_clk |= GENMASK(19, 16);
> > +     /* RX clock delay line */
> > +     meson_mmc_clk |= GENMASK(23, 20);
> > +     /* clk always on */
> > +     meson_mmc_clk |= BIT(20);
> > +     /* clk irq sdio sleep */
> > +     meson_mmc_clk |= BIT(25);
> > +#endif
> > +#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
> > +     /* TX clock delay line */
> > +     meson_mmc_clk |=  GENMASK(21, 16);
> > +     /* RX clock delay line */
> > +     meson_mmc_clk |=  GENMASK(27, 22);
> > +     /* clk always on */
> > +     meson_mmc_clk |= BIT(28);
> > +     /* clk irq sdio sleep */
> > +     meson_mmc_clk |= BIT(29);
> > +     /* clk irq sdio sleep_ds */
> > +     meson_mmc_clk |= BIT(30);
>
> As Jaehoon wrote, please define these MASKs and BITs
>
> > +#endif
>
> Instead of ifdef, please add a data in the compatible list and use thez
> fact we use amlogic,meson-axg-mmc on AXG & G12/SM1.
>
> Example: https://github.com/u-boot/u-boot/blob/master/drivers/video/meson/meson_vpu.c#L98
> and https://github.com/u-boot/u-boot/blob/master/drivers/video/meson/meson_vpu.c#L37
>

Ok, Thanks for the input.

> >       /* clock settings */
> >       meson_mmc_clk |= clk_src;
> >       meson_mmc_clk |= clk_div;
> >
>
> Thanks,
> Neil

-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..c193547aad 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,8 @@ 
 #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	  UPDATE(x, h, l)		(((x) << (l)) & GENMASK((h), (l)))
+
 #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..67b1b075ab 100644
--- a/drivers/mmc/meson_gx_mmc.c
+++ b/drivers/mmc/meson_gx_mmc.c
@@ -51,12 +51,38 @@  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;
-
+	/* Clock divider */
+	meson_mmc_clk = GENMASK(5, 0);
+	/* Clock source 1: Fix PLL, 1000MHz */
+	meson_mmc_clk |= UPDATE(1, 7, 6);
+	/* Core clock phase 2:180 */
+	meson_mmc_clk |= UPDATE(2, 9, 8);
+	/* TX clock phase 2:180 */
+	meson_mmc_clk |= UPDATE(2, 11, 10);
+	/* RX clock phase 0:180 */
+	meson_mmc_clk |= UPDATE(0, 13, 12);
+#ifdef CONFIG_MESON_GX
+	/* TX clock delay line */
+	meson_mmc_clk |= GENMASK(19, 16);
+	/* RX clock delay line */
+	meson_mmc_clk |= GENMASK(23, 20);
+	/* clk always on */
+	meson_mmc_clk |= BIT(20);
+	/* clk irq sdio sleep */
+	meson_mmc_clk |= BIT(25);
+#endif
+#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
+	/* TX clock delay line */
+	meson_mmc_clk |=  GENMASK(21, 16);
+	/* RX clock delay line */
+	meson_mmc_clk |=  GENMASK(27, 22);
+	/* clk always on */
+	meson_mmc_clk |= BIT(28);
+	/* clk irq sdio sleep */
+	meson_mmc_clk |= BIT(29);
+	/* clk irq sdio sleep_ds */
+	meson_mmc_clk |= BIT(30);
+#endif
 	/* clock settings */
 	meson_mmc_clk |= clk_src;
 	meson_mmc_clk |= clk_div;