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

[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_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


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;