Re: [U-Boot] [PATCH 1/7] mmc: support hs400 enhanced strobe mode

2019-07-10 Thread Peng Fan
Hi JJ,

> Subject: Re: [PATCH 1/7] mmc: support hs400 enhanced strobe mode
> 
> Hi Peng,
> 
> On 10/07/2019 09:51, Peng Fan wrote:
> > +#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
> > +#if !CONFIG_IS_ENABLED(DM_MMC)
> > +static void mmc_set_enhanced_strobe(struct mmc *mmc) { } #endif
> > +static int mmc_select_hs400es(struct mmc *mmc) {
> > +   int err;
> > +
> > +   err = mmc_set_card_speed(mmc, MMC_HS, true);
> > +   if (err)
> > +   return err;
> > +
> > +   err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_BUS_WIDTH,
> > +EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG |
> > +EXT_CSD_BUS_WIDTH_STROBE);
> > +   if (err) {
> > +   printf("switch to bus width for hs400 failed\n");
> > +   return err;
> > +   }
> > +   /* TODO: driver strength */
> 
> > +   err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_HS_TIMING,
> > +EXT_CSD_TIMING_HS400 | (0 <<
> EXT_CSD_DRV_STR_SHIFT));
> 
> you can use mmc_set_card_speed() for this.

Fixed in V2.

> 
> 
> > diff --git a/include/mmc.h b/include/mmc.h index
> > 2be3e91fcb..9eb328384e 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -65,6 +65,7 @@
> >   #define MMC_MODE_DDR_52MHzMMC_CAP(MMC_DDR_52)
> >   #define MMC_MODE_HS200MMC_CAP(MMC_HS_200)
> >   #define MMC_MODE_HS400MMC_CAP(MMC_HS_400)
> > +#define MMC_MODE_HS400_ES  MMC_CAP(MMC_HS_400_ES)
> >
> >   #define MMC_CAP_NONREMOVABLE  BIT(14)
> >   #define MMC_CAP_NEEDS_POLLBIT(15)
> > @@ -223,6 +224,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> >   #define EXT_CSD_BOOT_BUS_WIDTH177
> >   #define EXT_CSD_PART_CONF 179 /* R/W */
> >   #define EXT_CSD_BUS_WIDTH 183 /* R/W */
> > +#define EXT_CSD_STROBE_SUPPORT 184 /* R/W */
> >   #define EXT_CSD_HS_TIMING 185 /* R/W */
> >   #define EXT_CSD_REV   192 /* RO */
> >   #define EXT_CSD_CARD_TYPE 196 /* RO */
> > @@ -264,11 +266,13 @@ static inline bool mmc_is_tuning_cmd(uint
> cmdidx)
> >   #define EXT_CSD_DDR_BUS_WIDTH_4   5   /* Card is in 4 bit DDR
> mode */
> >   #define EXT_CSD_DDR_BUS_WIDTH_8   6   /* Card is in 8 bit DDR
> mode */
> >   #define EXT_CSD_DDR_FLAG  BIT(2)  /* Flag for DDR mode */
> > +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)/* Enhanced strobe
> mode */
> >
> >   #define EXT_CSD_TIMING_LEGACY 0   /* no high speed */
> >   #define EXT_CSD_TIMING_HS 1   /* HS */
> >   #define EXT_CSD_TIMING_HS200  2   /* HS200 */
> >   #define EXT_CSD_TIMING_HS400  3   /* HS400 */
> > +#define EXT_CSD_DRV_STR_SHIFT  4   /* Driver Strength shift */
> >
> >   #define EXT_CSD_BOOT_ACK_ENABLE   (1 << 6)
> >   #define EXT_CSD_BOOT_PARTITION_ENABLE (1 << 3)
> > @@ -465,6 +469,11 @@ struct dm_mmc_ops {
> >  */
> > int (*wait_dat0)(struct udevice *dev, int state, int timeout);
> >   #endif
> > +
> > +#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
> > +   /* set_enhanced_strobe() - set HS400 enhanced strobe */
> > +   void (*set_enhanced_strobe)(struct udevice *dev);
> 
> Can this function somehow fail ? I'd rather have it return a int if so

Fixed in v2.

Thanks,
Peng.

> 
> 
> JJ
> 
> 
> > +#endif
> >   };
> >
> >   #define mmc_get_ops(dev)((struct dm_mmc_ops
> *)(dev)->driver->ops)
> > @@ -485,6 +494,7 @@ int mmc_getcd(struct mmc *mmc);
> >   int mmc_getwp(struct mmc *mmc);
> >   int mmc_execute_tuning(struct mmc *mmc, uint opcode);
> >   int mmc_wait_dat0(struct mmc *mmc, int state, int timeout);
> > +void mmc_set_enhanced_strobe(struct mmc *mmc);
> >
> >   #else
> >   struct mmc_ops {
> > @@ -530,6 +540,7 @@ enum bus_mode {
> > UHS_SDR104,
> > MMC_HS_200,
> > MMC_HS_400,
> > +   MMC_HS_400_ES,
> > MMC_MODES_END
> >   };
> >
> > @@ -547,6 +558,10 @@ static inline bool mmc_is_mode_ddr(enum
> bus_mode mode)
> >   #if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> > else if (mode == MMC_HS_400)
> > return true;
> > +#endif
> > +#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
> > +   else if (mode == MMC_HS_400_ES)
> > +   return true;
> >   #endif
> > else
> > return false;

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/7] mmc: support hs400 enhanced strobe mode

2019-07-10 Thread Jean-Jacques Hiblot

Hi Peng,

On 10/07/2019 09:51, Peng Fan wrote:

+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+#if !CONFIG_IS_ENABLED(DM_MMC)
+static void mmc_set_enhanced_strobe(struct mmc *mmc)
+{
+}
+#endif
+static int mmc_select_hs400es(struct mmc *mmc)
+{
+   int err;
+
+   err = mmc_set_card_speed(mmc, MMC_HS, true);
+   if (err)
+   return err;
+
+   err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
+EXT_CSD_BUS_WIDTH_8 | EXT_CSD_DDR_FLAG |
+EXT_CSD_BUS_WIDTH_STROBE);
+   if (err) {
+   printf("switch to bus width for hs400 failed\n");
+   return err;
+   }
+   /* TODO: driver strength */



+   err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+EXT_CSD_TIMING_HS400 | (0 << EXT_CSD_DRV_STR_SHIFT));


you can use mmc_set_card_speed() for this.



diff --git a/include/mmc.h b/include/mmc.h
index 2be3e91fcb..9eb328384e 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -65,6 +65,7 @@
  #define MMC_MODE_DDR_52MHzMMC_CAP(MMC_DDR_52)
  #define MMC_MODE_HS200MMC_CAP(MMC_HS_200)
  #define MMC_MODE_HS400MMC_CAP(MMC_HS_400)
+#define MMC_MODE_HS400_ES  MMC_CAP(MMC_HS_400_ES)
  
  #define MMC_CAP_NONREMOVABLE	BIT(14)

  #define MMC_CAP_NEEDS_POLLBIT(15)
@@ -223,6 +224,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
  #define EXT_CSD_BOOT_BUS_WIDTH177
  #define EXT_CSD_PART_CONF 179 /* R/W */
  #define EXT_CSD_BUS_WIDTH 183 /* R/W */
+#define EXT_CSD_STROBE_SUPPORT 184 /* R/W */
  #define EXT_CSD_HS_TIMING 185 /* R/W */
  #define EXT_CSD_REV   192 /* RO */
  #define EXT_CSD_CARD_TYPE 196 /* RO */
@@ -264,11 +266,13 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
  #define EXT_CSD_DDR_BUS_WIDTH_4   5   /* Card is in 4 bit DDR mode */
  #define EXT_CSD_DDR_BUS_WIDTH_8   6   /* Card is in 8 bit DDR mode */
  #define EXT_CSD_DDR_FLAG  BIT(2)  /* Flag for DDR mode */
+#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)/* Enhanced strobe mode */
  
  #define EXT_CSD_TIMING_LEGACY	0	/* no high speed */

  #define EXT_CSD_TIMING_HS 1   /* HS */
  #define EXT_CSD_TIMING_HS200  2   /* HS200 */
  #define EXT_CSD_TIMING_HS400  3   /* HS400 */
+#define EXT_CSD_DRV_STR_SHIFT  4   /* Driver Strength shift */
  
  #define EXT_CSD_BOOT_ACK_ENABLE			(1 << 6)

  #define EXT_CSD_BOOT_PARTITION_ENABLE (1 << 3)
@@ -465,6 +469,11 @@ struct dm_mmc_ops {
 */
int (*wait_dat0)(struct udevice *dev, int state, int timeout);
  #endif
+
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+   /* set_enhanced_strobe() - set HS400 enhanced strobe */
+   void (*set_enhanced_strobe)(struct udevice *dev);


Can this function somehow fail ? I'd rather have it return a int if so


JJ



+#endif
  };
  
  #define mmc_get_ops(dev)((struct dm_mmc_ops *)(dev)->driver->ops)

@@ -485,6 +494,7 @@ int mmc_getcd(struct mmc *mmc);
  int mmc_getwp(struct mmc *mmc);
  int mmc_execute_tuning(struct mmc *mmc, uint opcode);
  int mmc_wait_dat0(struct mmc *mmc, int state, int timeout);
+void mmc_set_enhanced_strobe(struct mmc *mmc);
  
  #else

  struct mmc_ops {
@@ -530,6 +540,7 @@ enum bus_mode {
UHS_SDR104,
MMC_HS_200,
MMC_HS_400,
+   MMC_HS_400_ES,
MMC_MODES_END
  };
  
@@ -547,6 +558,10 @@ static inline bool mmc_is_mode_ddr(enum bus_mode mode)

  #if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
else if (mode == MMC_HS_400)
return true;
+#endif
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+   else if (mode == MMC_HS_400_ES)
+   return true;
  #endif
else
return false;


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 1/7] mmc: support hs400 enhanced strobe mode

2019-07-10 Thread Peng Fan
eMMC 5.1+ supports HS400 Enhances Strobe mode without the need for
tuning procedure.
The flow is as following:
 - set HS_TIMIMG (Highspeed)
 - Host change freq to <= 52Mhz
 - set the bus width to Enhanced strobe and DDR8Bit(CMD6),
   EXT_CSD[183] = 0x86 instead of 0x80
 - set HS_TIMING to 0x3 (HS400)
 - Host change freq to <= 200Mhz
 - Host select HS400 enhanced strobe complete

Signed-off-by: Peng Fan 
Cc: Jean-Jacques Hiblot 
Cc: Baruch Siach 
Cc: Michal Simek 
Cc: T Karthik Reddy 
Cc: Simon Glass 
Cc: Marek Vasut 
Cc: Jon Nettleton 
---
 drivers/mmc/Kconfig  | 12 
 drivers/mmc/mmc-uclass.c | 15 ++
 drivers/mmc/mmc.c| 73 +++-
 include/mmc.h| 15 ++
 4 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 93588725f2..f22c0a0589 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -117,6 +117,18 @@ config SPL_MMC_UHS_SUPPORT
  cards. The IO voltage must be switchable from 3.3v to 1.8v. The bus
  frequency can go up to 208MHz (SDR104)
 
+config MMC_HS400_ES_SUPPORT
+   bool "enable HS400 Enhanced Strobe support"
+   help
+ The HS400 Enhanced Strobe mode is support by some eMMC. The bus
+ frequency is up to 200MHz. This mode does not tune the IO.
+
+config SPL_MMC_HS400_ES_SUPPORT
+   bool "enable HS400 Enhanced Strobe support in SPL"
+   help
+ The HS400 Enhanced Strobe mode is support by some eMMC. The bus
+ frequency is up to 200MHz. This mode does not tune the IO.
+
 config MMC_HS400_SUPPORT
bool "enable HS400 support"
select MMC_HS200_SUPPORT
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index fa4d1af55d..bf34746404 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -120,6 +120,21 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode)
 }
 #endif
 
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+void dm_mmc_set_enhanced_strobe(struct udevice *dev)
+{
+   struct dm_mmc_ops *ops = mmc_get_ops(dev);
+
+   if (ops->set_enhanced_strobe)
+   ops->set_enhanced_strobe(dev);
+}
+
+void mmc_set_enhanced_strobe(struct mmc *mmc)
+{
+   dm_mmc_set_enhanced_strobe(mmc->dev);
+}
+#endif
+
 int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
 {
int val;
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 71b52c6cf2..e3b3a28fd7 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -148,6 +148,7 @@ const char *mmc_mode_name(enum bus_mode mode)
  [MMC_DDR_52]  = "MMC DDR52 (52MHz)",
  [MMC_HS_200]  = "HS200 (200MHz)",
  [MMC_HS_400]  = "HS400 (200MHz)",
+ [MMC_HS_400_ES]   = "HS400ES (200MHz)",
};
 
if (mode >= MMC_MODES_END)
@@ -173,6 +174,7 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode 
mode)
  [UHS_SDR104]  = 20800,
  [MMC_HS_200]  = 2,
  [MMC_HS_400]  = 2,
+ [MMC_HS_400_ES]   = 2,
};
 
if (mode == MMC_LEGACY)
@@ -788,6 +790,11 @@ static int mmc_set_card_speed(struct mmc *mmc, enum 
bus_mode mode,
case MMC_HS_400:
speed_bits = EXT_CSD_TIMING_HS400;
break;
+#endif
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+   case MMC_HS_400_ES:
+   speed_bits = EXT_CSD_TIMING_HS400;
+   break;
 #endif
case MMC_LEGACY:
speed_bits = EXT_CSD_TIMING_LEGACY;
@@ -859,7 +866,8 @@ static int mmc_get_capabilities(struct mmc *mmc)
mmc->card_caps |= MMC_MODE_HS200;
}
 #endif
-#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
+#if CONFIG_IS_ENABLED(MMC_HS400_SUPPORT) || \
+   CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
if (cardtype & (EXT_CSD_CARD_TYPE_HS400_1_2V |
EXT_CSD_CARD_TYPE_HS400_1_8V)) {
mmc->card_caps |= MMC_MODE_HS400;
@@ -873,6 +881,13 @@ static int mmc_get_capabilities(struct mmc *mmc)
if (cardtype & EXT_CSD_CARD_TYPE_26)
mmc->card_caps |= MMC_MODE_HS;
 
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+   if (ext_csd[EXT_CSD_STROBE_SUPPORT] &&
+   (mmc->card_caps & MMC_MODE_HS400)) {
+   mmc->card_caps |= MMC_MODE_HS400_ES;
+   }
+#endif
+
return 0;
 }
 #endif
@@ -1778,6 +1793,7 @@ static int mmc_set_lowest_voltage(struct mmc *mmc, enum 
bus_mode mode,
u32 card_mask = 0;
 
switch (mode) {
+   case MMC_HS_400_ES:
case MMC_HS_400:
case MMC_HS_200:
if (mmc->cardtype & (EXT_CSD_CARD_TYPE_HS200_1_8V |
@@ -1820,6 +1836,12 @@ static inline int mmc_set_lowest_voltage(struct mmc 
*mmc, enum bus_mode mode,
 #endif
 
 static const struct mode_width_tuning mmc_modes_by_pref[] = {
+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
+   {
+   .mode = MMC_HS_400_ES,
+