On Thu, March 13, 2014, Jaehoon Chung wrote:
> Dear, Seungwon.
>
> On 03/10/2014 08:59 PM, Seungwon Jeon wrote:
> > 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@xxxxxxxxxxx>
> >>> ---
> >>> 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.
> Don't mind. it's ok. :)
>
> >
> >> 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
> I have also checked that it is used in sdhci.c.
> but I think that capability like MMC_CAP2_HS200 was defined to use for core, not controller.
> It can be changed other quirks instead of MMC_CAP2_HS200 into sdhci.c
> how about?
Hmm.
I feel like current way is not bad in sdhci.c, but if you have an idea, it would be different patch.
Thanks,
Seungwon Jeon
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
