diff mbox

[1/7] mmc: clarify DDR timing mode between SD-UHS and eMMC

Message ID 001f01cf11fb$93d188c0$bb749a40$%jun@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Seungwon Jeon Jan. 15, 2014, 2:10 p.m. UTC
This change distinguishes DDR timing mode of current
mixed usage to clarify device type.

Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
 drivers/mmc/core/debugfs.c |    3 +++
 drivers/mmc/core/mmc.c     |    2 +-
 include/linux/mmc/host.h   |    3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

Comments

Ulf Hansson Jan. 16, 2014, 10:50 a.m. UTC | #1
On 15 January 2014 15:10, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> This change distinguishes DDR timing mode of current
> mixed usage to clarify device type.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/core/debugfs.c |    3 +++
>  drivers/mmc/core/mmc.c     |    2 +-
>  include/linux/mmc/host.h   |    3 ++-
>  3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 54829c0..509229b 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -135,6 +135,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>         case MMC_TIMING_UHS_DDR50:
>                 str = "sd uhs DDR50";
>                 break;
> +       case MMC_TIMING_MMC_DDR52:
> +               str = "mmc DDR52";
> +               break;
>         case MMC_TIMING_MMC_HS200:
>                 str = "mmc high-speed SDR200";
>                 break;
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 98e9eb0..6d91ff7 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1261,7 +1261,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>                                         goto err;
>                         }
>                         mmc_card_set_ddr_mode(card);
> -                       mmc_set_timing(card->host, MMC_TIMING_UHS_DDR50);
> +                       mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>                         mmc_set_bus_width(card->host, bus_width);
>                 }
>         }
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 99f5709..87b1f4f 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -58,7 +58,8 @@ struct mmc_ios {
>  #define MMC_TIMING_UHS_SDR50   5
>  #define MMC_TIMING_UHS_SDR104  6
>  #define MMC_TIMING_UHS_DDR50   7
> -#define MMC_TIMING_MMC_HS200   8
> +#define MMC_TIMING_MMC_DDR52   8
> +#define MMC_TIMING_MMC_HS200   9
>
>  #define MMC_SDR_MODE           0
>  #define MMC_1_2V_DDR_MODE      1
> --
> 1.7.0.4
>
>
--
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
Ulf Hansson Jan. 17, 2014, 9:22 p.m. UTC | #2
On 16 January 2014 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 15 January 2014 15:10, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> This change distinguishes DDR timing mode of current
>> mixed usage to clarify device type.
>>
>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>
>> ---
>>  drivers/mmc/core/debugfs.c |    3 +++
>>  drivers/mmc/core/mmc.c     |    2 +-
>>  include/linux/mmc/host.h   |    3 ++-
>>  3 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
>> index 54829c0..509229b 100644
>> --- a/drivers/mmc/core/debugfs.c
>> +++ b/drivers/mmc/core/debugfs.c
>> @@ -135,6 +135,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>>         case MMC_TIMING_UHS_DDR50:
>>                 str = "sd uhs DDR50";
>>                 break;
>> +       case MMC_TIMING_MMC_DDR52:
>> +               str = "mmc DDR52";

Just a minor thought. In the eMMC spec there are no such thing as
DDR52 mode. Only DDR is mentioned, should we maybe change to
"MMC_TIMING_MMC_DDR" to better reflect the spec?

Kind regards
Ulf Hansson

>> +               break;
>>         case MMC_TIMING_MMC_HS200:
>>                 str = "mmc high-speed SDR200";
>>                 break;
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 98e9eb0..6d91ff7 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1261,7 +1261,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>                                         goto err;
>>                         }
>>                         mmc_card_set_ddr_mode(card);
>> -                       mmc_set_timing(card->host, MMC_TIMING_UHS_DDR50);
>> +                       mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>>                         mmc_set_bus_width(card->host, bus_width);
>>                 }
>>         }
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 99f5709..87b1f4f 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -58,7 +58,8 @@ struct mmc_ios {
>>  #define MMC_TIMING_UHS_SDR50   5
>>  #define MMC_TIMING_UHS_SDR104  6
>>  #define MMC_TIMING_UHS_DDR50   7
>> -#define MMC_TIMING_MMC_HS200   8
>> +#define MMC_TIMING_MMC_DDR52   8
>> +#define MMC_TIMING_MMC_HS200   9
>>
>>  #define MMC_SDR_MODE           0
>>  #define MMC_1_2V_DDR_MODE      1
>> --
>> 1.7.0.4
>>
>>
--
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
Seungwon Jeon Jan. 20, 2014, 3:55 a.m. UTC | #3
Hi Ulf,

On Sat, January 18, 2014, Ulf Hansson wrote:
> On 16 January 2014 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > On 15 January 2014 15:10, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> >> This change distinguishes DDR timing mode of current
> >> mixed usage to clarify device type.
> >>
> >> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> >
> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> >
> >> ---
> >>  drivers/mmc/core/debugfs.c |    3 +++
> >>  drivers/mmc/core/mmc.c     |    2 +-
> >>  include/linux/mmc/host.h   |    3 ++-
> >>  3 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> >> index 54829c0..509229b 100644
> >> --- a/drivers/mmc/core/debugfs.c
> >> +++ b/drivers/mmc/core/debugfs.c
> >> @@ -135,6 +135,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
> >>         case MMC_TIMING_UHS_DDR50:
> >>                 str = "sd uhs DDR50";
> >>                 break;
> >> +       case MMC_TIMING_MMC_DDR52:
> >> +               str = "mmc DDR52";
> 
> Just a minor thought. In the eMMC spec there are no such thing as
> DDR52 mode. Only DDR is mentioned, should we maybe change to
> "MMC_TIMING_MMC_DDR" to better reflect the spec?
In eMMC spec, DDR mode for high speed is up to 52MHz unlike SD.
Actually DDR52 is appeared in spec. And it helps to ensure the meaning.
Also, for distinguishing from HS200's DDR mode(HS400), it would be not bad to keep it.
It makes sense?

Thanks,
Seungwon Jeon

> 
> Kind regards
> Ulf Hansson
> 
> >> +               break;
> >>         case MMC_TIMING_MMC_HS200:
> >>                 str = "mmc high-speed SDR200";
> >>                 break;
> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >> index 98e9eb0..6d91ff7 100644
> >> --- a/drivers/mmc/core/mmc.c
> >> +++ b/drivers/mmc/core/mmc.c
> >> @@ -1261,7 +1261,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >>                                         goto err;
> >>                         }
> >>                         mmc_card_set_ddr_mode(card);
> >> -                       mmc_set_timing(card->host, MMC_TIMING_UHS_DDR50);
> >> +                       mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
> >>                         mmc_set_bus_width(card->host, bus_width);
> >>                 }
> >>         }
> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> >> index 99f5709..87b1f4f 100644
> >> --- a/include/linux/mmc/host.h
> >> +++ b/include/linux/mmc/host.h
> >> @@ -58,7 +58,8 @@ struct mmc_ios {
> >>  #define MMC_TIMING_UHS_SDR50   5
> >>  #define MMC_TIMING_UHS_SDR104  6
> >>  #define MMC_TIMING_UHS_DDR50   7
> >> -#define MMC_TIMING_MMC_HS200   8
> >> +#define MMC_TIMING_MMC_DDR52   8
> >> +#define MMC_TIMING_MMC_HS200   9
> >>
> >>  #define MMC_SDR_MODE           0
> >>  #define MMC_1_2V_DDR_MODE      1
> >> --
> >> 1.7.0.4
> >>
> >>
> --
> 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

--
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
Ulf Hansson Jan. 23, 2014, 9:06 a.m. UTC | #4
On 20 January 2014 04:55, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Hi Ulf,
>
> On Sat, January 18, 2014, Ulf Hansson wrote:
>> On 16 January 2014 11:50, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> > On 15 January 2014 15:10, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> >> This change distinguishes DDR timing mode of current
>> >> mixed usage to clarify device type.
>> >>
>> >> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> >
>> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >
>> >> ---
>> >>  drivers/mmc/core/debugfs.c |    3 +++
>> >>  drivers/mmc/core/mmc.c     |    2 +-
>> >>  include/linux/mmc/host.h   |    3 ++-
>> >>  3 files changed, 6 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
>> >> index 54829c0..509229b 100644
>> >> --- a/drivers/mmc/core/debugfs.c
>> >> +++ b/drivers/mmc/core/debugfs.c
>> >> @@ -135,6 +135,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>> >>         case MMC_TIMING_UHS_DDR50:
>> >>                 str = "sd uhs DDR50";
>> >>                 break;
>> >> +       case MMC_TIMING_MMC_DDR52:
>> >> +               str = "mmc DDR52";
>>
>> Just a minor thought. In the eMMC spec there are no such thing as
>> DDR52 mode. Only DDR is mentioned, should we maybe change to
>> "MMC_TIMING_MMC_DDR" to better reflect the spec?
> In eMMC spec, DDR mode for high speed is up to 52MHz unlike SD.
> Actually DDR52 is appeared in spec. And it helps to ensure the meaning.
> Also, for distinguishing from HS200's DDR mode(HS400), it would be not bad to keep it.
> It makes sense?
>

Diving a bit deeper into the eMMC spec 5.0.

DDR52 is mentioned, you are correct, but it is not listed as a bus
speed mode. Have a look at chapter 5.3.2 Bus Speed Modes, there you
find the more correct name. "High Speed DDR".

"High Speed" indicates the interface can run up to 52 MHz. So maybe
the most proper name for this would be "MMC_TIMING_MMC_HS_DDR"? Still
I have no strong opinion, so if you favor MMC_TIMING_MMC_DDR52, I am
fine with it.

Kind regards
Ulf Hansson

> Thanks,
> Seungwon Jeon
>
>>
>> Kind regards
>> Ulf Hansson
>>
>> >> +               break;
>> >>         case MMC_TIMING_MMC_HS200:
>> >>                 str = "mmc high-speed SDR200";
>> >>                 break;
>> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> >> index 98e9eb0..6d91ff7 100644
>> >> --- a/drivers/mmc/core/mmc.c
>> >> +++ b/drivers/mmc/core/mmc.c
>> >> @@ -1261,7 +1261,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>> >>                                         goto err;
>> >>                         }
>> >>                         mmc_card_set_ddr_mode(card);
>> >> -                       mmc_set_timing(card->host, MMC_TIMING_UHS_DDR50);
>> >> +                       mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> >>                         mmc_set_bus_width(card->host, bus_width);
>> >>                 }
>> >>         }
>> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> >> index 99f5709..87b1f4f 100644
>> >> --- a/include/linux/mmc/host.h
>> >> +++ b/include/linux/mmc/host.h
>> >> @@ -58,7 +58,8 @@ struct mmc_ios {
>> >>  #define MMC_TIMING_UHS_SDR50   5
>> >>  #define MMC_TIMING_UHS_SDR104  6
>> >>  #define MMC_TIMING_UHS_DDR50   7
>> >> -#define MMC_TIMING_MMC_HS200   8
>> >> +#define MMC_TIMING_MMC_DDR52   8
>> >> +#define MMC_TIMING_MMC_HS200   9
>> >>
>> >>  #define MMC_SDR_MODE           0
>> >>  #define MMC_1_2V_DDR_MODE      1
>> >> --
>> >> 1.7.0.4
>> >>
>> >>
>> --
>> 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
>
--
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
diff mbox

Patch

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 54829c0..509229b 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -135,6 +135,9 @@  static int mmc_ios_show(struct seq_file *s, void *data)
 	case MMC_TIMING_UHS_DDR50:
 		str = "sd uhs DDR50";
 		break;
+	case MMC_TIMING_MMC_DDR52:
+		str = "mmc DDR52";
+		break;
 	case MMC_TIMING_MMC_HS200:
 		str = "mmc high-speed SDR200";
 		break;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 98e9eb0..6d91ff7 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1261,7 +1261,7 @@  static int mmc_init_card(struct mmc_host *host, u32 ocr,
 					goto err;
 			}
 			mmc_card_set_ddr_mode(card);
-			mmc_set_timing(card->host, MMC_TIMING_UHS_DDR50);
+			mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
 			mmc_set_bus_width(card->host, bus_width);
 		}
 	}
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 99f5709..87b1f4f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -58,7 +58,8 @@  struct mmc_ios {
 #define MMC_TIMING_UHS_SDR50	5
 #define MMC_TIMING_UHS_SDR104	6
 #define MMC_TIMING_UHS_DDR50	7
-#define MMC_TIMING_MMC_HS200	8
+#define MMC_TIMING_MMC_DDR52	8
+#define MMC_TIMING_MMC_HS200	9
 
 #define MMC_SDR_MODE		0
 #define MMC_1_2V_DDR_MODE	1