RE: [PATCH v2 2/5] mmc: identify available device type to select

[Seungwon Jeon <tgih.jun@samsung.com>]

On Mon, March 10, 2014, Jaehoon Chung wrote:
> On 03/07/2014 11:36 PM, Seungwon Jeon wrote:
> > Device types which are supported by both host and device
> > can be identified when EXT_CSD is read. There is no need to
> > check host's capability anymore.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > Changes in v2:
> > 	Just rebased with latest one.
> >
> >  drivers/mmc/core/mmc.c   |   77 ++++++++++++++++++++++++++-------------------
> >  include/linux/mmc/card.h |    6 ++-
> >  include/linux/mmc/host.h |    6 ---
> >  include/linux/mmc/mmc.h  |   12 +++++--
> >  4 files changed, 56 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index db9655f..0abece0 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -243,28 +243,46 @@ static void mmc_select_card_type(struct mmc_card *card)
> >  	u8 card_type = card->ext_csd.raw_card_type & EXT_CSD_CARD_TYPE_MASK;
> >  	u32 caps = host->caps, caps2 = host->caps2;
> >  	unsigned int hs_max_dtr = 0;
> > +	unsigned int avail_type = 0;
> >
> > -	if (card_type & EXT_CSD_CARD_TYPE_26)
> > +	if (caps & MMC_CAP_MMC_HIGHSPEED &&
> > +	    card_type & EXT_CSD_CARD_TYPE_HS_26) {
> >  		hs_max_dtr = MMC_HIGH_26_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_HS_26;
> > +	}
> >
> >  	if (caps & MMC_CAP_MMC_HIGHSPEED &&
> > -			card_type & EXT_CSD_CARD_TYPE_52)
> > +	    card_type & EXT_CSD_CARD_TYPE_HS_52) {
> >  		hs_max_dtr = MMC_HIGH_52_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
> > +	}
> Can it bind with "caps & MMC_CAP_MMC_HIGHSPEED"?
Yes, 'nested if' may be possible here.
I just think current style is more harmonious though.

> if (caps & MMC_CAP_MMC_HIGH_SPEED) {
> 	if (card_type & EXT_CSD_CARD_TYPE_HS_26) {
> 		...
> 	}
> 	if (card_type & EXT_CSD_CARD_TYPE_HS_52) {
> 		...
> 	}
> }
> >
> > -	if ((caps & MMC_CAP_1_8V_DDR &&
> > -			card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) ||
> > -	    (caps & MMC_CAP_1_2V_DDR &&
> > -			card_type & EXT_CSD_CARD_TYPE_DDR_1_2V))
> > +	if (caps & MMC_CAP_1_8V_DDR &&
> > +	    card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
> >  		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_DDR_1_8V;
> > +	}
> >
> > -	if ((caps2 & MMC_CAP2_HS200_1_8V_SDR &&
> > -			card_type & EXT_CSD_CARD_TYPE_SDR_1_8V) ||
> > -	    (caps2 & MMC_CAP2_HS200_1_2V_SDR &&
> > -			card_type & EXT_CSD_CARD_TYPE_SDR_1_2V))
> > +	if (caps & MMC_CAP_1_2V_DDR &&
> > +	    card_type & EXT_CSD_CARD_TYPE_DDR_1_2V) {
> > +		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_DDR_1_2V;
> > +	}
> > +
> > +	if (caps2 & MMC_CAP2_HS200_1_8V_SDR &&
> > +	    card_type & EXT_CSD_CARD_TYPE_HS200_1_8V) {
> >  		hs_max_dtr = MMC_HS200_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_HS200_1_8V;
> > +	}
> > +
> > +	if (caps2 & MMC_CAP2_HS200_1_2V_SDR &&
> > +	    card_type & EXT_CSD_CARD_TYPE_HS200_1_2V) {
> > +		hs_max_dtr = MMC_HS200_MAX_DTR;
> > +		avail_type |= EXT_CSD_CARD_TYPE_HS200_1_2V;
> > +	}
> >
> >  	card->ext_csd.hs_max_dtr = hs_max_dtr;
> > -	card->ext_csd.card_type = card_type;
> > +	card->mmc_avail_type = avail_type;
> >  }
> >
> >  /*
> > @@ -708,6 +726,11 @@ static struct device_type mmc_type = {
> >  	.groups = mmc_attr_groups,
> >  };
> >
> > +static inline unsigned int mmc_snoop_ddr(struct mmc_card *card)
> > +{
> > +	return card->mmc_avail_type & EXT_CSD_CARD_TYPE_DDR_52;
> > +}
> > +
> >  /*
> >   * Select the PowerClass for the current bus width
> >   * If power class is defined for 4/8 bit bus in the
> > @@ -808,12 +831,10 @@ static int mmc_select_hs200(struct mmc_card *card)
> >
> >  	host = card->host;
> >
> > -	if (card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_2V &&
> > -			host->caps2 & MMC_CAP2_HS200_1_2V_SDR)
> > +	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200_1_2V)
> >  		err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120);
> >
> > -	if (err && card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_8V &&
> > -			host->caps2 & MMC_CAP2_HS200_1_8V_SDR)
> > +	if (err && card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200_1_8V)
> >  		err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
> >
> >  	/* If fails try again during next card power cycle */
> > @@ -1072,10 +1093,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >  	 */
> >  	if (card->ext_csd.hs_max_dtr != 0) {
> >  		err = 0;
> > -		if (card->ext_csd.hs_max_dtr > 52000000 &&
> > -		    host->caps2 & MMC_CAP2_HS200)
> MMC_CAP2_HS200 need no more, doesn't?
If you mean to remove 'MMC_CAP2_HS200' definition, it is currently used in sdhci.c

Thanks,
Seungwon Jeon

> 
> > +		if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
> >  			err = mmc_select_hs200(card);
> > -		else if	(host->caps & MMC_CAP_MMC_HIGHSPEED)
> > +		else if	(card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
> >  			err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >  					EXT_CSD_HS_TIMING, 1,
> >  					card->ext_csd.generic_cmd6_time,
> > @@ -1089,13 +1109,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >  			       mmc_hostname(card->host));
> >  			err = 0;
> >  		} else {
> > -			if (card->ext_csd.hs_max_dtr > 52000000 &&
> > -			    host->caps2 & MMC_CAP2_HS200) {
> > +			if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
> >  				mmc_set_timing(card->host,
> >  					       MMC_TIMING_MMC_HS200);
> > -			} else {
> > +			else
> >  				mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> > -			}
> >  		}
> >  	}
> >
> > @@ -1118,14 +1136,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >  	/*
> >  	 * Indicate DDR mode (if supported).
> >  	 */
> > -	if (mmc_card_hs(card)) {
> > -		if ((card->ext_csd.card_type & EXT_CSD_CARD_TYPE_DDR_1_8V)
> > -			&& (host->caps & MMC_CAP_1_8V_DDR))
> > -				ddr = MMC_1_8V_DDR_MODE;
> > -		else if ((card->ext_csd.card_type & EXT_CSD_CARD_TYPE_DDR_1_2V)
> > -			&& (host->caps & MMC_CAP_1_2V_DDR))
> > -				ddr = MMC_1_2V_DDR_MODE;
> > -	}
> > +	if (mmc_card_hs(card))
> > +		ddr = mmc_snoop_ddr(card);
> >
> >  	/*
> >  	 * Indicate HS200 SDR mode (if supported).
> > @@ -1145,8 +1157,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >  		 * 3. set the clock to > 52Mhz <=200MHz and
> >  		 * 4. execute tuning for HS200
> >  		 */
> > -		if ((host->caps2 & MMC_CAP2_HS200) &&
> > -		    card->host->ops->execute_tuning) {
> > +		if (card->host->ops->execute_tuning) {
> >  			mmc_host_clk_hold(card->host);
> >  			err = card->host->ops->execute_tuning(card->host,
> >  				MMC_SEND_TUNING_BLOCK_HS200);
> > @@ -1255,7 +1266,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >  			 *
> >  			 * WARNING: eMMC rules are NOT the same as SD DDR
> >  			 */
> > -			if (ddr == MMC_1_2V_DDR_MODE) {
> > +			if (ddr & EXT_CSD_CARD_TYPE_DDR_1_2V) {
> >  				err = __mmc_set_signal_voltage(host,
> >  					MMC_SIGNAL_VOLTAGE_120);
> >  				if (err)
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index 5473133..c232b10 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -68,7 +68,6 @@ struct mmc_ext_csd {
> >  #define MMC_HIGH_DDR_MAX_DTR	52000000
> >  #define MMC_HS200_MAX_DTR	200000000
> >  	unsigned int		sectors;
> > -	unsigned int		card_type;
> >  	unsigned int		hc_erase_size;		/* In sectors */
> >  	unsigned int		hc_erase_timeout;	/* In milliseconds */
> >  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
> > @@ -297,7 +296,10 @@ struct mmc_card {
> >  	const char		**info;		/* info strings */
> >  	struct sdio_func_tuple	*tuples;	/* unknown common tuples */
> >
> > -	unsigned int		sd_bus_speed;	/* Bus Speed Mode set for the card */
> > +	union {
> > +		unsigned int		sd_bus_speed;	/* Bus Speed Mode set for the card */
> > +		unsigned int		mmc_avail_type;	/* supported device type by both host and card */
> > +	};
> >
> >  	struct dentry		*debugfs_root;
> >  	struct mmc_part	part[MMC_NUM_PHY_PARTITION]; /* physical partitions */
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 2f263ae..1ee3c10 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -62,12 +62,6 @@ struct mmc_ios {
> >  #define MMC_TIMING_MMC_DDR52	8
> >  #define MMC_TIMING_MMC_HS200	9
> >
> > -#define MMC_SDR_MODE		0
> > -#define MMC_1_2V_DDR_MODE	1
> > -#define MMC_1_8V_DDR_MODE	2
> > -#define MMC_1_2V_SDR_MODE	3
> > -#define MMC_1_8V_SDR_MODE	4
> > -
> >  	unsigned char	signal_voltage;		/* signalling voltage (1.8V or 3.3V) */
> >
> >  #define MMC_SIGNAL_VOLTAGE_330	0
> > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> > index 50bcde3..f734c0c 100644
> > --- a/include/linux/mmc/mmc.h
> > +++ b/include/linux/mmc/mmc.h
> > @@ -354,18 +354,22 @@ struct _mmc_csd {
> >  #define EXT_CSD_CMD_SET_SECURE		(1<<1)
> >  #define EXT_CSD_CMD_SET_CPSECURE	(1<<2)
> >
> > -#define EXT_CSD_CARD_TYPE_26	(1<<0)	/* Card can run at 26MHz */
> > -#define EXT_CSD_CARD_TYPE_52	(1<<1)	/* Card can run at 52MHz */
> >  #define EXT_CSD_CARD_TYPE_MASK	0x3F	/* Mask out reserved bits */
> > +#define EXT_CSD_CARD_TYPE_HS_26	(1<<0)	/* Card can run at 26MHz */
> > +#define EXT_CSD_CARD_TYPE_HS_52	(1<<1)	/* Card can run at 52MHz */
> > +#define EXT_CSD_CARD_TYPE_HS	(EXT_CSD_CARD_TYPE_HS_26 | \
> > +				 EXT_CSD_CARD_TYPE_HS_52)
> >  #define EXT_CSD_CARD_TYPE_DDR_1_8V  (1<<2)   /* Card can run at 52MHz */
> >  					     /* DDR mode @1.8V or 3V I/O */
> >  #define EXT_CSD_CARD_TYPE_DDR_1_2V  (1<<3)   /* Card can run at 52MHz */
> >  					     /* DDR mode @1.2V I/O */
> >  #define EXT_CSD_CARD_TYPE_DDR_52       (EXT_CSD_CARD_TYPE_DDR_1_8V  \
> >  					| EXT_CSD_CARD_TYPE_DDR_1_2V)
> > -#define EXT_CSD_CARD_TYPE_SDR_1_8V	(1<<4)	/* Card can run at 200MHz */
> > -#define EXT_CSD_CARD_TYPE_SDR_1_2V	(1<<5)	/* Card can run at 200MHz */
> > +#define EXT_CSD_CARD_TYPE_HS200_1_8V	(1<<4)	/* Card can run at 200MHz */
> > +#define EXT_CSD_CARD_TYPE_HS200_1_2V	(1<<5)	/* Card can run at 200MHz */
> >  						/* SDR mode @1.2V I/O */
> > +#define EXT_CSD_CARD_TYPE_HS200		(EXT_CSD_CARD_TYPE_HS200_1_8V | \
> > +					 EXT_CSD_CARD_TYPE_HS200_1_2V)
> >
> >  #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
> >  #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html