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_52MHz MMC_CAP(MMC_DDR_52)
> > #define MMC_MODE_HS200 MMC_CAP(MMC_HS_200)
> > #define MMC_MODE_HS400 MMC_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_POLL BIT(15)
> > @@ -223,6 +224,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> > #define EXT_CSD_BOOT_BUS_WIDTH 177
> > #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
