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

[Shawn Lin]

Hi

在 2016-5-20 11:54, Jaehoon Chung 写道:

On 05/20/2016 12:15 PM, Shawn Lin wrote:

Hi

在 2016-5-20 10:45, Jaehoon Chung 写道:

On 05/10/2016 06:09 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 
---

Changes in v3: None
Changes in v2: None

   drivers/mmc/core/bus.c   |  3 +-
   drivers/mmc/core/core.c  |  8 +
   drivers/mmc/core/mmc.c   | 79 
++--
   include/linux/mmc/card.h |  1 +
   include/linux/mmc/host.h | 11 +++
   include/linux/mmc/mmc.h  |  3 ++
   6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+host->ops->hs400_enhanced_strobe(host, >ios);

   mmc_set_ios(host);
   }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+(card_type & EXT_CSD_CARD_TYPE_HS400))
+avail_type |= EXT_CSD_CARD_TYPE_HS400ES;


Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.



Ahh, right.
I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
"card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
EXT_CSD_CARD_TYPE_HS400" must be true.


Otherwise, you can check or add the capabilities in the parse_of_mmc().
It can choose according to your preference. :)

Anyway, i have tested your patches..If you send the patch V4, i will also test.


Thanks for testing. I will cc you when pushing V4.



Best Regards,
Jaehoon Chung




Thansk for catching this!



if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
 if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung


+
   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,73 @@ out_err:
   return err;
   }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+struct mmc_host *host = card->host;
+int err = 0;
+u8 val;
+
+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,
+ 

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

[Shawn Lin]

Hi

在 2016-5-20 11:54, Jaehoon Chung 写道:

On 05/20/2016 12:15 PM, Shawn Lin wrote:

Hi

在 2016-5-20 10:45, Jaehoon Chung 写道:

On 05/10/2016 06:09 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 
---

Changes in v3: None
Changes in v2: None

   drivers/mmc/core/bus.c   |  3 +-
   drivers/mmc/core/core.c  |  8 +
   drivers/mmc/core/mmc.c   | 79 
++--
   include/linux/mmc/card.h |  1 +
   include/linux/mmc/host.h | 11 +++
   include/linux/mmc/mmc.h  |  3 ++
   6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+host->ops->hs400_enhanced_strobe(host, >ios);

   mmc_set_ios(host);
   }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+(card_type & EXT_CSD_CARD_TYPE_HS400))
+avail_type |= EXT_CSD_CARD_TYPE_HS400ES;


Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.



Ahh, right.
I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
"card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
EXT_CSD_CARD_TYPE_HS400" must be true.


Otherwise, you can check or add the capabilities in the parse_of_mmc().
It can choose according to your preference. :)

Anyway, i have tested your patches..If you send the patch V4, i will also test.


Thanks for testing. I will cc you when pushing V4.



Best Regards,
Jaehoon Chung




Thansk for catching this!



if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
 if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung


+
   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,73 @@ out_err:
   return err;
   }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+struct mmc_host *host = card->host;
+int err = 0;
+u8 val;
+
+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,
+ 

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

[Jaehoon Chung]

On 05/20/2016 12:15 PM, Shawn Lin wrote:
> Hi
> 
> 在 2016-5-20 10:45, Jaehoon Chung 写道:
>> On 05/10/2016 06:09 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 
>>> ---
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>   drivers/mmc/core/bus.c   |  3 +-
>>>   drivers/mmc/core/core.c  |  8 +
>>>   drivers/mmc/core/mmc.c   | 79 
>>> ++--
>>>   include/linux/mmc/card.h |  1 +
>>>   include/linux/mmc/host.h | 11 +++
>>>   include/linux/mmc/mmc.h  |  3 ++
>>>   6 files changed, 102 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index 4bc48f1..7e94b9d 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..a559501 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
>>> +host->ops->hs400_enhanced_strobe(host, >ios);
>>>
>>>   mmc_set_ios(host);
>>>   }
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f99c47e..c2d1981 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 &&
>>> +(card_type & EXT_CSD_CARD_TYPE_HS400))
>>> +avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>>
>> Does it need to check whether support HS400_1.8V/1.2V or not?
>> It should be more stable than now.
>>
> 
> Ahh, right.
> I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
> "card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
> EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
> EXT_CSD_CARD_TYPE_HS400" must be true.

Otherwise, you can check or add the capabilities in the parse_of_mmc().
It can choose according to your preference. :)

Anyway, i have tested your patches..If you send the patch V4, i will also test.

Best Regards,
Jaehoon Chung

> 
> 
> Thansk for catching this!
> 
> 
>> if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | 
>> EXT_CSD_CARD_TYPE_HS400_1.2V)) {
>> if ((cap2 & MMC_CAP2_HS400_ES) && ...
>> }
>>
>> how about this?
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>>   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,73 @@ out_err:
>>>   return err;
>>>   }
>>>
>>> +static int mmc_select_hs400es(struct mmc_card *card)
>>> +{
>>> +struct mmc_host *host = card->host;
>>> +int err = 0;
>>> +u8 val;
>>> +
>>> +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);
>>> +  

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

[Jaehoon Chung]

On 05/20/2016 12:15 PM, Shawn Lin wrote:
> Hi
> 
> 在 2016-5-20 10:45, Jaehoon Chung 写道:
>> On 05/10/2016 06:09 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 
>>> ---
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>   drivers/mmc/core/bus.c   |  3 +-
>>>   drivers/mmc/core/core.c  |  8 +
>>>   drivers/mmc/core/mmc.c   | 79 
>>> ++--
>>>   include/linux/mmc/card.h |  1 +
>>>   include/linux/mmc/host.h | 11 +++
>>>   include/linux/mmc/mmc.h  |  3 ++
>>>   6 files changed, 102 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index 4bc48f1..7e94b9d 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..a559501 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
>>> +host->ops->hs400_enhanced_strobe(host, >ios);
>>>
>>>   mmc_set_ios(host);
>>>   }
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f99c47e..c2d1981 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 &&
>>> +(card_type & EXT_CSD_CARD_TYPE_HS400))
>>> +avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>>
>> Does it need to check whether support HS400_1.8V/1.2V or not?
>> It should be more stable than now.
>>
> 
> Ahh, right.
> I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
> "card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
> EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
> EXT_CSD_CARD_TYPE_HS400" must be true.

Otherwise, you can check or add the capabilities in the parse_of_mmc().
It can choose according to your preference. :)

Anyway, i have tested your patches..If you send the patch V4, i will also test.

Best Regards,
Jaehoon Chung

> 
> 
> Thansk for catching this!
> 
> 
>> if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | 
>> EXT_CSD_CARD_TYPE_HS400_1.2V)) {
>> if ((cap2 & MMC_CAP2_HS400_ES) && ...
>> }
>>
>> how about this?
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>>   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,73 @@ out_err:
>>>   return err;
>>>   }
>>>
>>> +static int mmc_select_hs400es(struct mmc_card *card)
>>> +{
>>> +struct mmc_host *host = card->host;
>>> +int err = 0;
>>> +u8 val;
>>> +
>>> +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;
>>> + 

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

[Shawn Lin]

Hi

在 2016-5-20 10:45, Jaehoon Chung 写道:

On 05/10/2016 06:09 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 
---

Changes in v3: None
Changes in v2: None

  drivers/mmc/core/bus.c   |  3 +-
  drivers/mmc/core/core.c  |  8 +
  drivers/mmc/core/mmc.c   | 79 ++--
  include/linux/mmc/card.h |  1 +
  include/linux/mmc/host.h | 11 +++
  include/linux/mmc/mmc.h  |  3 ++
  6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+   host->ops->hs400_enhanced_strobe(host, >ios);

mmc_set_ios(host);
  }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+   (card_type & EXT_CSD_CARD_TYPE_HS400))
+   avail_type |= EXT_CSD_CARD_TYPE_HS400ES;


Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.



Ahh, right.
I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
"card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
EXT_CSD_CARD_TYPE_HS400" must be true.



Thansk for catching this!



if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung


+
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,73 @@ out_err:
return err;
  }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+   struct mmc_host *host = card->host;
+   int err = 0;
+   u8 val;
+
+   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 

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

[Shawn Lin]

Hi

在 2016-5-20 10:45, Jaehoon Chung 写道:

On 05/10/2016 06:09 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 
---

Changes in v3: None
Changes in v2: None

  drivers/mmc/core/bus.c   |  3 +-
  drivers/mmc/core/core.c  |  8 +
  drivers/mmc/core/mmc.c   | 79 ++--
  include/linux/mmc/card.h |  1 +
  include/linux/mmc/host.h | 11 +++
  include/linux/mmc/mmc.h  |  3 ++
  6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+   host->ops->hs400_enhanced_strobe(host, >ios);

mmc_set_ios(host);
  }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+   (card_type & EXT_CSD_CARD_TYPE_HS400))
+   avail_type |= EXT_CSD_CARD_TYPE_HS400ES;


Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.



Ahh, right.
I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
"card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
EXT_CSD_CARD_TYPE_HS400" must be true.



Thansk for catching this!



if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung


+
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,73 @@ out_err:
return err;
  }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+   struct mmc_host *host = card->host;
+   int err = 0;
+   u8 val;
+
+   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 

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

[Jaehoon Chung]

On 05/10/2016 06:09 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 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +
>  drivers/mmc/core/mmc.c   | 79 
> ++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 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..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
> + host->ops->hs400_enhanced_strobe(host, >ios);
>  
>   mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 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 &&
> + (card_type & EXT_CSD_CARD_TYPE_HS400))
> + avail_type |= EXT_CSD_CARD_TYPE_HS400ES;

Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.

if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung

> +
>   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,73 @@ out_err:
>   return err;
>  }
>  
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> + struct mmc_host *host = card->host;
> + int err = 0;
> + u8 val;
> +
> + 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 |
> + 

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

[Jaehoon Chung]

On 05/10/2016 06:09 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 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +
>  drivers/mmc/core/mmc.c   | 79 
> ++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 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..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
> + host->ops->hs400_enhanced_strobe(host, >ios);
>  
>   mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 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 &&
> + (card_type & EXT_CSD_CARD_TYPE_HS400))
> + avail_type |= EXT_CSD_CARD_TYPE_HS400ES;

Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.

if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) 
{
if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung

> +
>   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,73 @@ out_err:
>   return err;
>  }
>  
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> + struct mmc_host *host = card->host;
> + int err = 0;
> + u8 val;
> +
> + 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 << 

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

[Doug Anderson]

Shawn,

On Thu, May 19, 2016 at 6:48 PM, Shawn Lin  wrote:
>>>   #define EXT_CSD_BUS_WIDTH_10   /* Card is in 1 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_41   /* Card is in 4 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_82   /* Card is in 8 bit mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_45   /* Card is in 4 bit DDR
>>> mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_86   /* Card is in 8 bit DDR
>>> mode */
>>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)/* Enhanced strobe mode
>>> */
>>
>>
>> nit: while your code should be OK, it seems better to be BIT(3), since
>> above is really:
>>
>
> I don't unstand here. EXT_CSD_BUS_WIDTH_STROBE is the MSB of
> BUS_WIDTH[183] which is a 8-bit reg.

I totally misunderstood.  You are correct.

-Doug

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

[Doug Anderson]

Shawn,

On Thu, May 19, 2016 at 6:48 PM, Shawn Lin  wrote:
>>>   #define EXT_CSD_BUS_WIDTH_10   /* Card is in 1 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_41   /* Card is in 4 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_82   /* Card is in 8 bit mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_45   /* Card is in 4 bit DDR
>>> mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_86   /* Card is in 8 bit DDR
>>> mode */
>>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)/* Enhanced strobe mode
>>> */
>>
>>
>> nit: while your code should be OK, it seems better to be BIT(3), since
>> above is really:
>>
>
> I don't unstand here. EXT_CSD_BUS_WIDTH_STROBE is the MSB of
> BUS_WIDTH[183] which is a 8-bit reg.

I totally misunderstood.  You are correct.

-Doug

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

[Shawn Lin]

Hi

在 2016-5-20 8:18, Doug Anderson 写道:

Hi,

On Tue, May 10, 2016 at 2:09 AM, 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.


I don't totally understand it, but two boards I'm working with have
eMMC devices that claim to be eMMC 5.0 devices but also have support
for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
their eMMC 5.0 devices?


Presumably I guess your "support for strobe" means "support for
enhanced strobe", right?

hs400es is also optional for emmc 5.1, so it's impossible to find a
emmc 5.0 claiming to support hs400es.

Coud you share me the device's ID number on its package or from the
device's manual?






Signed-off-by: Shawn Lin 
---

Changes in v3: None
Changes in v2: None

  drivers/mmc/core/bus.c   |  3 +-
  drivers/mmc/core/core.c  |  8 +
  drivers/mmc/core/mmc.c   | 79 ++--
  include/linux/mmc/card.h |  1 +
  include/linux/mmc/host.h | 11 +++
  include/linux/mmc/mmc.h  |  3 ++
  6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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" : "",


Nit: there should be a space before the quote.  Otherwise the message
runs together, like:

new HS400 Enhanced strobeMMC card at address 0001


Good catch, thanks.





 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+   host->ops->hs400_enhanced_strobe(host, >ios);

 mmc_set_ios(host);
  }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+   (card_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,73 @@ out_err:
 return err;
  }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+   struct mmc_host *host = card->host;
+   int err = 0;
+   u8 val;
+
+   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;


In the hs400 function earlier in the file 

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

[Shawn Lin]

Hi

在 2016-5-20 8:18, Doug Anderson 写道:

Hi,

On Tue, May 10, 2016 at 2:09 AM, 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.


I don't totally understand it, but two boards I'm working with have
eMMC devices that claim to be eMMC 5.0 devices but also have support
for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
their eMMC 5.0 devices?


Presumably I guess your "support for strobe" means "support for
enhanced strobe", right?

hs400es is also optional for emmc 5.1, so it's impossible to find a
emmc 5.0 claiming to support hs400es.

Coud you share me the device's ID number on its package or from the
device's manual?






Signed-off-by: Shawn Lin 
---

Changes in v3: None
Changes in v2: None

  drivers/mmc/core/bus.c   |  3 +-
  drivers/mmc/core/core.c  |  8 +
  drivers/mmc/core/mmc.c   | 79 ++--
  include/linux/mmc/card.h |  1 +
  include/linux/mmc/host.h | 11 +++
  include/linux/mmc/mmc.h  |  3 ++
  6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 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" : "",


Nit: there should be a space before the quote.  Otherwise the message
runs together, like:

new HS400 Enhanced strobeMMC card at address 0001


Good catch, thanks.





 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..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
+   host->ops->hs400_enhanced_strobe(host, >ios);

 mmc_set_ios(host);
  }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 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 &&
+   (card_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,73 @@ out_err:
 return err;
  }

+static int mmc_select_hs400es(struct mmc_card *card)
+{
+   struct mmc_host *host = card->host;
+   int err = 0;
+   u8 val;
+
+   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;


In the hs400 function earlier in the file the check to confirm that
the bus width is 8 before 

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

[Doug Anderson]

Hi,

On Tue, May 10, 2016 at 2:09 AM, 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.

I don't totally understand it, but two boards I'm working with have
eMMC devices that claim to be eMMC 5.0 devices but also have support
for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
their eMMC 5.0 devices?


> Signed-off-by: Shawn Lin 
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +
>  drivers/mmc/core/mmc.c   | 79 
> ++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 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" : "",

Nit: there should be a space before the quote.  Otherwise the message
runs together, like:

new HS400 Enhanced strobeMMC card at address 0001


> 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..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
> +   host->ops->hs400_enhanced_strobe(host, >ios);
>
> mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 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 &&
> +   (card_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,73 @@ out_err:
> return err;
>  }
>
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> +   struct mmc_host *host = card->host;
> +   int err = 0;
> +   u8 val;
> +
> +   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;

In the hs400 function earlier in the file the check to confirm that
the bus width is 8 before doing this.  Should we do that here, too?

> +   err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +  

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

[Doug Anderson]

Hi,

On Tue, May 10, 2016 at 2:09 AM, 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.

I don't totally understand it, but two boards I'm working with have
eMMC devices that claim to be eMMC 5.0 devices but also have support
for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
their eMMC 5.0 devices?


> Signed-off-by: Shawn Lin 
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +
>  drivers/mmc/core/mmc.c   | 79 
> ++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 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" : "",

Nit: there should be a space before the quote.  Otherwise the message
runs together, like:

new HS400 Enhanced strobeMMC card at address 0001


> 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..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ 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->ops->hs400_enhanced_strobe)
> +   host->ops->hs400_enhanced_strobe(host, >ios);
>
> mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 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 &&
> +   (card_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,73 @@ out_err:
> return err;
>  }
>
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> +   struct mmc_host *host = card->host;
> +   int err = 0;
> +   u8 val;
> +
> +   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;

In the hs400 function earlier in the file the check to confirm that
the bus width is 8 before doing this.  Should we do that here, too?

> +   err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +EXT_CSD_BUS_WIDTH,
> +