Re: [PATCH v5 3/5] mmc: core: implement enhanced strobe support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/24/2016 07:11 PM, Shawn Lin wrote:
> 在 2016/5/24 17:57, Jaehoon Chung 写道:
>> On 05/23/2016 01:13 PM, Shawn Lin wrote:
>>> Controllers use data strobe line to latch data from devices
>>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
>>> introduces enhanced strobe mode for latching cmd response from
>>> emmc devices to host controllers. This new feature is optional,
>>> so it depends both on device's cap and host's cap to decide
>>> whether to use it or not.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
>>> ---
>>>
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>  drivers/mmc/core/bus.c   |  3 +-
>>>  drivers/mmc/core/core.c  |  9 ++++++
>>>  drivers/mmc/core/mmc.c   | 84 ++++++++++++++++++++++++++++++++++++++++++++++--
>>>  include/linux/mmc/card.h |  1 +
>>>  include/linux/mmc/host.h | 11 +++++++
>>>  include/linux/mmc/mmc.h  |  3 ++
>>>  6 files changed, 108 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index 4bc48f1..c64266f 100644
>>> --- a/drivers/mmc/core/bus.c
>>> +++ b/drivers/mmc/core/bus.c
>>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>>>              mmc_card_ddr52(card) ? "DDR " : "",
>>>              type);
>>>      } else {
>>> -        pr_info("%s: new %s%s%s%s%s card at address %04x\n",
>>> +        pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>>>              mmc_hostname(card->host),
>>>              mmc_card_uhs(card) ? "ultra high speed " :
>>>              (mmc_card_hs(card) ? "high speed " : ""),
>>>              mmc_card_hs400(card) ? "HS400 " :
>>>              (mmc_card_hs200(card) ? "HS200 " : ""),
>>> +            mmc_card_hs400es(card) ? "Enhanced strobe " : "",
>>>              mmc_card_ddr52(card) ? "DDR " : "",
>>>              uhs_bus_speed_mode, type, card->rca);
>>>      }
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 99275e4..b0c1f07 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1127,6 +1127,15 @@ void mmc_set_initial_state(struct mmc_host *host)
>>>      host->ios.bus_width = MMC_BUS_WIDTH_1;
>>>      host->ios.timing = MMC_TIMING_LEGACY;
>>>      host->ios.drv_type = 0;
>>> +    host->ios.enhanced_strobe = false;
>>> +
>>> +    /*
>>> +     * Make sure we are in non-enhanced strobe mode before we
>>> +     * actually enable it in ext_csd.
>>> +     */
>>> +    if ((host->caps2 & MMC_CAP2_HS400_ES) &&
>>> +         host->ops->hs400_enhanced_strobe)
>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>
>>
>> I wonder what is enhanced_strobe's purpose.
>> When i saw your patches, this is used to check whether timing is strobe or not in sdhci-of-arasan, right?
> 
> 
> Also we use it for debugfs.
> 
>> If ios.enhanced_strobe is used only this purpose, is it possible to use the below?
>>
>> host->ops->hs400_enhanced_strobe(host, false/true); ?
>>
>> This callback is also called only two locations..init (false) and switch hs400es (true).
> 
> It was suggested by Adrian. I'm ok with both of them.

I don't mind what using.. :) It's your preference.

> 
>>
>>>
>>>      mmc_set_ios(host);
>>>  }
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f99c47e..9dc91ba 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>          avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>>>      }
>>>
>>> +    if ((caps2 & MMC_CAP2_HS400_ES) &&
>>> +        card->ext_csd.strobe_support &&
>>> +        (avail_type & EXT_CSD_CARD_TYPE_HS400))
>>> +        avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>>> +
>>>      card->ext_csd.hs_max_dtr = hs_max_dtr;
>>>      card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>>>      card->mmc_avail_type = avail_type;
>>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>              mmc_card_set_blockaddr(card);
>>>      }
>>>
>>> +    card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>>>      card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>>>      mmc_select_card_type(card);
>>>
>>> @@ -1216,6 +1222,78 @@ out_err:
>>>      return err;
>>>  }
>>>
>>> +static int mmc_select_hs400es(struct mmc_card *card)
>>> +{
>>> +    struct mmc_host *host = card->host;
>>> +    int err = 0;
>>> +    u8 val;
>>> +
>>> +    if (!(host->caps & MMC_CAP_8_BIT_DATA)) {
>>> +        err = -ENOTSUPP;
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    err = mmc_select_bus_width(card);
>>> +    if (IS_ERR_VALUE(err))
>>> +        goto out_err;
>>> +
>>> +    /* Switch card to HS mode */
>>> +    err = mmc_select_hs(card);
>>> +    if (err) {
>>> +        pr_err("%s: switch to high-speed failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    err = mmc_switch_status(card);
>>> +    if (err)
>>> +        goto out_err;
>>> +
>>> +    /* Switch card to DDR with strobe bit */
>>> +    val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>>> +    err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>> +             EXT_CSD_BUS_WIDTH,
>>> +             val,
>>> +             card->ext_csd.generic_cmd6_time);
>>> +    if (err) {
>>> +        pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    /* Switch card to HS400 */
>>> +    val = EXT_CSD_TIMING_HS400 |
>>> +          card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>>> +    err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>> +               EXT_CSD_HS_TIMING, val,
>>> +               card->ext_csd.generic_cmd6_time,
>>> +               true, false, true);
>>> +    if (err) {
>>> +        pr_err("%s: switch to hs400es failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    /* Set host controller to HS400 timing and frequency */
>>> +    mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>>> +
>>> +    /* Controller enable enhanced strobe function */
>>> +    host->ios.enhanced_strobe = true;
>>> +    if (host->ops->hs400_enhanced_strobe)
>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>> +
>>> +    err = mmc_switch_status(card);
>>> +    if (err)
>>> +        goto out_err;
>>> +
>>> +    return 0;
>>> +
>>> +out_err:
>>
>> If ios.ehanced_strobe is really needs, you need to set from true to false at here?
> 
> I don't think so. If failing to run into HS400es, I'd not expect it
> fallback to HS400, and why not let mmc core retring it. We currently
> don't have speed-mode fallback mechanism,  right?

Ok. I have tested with my exynos board. It's working fine.

Tested-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
Reviewed-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>

Best Regards,
Jaehoon Chung

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +    pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
>>> +           __func__, err);
>>> +    return err;
>>> +}
>>> +
>>>  static void mmc_select_driver_type(struct mmc_card *card)
>>>  {
>>>      int card_drv_type, drive_strength, drv_type;
>>> @@ -1297,7 +1375,7 @@ err:
>>>  }
>>>
>>>  /*
>>> - * Activate High Speed or HS200 mode if supported.
>>> + * Activate High Speed, HS200 or HS400ES mode if supported.
>>>   */
>>>  static int mmc_select_timing(struct mmc_card *card)
>>>  {
>>> @@ -1306,7 +1384,9 @@ static int mmc_select_timing(struct mmc_card *card)
>>>      if (!mmc_can_ext_csd(card))
>>>          goto bus_speed;
>>>
>>> -    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>> +    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
>>> +        err = mmc_select_hs400es(card);
>>> +    else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>>          err = mmc_select_hs200(card);
>>>      else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>>>          err = mmc_select_hs(card);
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index eb0151b..22defc2 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>>>      u8            raw_partition_support;    /* 160 */
>>>      u8            raw_rpmb_size_mult;    /* 168 */
>>>      u8            raw_erased_mem_count;    /* 181 */
>>> +    u8            strobe_support;        /* 184 */
>>>      u8            raw_ext_csd_structure;    /* 194 */
>>>      u8            raw_card_type;        /* 196 */
>>>      u8            raw_driver_strength;    /* 197 */
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index 2a06fb0..0eabb72 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -19,6 +19,7 @@
>>>
>>>  #include <linux/mmc/core.h>
>>>  #include <linux/mmc/card.h>
>>> +#include <linux/mmc/mmc.h>
>>>  #include <linux/mmc/pm.h>
>>>
>>>  struct mmc_ios {
>>> @@ -77,6 +78,8 @@ struct mmc_ios {
>>>  #define MMC_SET_DRIVER_TYPE_A    1
>>>  #define MMC_SET_DRIVER_TYPE_C    2
>>>  #define MMC_SET_DRIVER_TYPE_D    3
>>> +
>>> +    bool enhanced_strobe;            /* hs400es selection */
>>>  };
>>>
>>>  struct mmc_host_ops {
>>> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>>>
>>>      /* Prepare HS400 target operating frequency depending host driver */
>>>      int    (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
>>> +    /* Prepare enhanced strobe depending host driver */
>>> +    void    (*hs400_enhanced_strobe)(struct mmc_host *host,
>>> +                     struct mmc_ios *ios);
>>>      int    (*select_drive_strength)(struct mmc_card *card,
>>>                       unsigned int max_dtr, int host_drv,
>>>                       int card_drv, int *drv_type);
>>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>>>      return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>>>  }
>>>
>>> +static inline bool mmc_card_hs400es(struct mmc_card *card)
>>> +{
>>> +    return card->host->ios.enhanced_strobe;
>>> +}
>>> +
>>>  void mmc_retune_timer_stop(struct mmc_host *host);
>>>
>>>  static inline void mmc_retune_needed(struct mmc_host *host)
>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>> index 15f2c4a..c376209 100644
>>> --- a/include/linux/mmc/mmc.h
>>> +++ b/include/linux/mmc/mmc.h
>>> @@ -297,6 +297,7 @@ struct _mmc_csd {
>>>  #define EXT_CSD_PART_CONFIG        179    /* R/W */
>>>  #define EXT_CSD_ERASED_MEM_CONT        181    /* RO */
>>>  #define EXT_CSD_BUS_WIDTH        183    /* R/W */
>>> +#define EXT_CSD_STROBE_SUPPORT        184    /* RO */
>>>  #define EXT_CSD_HS_TIMING        185    /* R/W */
>>>  #define EXT_CSD_POWER_CLASS        187    /* R/W */
>>>  #define EXT_CSD_REV            192    /* RO */
>>> @@ -380,12 +381,14 @@ struct _mmc_csd {
>>>  #define EXT_CSD_CARD_TYPE_HS400_1_2V    (1<<7)    /* Card can run at 200MHz DDR, 1.2V */
>>>  #define EXT_CSD_CARD_TYPE_HS400        (EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>>                       EXT_CSD_CARD_TYPE_HS400_1_2V)
>>> +#define EXT_CSD_CARD_TYPE_HS400ES    (1<<8)    /* Card can run at HS400ES */
>>>
>>>  #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 */
>>>  #define EXT_CSD_BUS_WIDTH_8    2    /* Card is in 8 bit mode */
>>>  #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_BUS_WIDTH_STROBE BIT(7)    /* Enhanced strobe mode */
>>>
>>>  #define EXT_CSD_TIMING_BC    0    /* Backwards compatility */
>>>  #define EXT_CSD_TIMING_HS    1    /* High speed */
>>>
>>
>>
>>
>>
> 
> 

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



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux