Hi Jaehoon, On Thu, 23 Jan 2020 at 05:09, Jaehoon Chung <[email protected]> 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 <[email protected]> > > --- > > 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; > > >
