[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.

It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).

* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s

* with DDR support:
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

[1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
[2] - http://www.spinics.net/lists/linux-mmc/msg34418.html

Stefan Wahren (3):
  DT: bindings: mmc: Add property for 3.3V only support
  mmc: core: add new cap for 3.3V only DDR MMCs
  mmc: mxs-mmc: Implement DDR support

 Documentation/devicetree/bindings/mmc/mmc.txt |    7 +++++--
 drivers/mmc/core/host.c                       |    2 ++
 drivers/mmc/core/mmc.c                        |    6 ++++++
 drivers/mmc/host/mxs-mmc.c                    |   22 ++++++++++++++++++++++
 include/linux/mmc/host.h                      |    1 +
 5 files changed, 36 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Stefan Wahren]

Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 22d1e1f..b2b8960 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -27,8 +27,6 @@ Optional properties:
   logic it is sufficient to not specify wp-gpios property in the absence of a WP
   line.
 - max-frequency: maximum operating clock frequency
-- no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
-  this system, even if the controller claims it is.
 - cap-sd-highspeed: SD high-speed timing is supported
 - cap-mmc-highspeed: MMC high-speed timing is supported
 - sd-uhs-sdr12: SD UHS SDR12 speed is supported
@@ -40,6 +38,7 @@ Optional properties:
 - cap-mmc-hw-reset: eMMC hardware reset is supported
 - cap-sdio-irq: enable SDIO IRQ signalling on this interface
 - full-pwr-cycle: full power cycle of the card is supported
+- mmc-ddr-3_3v: eMMC high-speed DDR mode(3.3V I/O) is supported
 - mmc-ddr-1_8v: eMMC high-speed DDR mode(1.8V I/O) is supported
 - mmc-ddr-1_2v: eMMC high-speed DDR mode(1.2V I/O) is supported
 - mmc-hs200-1_8v: eMMC HS200 mode(1.8V I/O) is supported
@@ -53,6 +52,10 @@ Optional properties:
 - no-sd: controller is limited to send sd cmd during initialization
 - no-mmc: controller is limited to send mmc cmd during initialization
 
+Deprecated properties:
+- no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
+  this system, even if the controller claims it is.
+
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
 line levels. We choose to follow the SDHCI standard, which specifies both those
-- 
1.7.9.5

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Rob Herring]

On Sat, Aug 06, 2016 at 12:55:38PM +0000, Stefan Wahren wrote:
> Currently there is no proper way to define that a MMC host supports
> only 3.3V. The property no-1-8-v is broken and has different meanings
> for different sdhci variants. So add a new property for 3.3V only
> support and mark no-1-8-v as deprecated.

Why is it broken? How would I override a controller saying 1.8V is 
supported and it is not?

Rob

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Stefan Wahren]

Hi,

> Rob Herring <robh@kernel.org> hat am 10. August 2016 um 20:44 geschrieben:
> 
> 
> On Sat, Aug 06, 2016 at 12:55:38PM +0000, Stefan Wahren wrote:
> > Currently there is no proper way to define that a MMC host supports
> > only 3.3V. The property no-1-8-v is broken and has different meanings
> > for different sdhci variants. So add a new property for 3.3V only
> > support and mark no-1-8-v as deprecated.
> 
> Why is it broken? 

i want to quote Ulf Hansson here [1]:

  The problem with the "no-1-8-v" binding is that it's describing what
  the hardware *can't* do. It thus becomes easy to abuse it.

  I suggest we stop using it, we should mark it deprecated.

[1] - http://www.spinics.net/lists/linux-mmc/msg34604.html

> How would I override a controller saying 1.8V is 
> supported and it is not?

Sorry, i'm not sure that i understand your question. In case a board or a MMC
controller doesn't support 1.8V, it usually supports only 3.3V which is the
intension of this patch.

> 
> Rob
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Rob Herring]

On Wed, Aug 10, 2016 at 3:27 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Hi,
>
>> Rob Herring <robh@kernel.org> hat am 10. August 2016 um 20:44 geschrieben:
>>
>>
>> On Sat, Aug 06, 2016 at 12:55:38PM +0000, Stefan Wahren wrote:
>> > Currently there is no proper way to define that a MMC host supports
>> > only 3.3V. The property no-1-8-v is broken and has different meanings
>> > for different sdhci variants. So add a new property for 3.3V only
>> > support and mark no-1-8-v as deprecated.
>>
>> Why is it broken?
>
> i want to quote Ulf Hansson here [1]:
>
>   The problem with the "no-1-8-v" binding is that it's describing what
>   the hardware *can't* do. It thus becomes easy to abuse it.

Sounds like a quirk which is perfectly normal for a property. I'd
agree it should be what the h/w *can* do if h/w didn't have capability
bit that does that.

>   I suggest we stop using it, we should mark it deprecated.
>
> [1] - http://www.spinics.net/lists/linux-mmc/msg34604.html
>
>> How would I override a controller saying 1.8V is
>> supported and it is not?
>
> Sorry, i'm not sure that i understand your question. In case a board or a MMC
> controller doesn't support 1.8V, it usually supports only 3.3V which is the
> intension of this patch.

Some controllers have capability bits saying what voltages they
support, right? And those bits can be wrong (unless firmware sets them
I'd expect that is the common case) which as I read it is what
"no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
present mean and how do they relate to controller capability bits? I
assume they would override the controller bits, but you can only
override the capability bit not set case. I would think the property
not present means use the capability bit, not that that voltage is not
supported. I think you probably need tri-state properties here where
value 1 means supported, 0 means not supported, and not present means
use capability bit (or other method).

Rob

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Shawn Lin]

+ Adrian

Let's queue Adrian here who now maintains SDHCI stuff.

On 2016/8/11 5:39, Rob Herring wrote:
> On Wed, Aug 10, 2016 at 3:27 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Hi,
>>
>>> Rob Herring <robh@kernel.org> hat am 10. August 2016 um 20:44 geschrieben:
>>>
>>>
>>> On Sat, Aug 06, 2016 at 12:55:38PM +0000, Stefan Wahren wrote:
>>>> Currently there is no proper way to define that a MMC host supports
>>>> only 3.3V. The property no-1-8-v is broken and has different meanings
>>>> for different sdhci variants. So add a new property for 3.3V only
>>>> support and mark no-1-8-v as deprecated.
>>>
>>> Why is it broken?
>>
>> i want to quote Ulf Hansson here [1]:
>>
>>   The problem with the "no-1-8-v" binding is that it's describing what
>>   the hardware *can't* do. It thus becomes easy to abuse it.
>
> Sounds like a quirk which is perfectly normal for a property. I'd
> agree it should be what the h/w *can* do if h/w didn't have capability
> bit that does that.
>
>>   I suggest we stop using it, we should mark it deprecated.
>>
>> [1] - http://www.spinics.net/lists/linux-mmc/msg34604.html
>>
>>> How would I override a controller saying 1.8V is
>>> supported and it is not?
>>
>> Sorry, i'm not sure that i understand your question. In case a board or a MMC
>> controller doesn't support 1.8V, it usually supports only 3.3V which is the
>> intension of this patch.
>
> Some controllers have capability bits saying what voltages they
> support, right? And those bits can be wrong (unless firmware sets them
> I'd expect that is the common case) which as I read it is what
> "no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
> present mean and how do they relate to controller capability bits? I
> assume they would override the controller bits, but you can only
> override the capability bit not set case. I would think the property
> not present means use the capability bit, not that that voltage is not
> supported. I think you probably need tri-state properties here where
> value 1 means supported, 0 means not supported, and not present means
> use capability bit (or other method).
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Adrian Hunter]

On 11/08/16 03:48, Shawn Lin wrote:
> + Adrian
> 
> Let's queue Adrian here who now maintains SDHCI stuff.

SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
as I can see, the meaning is still clear: 1.8V will not be used for either
supply or signaling.

SDHCI is complicated because the SDHCI specification does not cover eMMC.
>From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
support for 1.8V signaling is the same as support for one of those modes
(the spec even says as much).  But what happens is that the host controller
can support those modes but the board can't supply 1.8V so the drivers
remove capability for the modes.  Support for 1.8V supply has a capability
bit which drivers can override if necessary but removable SD cards don't
support 1.8V supply anyway, so the issue doesn't arise if the host
controller is only used for uSD cards.

> 
> On 2016/8/11 5:39, Rob Herring wrote:
>> On Wed, Aug 10, 2016 at 3:27 PM, Stefan Wahren <stefan.wahren@i2se.com>
>> wrote:
>>> Hi,
>>>
>>>> Rob Herring <robh@kernel.org> hat am 10. August 2016 um 20:44 geschrieben:
>>>>
>>>>
>>>> On Sat, Aug 06, 2016 at 12:55:38PM +0000, Stefan Wahren wrote:
>>>>> Currently there is no proper way to define that a MMC host supports
>>>>> only 3.3V. The property no-1-8-v is broken and has different meanings
>>>>> for different sdhci variants. So add a new property for 3.3V only
>>>>> support and mark no-1-8-v as deprecated.
>>>>
>>>> Why is it broken?
>>>
>>> i want to quote Ulf Hansson here [1]:
>>>
>>>   The problem with the "no-1-8-v" binding is that it's describing what
>>>   the hardware *can't* do. It thus becomes easy to abuse it.
>>
>> Sounds like a quirk which is perfectly normal for a property. I'd
>> agree it should be what the h/w *can* do if h/w didn't have capability
>> bit that does that.
>>
>>>   I suggest we stop using it, we should mark it deprecated.
>>>
>>> [1] - http://www.spinics.net/lists/linux-mmc/msg34604.html
>>>
>>>> How would I override a controller saying 1.8V is
>>>> supported and it is not?
>>>
>>> Sorry, i'm not sure that i understand your question. In case a board or a
>>> MMC
>>> controller doesn't support 1.8V, it usually supports only 3.3V which is the
>>> intension of this patch.
>>
>> Some controllers have capability bits saying what voltages they
>> support, right? And those bits can be wrong (unless firmware sets them
>> I'd expect that is the common case) which as I read it is what
>> "no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
>> present mean and how do they relate to controller capability bits? I
>> assume they would override the controller bits, but you can only
>> override the capability bit not set case. I would think the property
>> not present means use the capability bit, not that that voltage is not
>> supported. I think you probably need tri-state properties here where
>> value 1 means supported, 0 means not supported, and not present means
>> use capability bit (or other method).
>>
>> Rob
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Ulf Hansson]

On 18 August 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/08/16 03:48, Shawn Lin wrote:
>> + Adrian
>>
>> Let's queue Adrian here who now maintains SDHCI stuff.
>
> SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
> as I can see, the meaning is still clear: 1.8V will not be used for either
> supply or signaling.

Okay.

>
> SDHCI is complicated because the SDHCI specification does not cover eMMC.
> From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
> support for 1.8V signaling is the same as support for one of those modes
> (the spec even says as much).  But what happens is that the host controller
> can support those modes but the board can't supply 1.8V so the drivers
> remove capability for the modes.  Support for 1.8V supply has a capability
> bit which drivers can override if necessary but removable SD cards don't
> support 1.8V supply anyway, so the issue doesn't arise if the host
> controller is only used for uSD cards.

By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
provided), this becomes a bit messy.

>From Adrian's summary above, it then seems appropriate to limit the
no-1-8-v DT property to apply only to capabilities related to SD
cards, as I assume that also was the original purpose.

Do you think it's possible to clean up this in sdhci when assigning
the caps masks, and then also clarify the no-1-8-v DT binding in the
documentation?

Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
seems we need this to be able to properly describe the HW.
Rob, do you have an issue with adding this binding? I am thinking that
we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
existing pattern.

[...]

Kind regards
Uffe

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Stefan Wahren]

Hi Ulf, hi Rob

> Ulf Hansson <ulf.hansson@linaro.org> hat am 30. August 2016 um 11:26
> geschrieben:
> 
> 
> On 18 August 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote:
> > On 11/08/16 03:48, Shawn Lin wrote:
> >> + Adrian
> >>
> >> Let's queue Adrian here who now maintains SDHCI stuff.
> >
> > SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
> > as I can see, the meaning is still clear: 1.8V will not be used for either
> > supply or signaling.
> 
> Okay.
> 
> >
> > SDHCI is complicated because the SDHCI specification does not cover eMMC.
> > From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
> > support for 1.8V signaling is the same as support for one of those modes
> > (the spec even says as much).  But what happens is that the host controller
> > can support those modes but the board can't supply 1.8V so the drivers
> > remove capability for the modes.  Support for 1.8V supply has a capability
> > bit which drivers can override if necessary but removable SD cards don't
> > support 1.8V supply anyway, so the issue doesn't arise if the host
> > controller is only used for uSD cards.
> 
> By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
> SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
> provided), this becomes a bit messy.
> 
> From Adrian's summary above, it then seems appropriate to limit the
> no-1-8-v DT property to apply only to capabilities related to SD
> cards, as I assume that also was the original purpose.
> 
> Do you think it's possible to clean up this in sdhci when assigning
> the caps masks, and then also clarify the no-1-8-v DT binding in the
> documentation?

was the question addressed to me? I think this clean up should be a separate
patch series. Unfortunately i don't have a clue about what exactly and how it
should be fixed.

> 
> Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
> seems we need this to be able to properly describe the HW.
> Rob, do you have an issue with adding this binding? I am thinking that
> we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
> existing pattern.

@Rob: gently ping ...

Regards
Stefan

> 
> [...]
> 
> Kind regards
> Uffe
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC 1/3] DT: bindings: mmc: Add property for 3.3V only support

[Rob Herring]

On Fri, Sep 02, 2016 at 08:50:28PM +0200, Stefan Wahren wrote:
> Hi Ulf, hi Rob
> 
> > Ulf Hansson <ulf.hansson@linaro.org> hat am 30. August 2016 um 11:26
> > geschrieben:
> > 
> > 
> > On 18 August 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote:
> > > On 11/08/16 03:48, Shawn Lin wrote:
> > >> + Adrian
> > >>
> > >> Let's queue Adrian here who now maintains SDHCI stuff.
> > >
> > > SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
> > > as I can see, the meaning is still clear: 1.8V will not be used for either
> > > supply or signaling.
> > 
> > Okay.
> > 
> > >
> > > SDHCI is complicated because the SDHCI specification does not cover eMMC.
> > > From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
> > > support for 1.8V signaling is the same as support for one of those modes
> > > (the spec even says as much).  But what happens is that the host controller
> > > can support those modes but the board can't supply 1.8V so the drivers
> > > remove capability for the modes.  Support for 1.8V supply has a capability
> > > bit which drivers can override if necessary but removable SD cards don't
> > > support 1.8V supply anyway, so the issue doesn't arise if the host
> > > controller is only used for uSD cards.
> > 
> > By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
> > SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
> > provided), this becomes a bit messy.
> > 
> > From Adrian's summary above, it then seems appropriate to limit the
> > no-1-8-v DT property to apply only to capabilities related to SD
> > cards, as I assume that also was the original purpose.
> > 
> > Do you think it's possible to clean up this in sdhci when assigning
> > the caps masks, and then also clarify the no-1-8-v DT binding in the
> > documentation?
> 
> was the question addressed to me? I think this clean up should be a separate
> patch series. Unfortunately i don't have a clue about what exactly and how it
> should be fixed.
> 
> > 
> > Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
> > seems we need this to be able to properly describe the HW.
> > Rob, do you have an issue with adding this binding? I am thinking that
> > we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
> > existing pattern.
> 
> @Rob: gently ping ...

Yes, this seems fine. I was only the no-1-8-v removal I had issue with.

Rob

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Stefan Wahren]

This patch based on the work of Fabio Estevam:
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"

It adds the support for 3.3V only DDR MMC hosts.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/mmc/core/host.c  |    2 ++
 drivers/mmc/core/mmc.c   |    6 ++++++
 include/linux/mmc/host.h |    1 +
 3 files changed, 9 insertions(+)

Changes to Fabio's patch:
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
 	if (of_property_read_bool(np, "wakeup-source") ||
 	    of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
 		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+	if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+		host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
 	if (of_property_read_bool(np, "mmc-ddr-1_8v"))
 		host->caps |= MMC_CAP_1_8V_DDR;
 	if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
 		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
 	}
 
+	if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
+	}
+
 	if (caps & MMC_CAP_1_8V_DDR &&
 	    card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
 		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
 #define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
 #define MMC_CAP2_NO_SD		(1 << 21)	/* Do not send SD commands during initialization */
 #define MMC_CAP2_NO_MMC		(1 << 22)	/* Do not send (e)MMC commands during initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR	(1 << 23)	/* Only supports 3.3V DDR */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
-- 
1.7.9.5

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Marek Vasut]

On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> This patch based on the work of Fabio Estevam:
> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
> 'no-1-8-v' is present"
> 
> It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/mmc/core/host.c  |    2 ++
>  drivers/mmc/core/mmc.c   |    6 ++++++
>  include/linux/mmc/host.h |    1 +
>  3 files changed, 9 insertions(+)
> 
> Changes to Fabio's patch:
> - rebase to current linux-next
> - rename DT property to mmc-ddr-3_3v
> - use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 98f25ff..4c971de 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
>  	if (of_property_read_bool(np, "wakeup-source") ||
>  	    of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
>  		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> +	if (of_property_read_bool(np, "mmc-ddr-3_3v"))
> +		host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
>  	if (of_property_read_bool(np, "mmc-ddr-1_8v"))
>  		host->caps |= MMC_CAP_1_8V_DDR;
>  	if (of_property_read_bool(np, "mmc-ddr-1_2v"))
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f2d185c..8a933d5 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
>  		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
>  	}
>  
> +	if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
> +	}
> +
>  	if (caps & MMC_CAP_1_8V_DDR &&
>  	    card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
>  		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index aa4bfbf..db0775d 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -311,6 +311,7 @@ struct mmc_host {
>  #define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
>  #define MMC_CAP2_NO_SD		(1 << 21)	/* Do not send SD commands during initialization */
>  #define MMC_CAP2_NO_MMC		(1 << 22)	/* Do not send (e)MMC commands during initialization */
> +#define MMC_CAP2_3_3V_ONLY_DDR	(1 << 23)	/* Only supports 3.3V DDR */
>  
>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>  
> 


-- 
Best regards,
Marek Vasut

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Stefan Wahren]

Hi Marek,

> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
> 
> 
> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> > This patch based on the work of Fabio Estevam:
> > "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
> > 'no-1-8-v' is present"
> > 
> > It adds the support for 3.3V only DDR MMC hosts.
> 
> Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].

Please don't blame me if it's not compatible to i.MX28. It's only an example.

[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

> 
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> >  drivers/mmc/core/host.c  |    2 ++
> >  drivers/mmc/core/mmc.c   |    6 ++++++
> >  include/linux/mmc/host.h |    1 +
> >  3 files changed, 9 insertions(+)
> > 
> > Changes to Fabio's patch:
> > - rebase to current linux-next
> > - rename DT property to mmc-ddr-3_3v
> > - use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
> > 
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > index 98f25ff..4c971de 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
> >  	if (of_property_read_bool(np, "wakeup-source") ||
> >  	    of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
> >  		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> > +	if (of_property_read_bool(np, "mmc-ddr-3_3v"))
> > +		host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
> >  	if (of_property_read_bool(np, "mmc-ddr-1_8v"))
> >  		host->caps |= MMC_CAP_1_8V_DDR;
> >  	if (of_property_read_bool(np, "mmc-ddr-1_2v"))
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index f2d185c..8a933d5 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
> >  		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
> >  	}
> >  
> > +	if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
> > +	}
> > +
> >  	if (caps & MMC_CAP_1_8V_DDR &&
> >  	    card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
> >  		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index aa4bfbf..db0775d 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -311,6 +311,7 @@ struct mmc_host {
> >  #define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
> >  #define MMC_CAP2_NO_SD		(1 << 21)	/* Do not send SD commands during
> > initialization */
> >  #define MMC_CAP2_NO_MMC		(1 << 22)	/* Do not send (e)MMC commands during
> > initialization */
> > +#define MMC_CAP2_3_3V_ONLY_DDR	(1 << 23)	/* Only supports 3.3V DDR */
> >  
> >  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
> >  
> > 
> 
> 
> -- 
> Best regards,
> Marek Vasut

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Marek Vasut]

On 08/06/2016 04:18 PM, Stefan Wahren wrote:
> Hi Marek,
> 
>> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
>>
>>
>> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
>>> This patch based on the work of Fabio Estevam:
>>> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
>>> 'no-1-8-v' is present"
>>>
>>> It adds the support for 3.3V only DDR MMC hosts.
>>
>> Do such cards even exist ? Do you have a link where I can find some ?
> 
> i never said anything about SD cards. I mean eMMC modules which usually have 8
> data pins like this one [1].
> 
> Please don't blame me if it's not compatible to i.MX28. It's only an example.
> 
> [1] -
> http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

Interesting, thanks!

-- 
Best regards,
Marek Vasut

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Shawn Lin]

? 2016/8/6 22:18, Stefan Wahren ??:
> Hi Marek,
>
>> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
>>
>>
>> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
>>> This patch based on the work of Fabio Estevam:
>>> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
>>> 'no-1-8-v' is present"
>>>
>>> It adds the support for 3.3V only DDR MMC hosts.
>>
>> Do such cards even exist ? Do you have a link where I can find some ?
>
> i never said anything about SD cards. I mean eMMC modules which usually have 8
> data pins like this one [1].
>
> Please don't blame me if it's not compatible to i.MX28. It's only an example.
>
> [1] -
> http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf


I download the datasheet you mentioned, and I explicitly see it
describe 1V8 everywhere, especially for the section of "ELECTRICAL
CHARACTERISTICS".

I never see one eMMC claiming DDR52 capability but doesn't support 1V8
so far. Could you kindly share me the part number of the eMMC you are
using? :)

>
>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>>  drivers/mmc/core/host.c  |    2 ++
>>>  drivers/mmc/core/mmc.c   |    6 ++++++
>>>  include/linux/mmc/host.h |    1 +
>>>  3 files changed, 9 insertions(+)
>>>
>>> Changes to Fabio's patch:
>>> - rebase to current linux-next
>>> - rename DT property to mmc-ddr-3_3v
>>> - use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
>>>
>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>> index 98f25ff..4c971de 100644
>>> --- a/drivers/mmc/core/host.c
>>> +++ b/drivers/mmc/core/host.c
>>> @@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
>>>  	if (of_property_read_bool(np, "wakeup-source") ||
>>>  	    of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
>>>  		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
>>> +	if (of_property_read_bool(np, "mmc-ddr-3_3v"))
>>> +		host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
>>>  	if (of_property_read_bool(np, "mmc-ddr-1_8v"))
>>>  		host->caps |= MMC_CAP_1_8V_DDR;
>>>  	if (of_property_read_bool(np, "mmc-ddr-1_2v"))
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f2d185c..8a933d5 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>  		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
>>>  	}
>>>
>>> +	if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
>>> +	}
>>> +
>>>  	if (caps & MMC_CAP_1_8V_DDR &&
>>>  	    card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
>>>  		hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index aa4bfbf..db0775d 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -311,6 +311,7 @@ struct mmc_host {
>>>  #define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
>>>  #define MMC_CAP2_NO_SD		(1 << 21)	/* Do not send SD commands during
>>> initialization */
>>>  #define MMC_CAP2_NO_MMC		(1 << 22)	/* Do not send (e)MMC commands during
>>> initialization */
>>> +#define MMC_CAP2_3_3V_ONLY_DDR	(1 << 23)	/* Only supports 3.3V DDR */
>>>
>>>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>>>
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Stefan Wahren]

Hi Shawn,

> Shawn Lin <shawn.lin@rock-chips.com> hat am 7. August 2016 um 04:07
> geschrieben:
> 
> 
> ? 2016/8/6 22:18, Stefan Wahren ??:
> > Hi Marek,
> >
> >> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
> >>
> >>
> >> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> >>> This patch based on the work of Fabio Estevam:
> >>> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
> >>> 'no-1-8-v' is present"
> >>>
> >>> It adds the support for 3.3V only DDR MMC hosts.
> >>
> >> Do such cards even exist ? Do you have a link where I can find some ?
> >
> > i never said anything about SD cards. I mean eMMC modules which usually have
> > 8
> > data pins like this one [1].
> >
> > Please don't blame me if it's not compatible to i.MX28. It's only an
> > example.
> >
> > [1] -
> > http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf
> 
> 
> I download the datasheet you mentioned, and I explicitly see it
> describe 1V8 everywhere, especially for the section of "ELECTRICAL
> CHARACTERISTICS".
> 
> I never see one eMMC claiming DDR52 capability but doesn't support 1V8
> so far. Could you kindly share me the part number of the eMMC you are
> using? :)

sorry the subject line is misleading. The capability flag is about the host not
about the MMC itself. There are hosts which doesn't support 1.8V. I will clarify
this in the next version.

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Jaehoon Chung]

On 08/07/2016 11:07 AM, Shawn Lin wrote:
> ? 2016/8/6 22:18, Stefan Wahren ??:
>> Hi Marek,
>>
>>> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
>>>
>>>
>>> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
>>>> This patch based on the work of Fabio Estevam:
>>>> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
>>>> 'no-1-8-v' is present"
>>>>
>>>> It adds the support for 3.3V only DDR MMC hosts.
>>>
>>> Do such cards even exist ? Do you have a link where I can find some ?
>>
>> i never said anything about SD cards. I mean eMMC modules which usually have 8
>> data pins like this one [1].
>>
>> Please don't blame me if it's not compatible to i.MX28. It's only an example.
>>
>> [1] -
>> http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf
> 
> 
> I download the datasheet you mentioned, and I explicitly see it
> describe 1V8 everywhere, especially for the section of "ELECTRICAL
> CHARACTERISTICS".
> 
> I never see one eMMC claiming DDR52 capability but doesn't support 1V8
> so far. Could you kindly share me the part number of the eMMC you are
> using? :)
> 
>>
>>>
>>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>> ---
>>>>  drivers/mmc/core/host.c  |    2 ++
>>>>  drivers/mmc/core/mmc.c   |    6 ++++++
>>>>  include/linux/mmc/host.h |    1 +
>>>>  3 files changed, 9 insertions(+)
>>>>
>>>> Changes to Fabio's patch:
>>>> - rebase to current linux-next
>>>> - rename DT property to mmc-ddr-3_3v
>>>> - use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
>>>>
>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>> index 98f25ff..4c971de 100644
>>>> --- a/drivers/mmc/core/host.c
>>>> +++ b/drivers/mmc/core/host.c
>>>> @@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
>>>>      if (of_property_read_bool(np, "wakeup-source") ||
>>>>          of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
>>>>          host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
>>>> +    if (of_property_read_bool(np, "mmc-ddr-3_3v"))
>>>> +        host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
>>>>      if (of_property_read_bool(np, "mmc-ddr-1_8v"))
>>>>          host->caps |= MMC_CAP_1_8V_DDR;
>>>>      if (of_property_read_bool(np, "mmc-ddr-1_2v"))
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index f2d185c..8a933d5 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>>          avail_type |= EXT_CSD_CARD_TYPE_HS_52;
>>>>      }
>>>>
>>>> +    if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
>>>> +    }

did you know EXT_CSD_CARD_TYPE_DDR_52 -> TYPE_DDR_1_8V and TYPE_DDR_1_2V?
It means card can use to the 1.8v and 1.2v ddr mode.
So this patch is strange..You added capability for only supporting 3_3v.

Confusing...

Best Regards,
Jaehoon Chung

>>>> +
>>>>      if (caps & MMC_CAP_1_8V_DDR &&
>>>>          card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
>>>>          hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>> index aa4bfbf..db0775d 100644
>>>> --- a/include/linux/mmc/host.h
>>>> +++ b/include/linux/mmc/host.h
>>>> @@ -311,6 +311,7 @@ struct mmc_host {
>>>>  #define MMC_CAP2_HS400_ES    (1 << 20)    /* Host supports enhanced strobe */
>>>>  #define MMC_CAP2_NO_SD        (1 << 21)    /* Do not send SD commands during
>>>> initialization */
>>>>  #define MMC_CAP2_NO_MMC        (1 << 22)    /* Do not send (e)MMC commands during
>>>> initialization */
>>>> +#define MMC_CAP2_3_3V_ONLY_DDR    (1 << 23)    /* Only supports 3.3V DDR */
>>>>
>>>>      mmc_pm_flag_t        pm_caps;    /* supported pm features */
>>>>
>>>>
>>>
>>>
>>> -- 
>>> Best regards,
>>> Marek Vasut
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 

[PATCH RFC 2/3] mmc: core: add new cap for 3.3V only DDR MMCs

[Stefan Wahren]

Am 11.08.2016 um 13:12 schrieb Jaehoon Chung:
> On 08/07/2016 11:07 AM, Shawn Lin wrote:
>> ? 2016/8/6 22:18, Stefan Wahren ??:
>>> Hi Marek,
>>>
>>>> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:14 geschrieben:
>>>>
>>>>
>>>> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
>>>>> This patch based on the work of Fabio Estevam:
>>>>> "[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
>>>>> 'no-1-8-v' is present"
>>>>>
>>>>> It adds the support for 3.3V only DDR MMC hosts.
>>>>
>>>> Do such cards even exist ? Do you have a link where I can find some ?
>>>
>>> i never said anything about SD cards. I mean eMMC modules which usually have 8
>>> data pins like this one [1].
>>>
>>> Please don't blame me if it's not compatible to i.MX28. It's only an example.
>>>
>>> [1] -
>>> http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf
>>
>>
>> I download the datasheet you mentioned, and I explicitly see it
>> describe 1V8 everywhere, especially for the section of "ELECTRICAL
>> CHARACTERISTICS".
>>
>> I never see one eMMC claiming DDR52 capability but doesn't support 1V8
>> so far. Could you kindly share me the part number of the eMMC you are
>> using? :)
>>
>>>
>>>>
>>>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>> ---
>>>>>  drivers/mmc/core/host.c  |    2 ++
>>>>>  drivers/mmc/core/mmc.c   |    6 ++++++
>>>>>  include/linux/mmc/host.h |    1 +
>>>>>  3 files changed, 9 insertions(+)
>>>>>
>>>>> Changes to Fabio's patch:
>>>>> - rebase to current linux-next
>>>>> - rename DT property to mmc-ddr-3_3v
>>>>> - use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
>>>>>
>>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>>> index 98f25ff..4c971de 100644
>>>>> --- a/drivers/mmc/core/host.c
>>>>> +++ b/drivers/mmc/core/host.c
>>>>> @@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
>>>>>      if (of_property_read_bool(np, "wakeup-source") ||
>>>>>          of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
>>>>>          host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
>>>>> +    if (of_property_read_bool(np, "mmc-ddr-3_3v"))
>>>>> +        host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
>>>>>      if (of_property_read_bool(np, "mmc-ddr-1_8v"))
>>>>>          host->caps |= MMC_CAP_1_8V_DDR;
>>>>>      if (of_property_read_bool(np, "mmc-ddr-1_2v"))
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index f2d185c..8a933d5 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>>>          avail_type |= EXT_CSD_CARD_TYPE_HS_52;
>>>>>      }
>>>>>
>>>>> +    if (caps2 & MMC_CAP2_3_3V_ONLY_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_52;
>>>>> +    }
>
> did you know EXT_CSD_CARD_TYPE_DDR_52 -> TYPE_DDR_1_8V and TYPE_DDR_1_2V?

Thanks for pointing out. Fabio's patch was better here.

As i wrote to Shawn the subject line is incorrect. I want to add 3.3V 
only support for hosts not for "card".

According to the comments EXT_CSD_CARD_TYPE_DDR_1_8V stands for 1.8V or 
3.3V. So i better add EXT_CSD_CARD_TYPE_DDR_3_3V_ONLY even the "card" 
supports 1.8V.

[PATCH RFC 3/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

This patch implements driver support for 3.3V DDR eMMCs.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/mmc/host/mxs-mmc.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..84019d5 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -71,6 +71,7 @@ struct mxs_mmc_host {
 	spinlock_t			lock;
 	int				sdio_irq_en;
 	bool				broken_cd;
+	bool				is_ddr;
 };
 
 static int mxs_mmc_get_cd(struct mmc_host *mmc)
@@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 		cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
 			BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
 	} else {
+		if (host->is_ddr)
+			cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
+
 		writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
 		writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
 		       BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
@@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 
 	if (ios->bus_width == MMC_BUS_WIDTH_8)
 		host->bus_width = 2;
@@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	if (ios->clock)
 		mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
+
+	if (ssp_is_old(ssp))
+		return;
+
+	if (ios->timing == MMC_TIMING_MMC_DDR52) {
+		/*
+		 * ENGR00133481-1: In DDR mode the host send the data at
+		 * negative edge and the MMC receive the data at positive edge.
+		 */
+		host->is_ddr = true;
+		writel(BM_SSP_CTRL1_POLARITY, ssp->base +
+			HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
+	} else {
+		host->is_ddr = false;
+		writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
+		       STMP_OFFSET_REG_SET);
+	}
 }
 
 static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
-- 
1.7.9.5

[PATCH RFC 3/3] mmc: mxs-mmc: Implement DDR support

[Marek Vasut]

On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> This patch implements driver support for 3.3V DDR eMMCs.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/mmc/host/mxs-mmc.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> index d839147..84019d5 100644
> --- a/drivers/mmc/host/mxs-mmc.c
> +++ b/drivers/mmc/host/mxs-mmc.c
> @@ -71,6 +71,7 @@ struct mxs_mmc_host {
>  	spinlock_t			lock;
>  	int				sdio_irq_en;
>  	bool				broken_cd;
> +	bool				is_ddr;
>  };
>  
>  static int mxs_mmc_get_cd(struct mmc_host *mmc)
> @@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
>  		cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
>  			BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
>  	} else {
> +		if (host->is_ddr)
> +			cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
> +
>  		writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
>  		writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
>  		       BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
> @@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>  	struct mxs_mmc_host *host = mmc_priv(mmc);
> +	struct mxs_ssp *ssp = &host->ssp;
>  
>  	if (ios->bus_width == MMC_BUS_WIDTH_8)
>  		host->bus_width = 2;
> @@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  
>  	if (ios->clock)
>  		mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
> +
> +	if (ssp_is_old(ssp))
> +		return;
> +
> +	if (ios->timing == MMC_TIMING_MMC_DDR52) {

Shouldn't you validate that the clock are set to at least 52MHz before
neabling the DDR mode ?

> +		/*
> +		 * ENGR00133481-1: In DDR mode the host send the data at
> +		 * negative edge and the MMC receive the data at positive edge.
> +		 */
> +		host->is_ddr = true;
> +		writel(BM_SSP_CTRL1_POLARITY, ssp->base +
> +			HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
> +	} else {

Is it by any chance possible that this else branch will catch not only
the non-DDR options, but also some DDR ones , thus causing problems ?

> +		host->is_ddr = false;
> +		writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
> +		       STMP_OFFSET_REG_SET);
> +	}
>  }
>  
>  static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> 


-- 
Best regards,
Marek Vasut

[PATCH RFC 3/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Hi Marek,

> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:13 geschrieben:
> 
> 
> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> > This patch implements driver support for 3.3V DDR eMMCs.
> > 
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> >  drivers/mmc/host/mxs-mmc.c |   22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> > index d839147..84019d5 100644
> > --- a/drivers/mmc/host/mxs-mmc.c
> > +++ b/drivers/mmc/host/mxs-mmc.c
> > @@ -71,6 +71,7 @@ struct mxs_mmc_host {
> >  	spinlock_t			lock;
> >  	int				sdio_irq_en;
> >  	bool				broken_cd;
> > +	bool				is_ddr;
> >  };
> >  
> >  static int mxs_mmc_get_cd(struct mmc_host *mmc)
> > @@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
> >  		cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
> >  			BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
> >  	} else {
> > +		if (host->is_ddr)
> > +			cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
> > +
> >  		writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
> >  		writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
> >  		       BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
> > @@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct
> > mmc_request *mrq)
> >  static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >  {
> >  	struct mxs_mmc_host *host = mmc_priv(mmc);
> > +	struct mxs_ssp *ssp = &host->ssp;
> >  
> >  	if (ios->bus_width == MMC_BUS_WIDTH_8)
> >  		host->bus_width = 2;
> > @@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc,
> > struct mmc_ios *ios)
> >  
> >  	if (ios->clock)
> >  		mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
> > +
> > +	if (ssp_is_old(ssp))
> > +		return;
> > +
> > +	if (ios->timing == MMC_TIMING_MMC_DDR52) {
> 
> Shouldn't you validate that the clock are set to at least 52MHz before
> neabling the DDR mode ?

according to the i.MX28 datasheet IMX28CEC
3.5.14.2 MMC4.4 (Dual Data Rate) AC Timing
the minimum clock frequency is 0 and maximum is 52 MHz. So i assume the core
take care of the right clock rate.

> 
> > +		/*
> > +		 * ENGR00133481-1: In DDR mode the host send the data at
> > +		 * negative edge and the MMC receive the data at positive edge.
> > +		 */
> > +		host->is_ddr = true;
> > +		writel(BM_SSP_CTRL1_POLARITY, ssp->base +
> > +			HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
> > +	} else {
> 
> Is it by any chance possible that this else branch will catch not only
> the non-DDR options, but also some DDR ones , thus causing problems ?

No, because the polarity bit has been set all the time by mxs_mmc_reset().

But it's possible that i didn't catch all DDR cases.

> 
> > +		host->is_ddr = false;
> > +		writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
> > +		       STMP_OFFSET_REG_SET);
> > +	}
> >  }
> >  
> >  static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> > 
> 
> 
> -- 
> Best regards,
> Marek Vasut

[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Marek Vasut]

On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> Based on these discussions: [1], [2] this patch series implements
> DDR support for the MXS MMC host driver. This feature has never been
> ported from the vendor kernel.
> 
> It has been tested on a i.MX28 board with eMMC which is currently
> not in mainline (Duckbill 2).
> 
> * without DDR support
> dd if=/dev/zero of=test bs=8k count=10000
> 81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
> 
> * with DDR support:
> dd if=/dev/zero of=test bs=8k count=10000
> 81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

Not much of a speedup, how so ?

> [1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
> [2] - http://www.spinics.net/lists/linux-mmc/msg34418.html
> 
> Stefan Wahren (3):
>   DT: bindings: mmc: Add property for 3.3V only support
>   mmc: core: add new cap for 3.3V only DDR MMCs
>   mmc: mxs-mmc: Implement DDR support
> 
>  Documentation/devicetree/bindings/mmc/mmc.txt |    7 +++++--
>  drivers/mmc/core/host.c                       |    2 ++
>  drivers/mmc/core/mmc.c                        |    6 ++++++
>  drivers/mmc/host/mxs-mmc.c                    |   22 ++++++++++++++++++++++
>  include/linux/mmc/host.h                      |    1 +
>  5 files changed, 36 insertions(+), 2 deletions(-)
> 


-- 
Best regards,
Marek Vasut

[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Hi Marek,

> Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:10 geschrieben:
> 
> 
> On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> > Based on these discussions: [1], [2] this patch series implements
> > DDR support for the MXS MMC host driver. This feature has never been
> > ported from the vendor kernel.
> > 
> > It has been tested on a i.MX28 board with eMMC which is currently
> > not in mainline (Duckbill 2).
> > 
> > * without DDR support
> > dd if=/dev/zero of=test bs=8k count=10000
> > 81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
> > 
> > * with DDR support:
> > dd if=/dev/zero of=test bs=8k count=10000
> > 81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s
> 
> Not much of a speedup, how so ?

i agree. Unfortunately i never had the time for a deeper analysis, but i noticed
big write performance differences between the vendor kernel 2.6.35 and current
mainline kernel in case of writing 100 MB directly to an eMMC partition (factor
2). Implementation of DDR mode was the lower fruit.

I've the suspicion that is related to the usage of DMA engine. Maybe the
overhead for AC/BC commands is to big. In the Freescale kernel is an
optimization [1].

[1] -
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/?h=imx_2.6.35_maintain&id=b09358887fb4b67f6d497fac8cc48475c8bd292d

> 
> > [1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
> > [2] - http://www.spinics.net/lists/linux-mmc/msg34418.html
> > 
> > Stefan Wahren (3):
> >   DT: bindings: mmc: Add property for 3.3V only support
> >   mmc: core: add new cap for 3.3V only DDR MMCs
> >   mmc: mxs-mmc: Implement DDR support
> > 
> >  Documentation/devicetree/bindings/mmc/mmc.txt |    7 +++++--
> >  drivers/mmc/core/host.c                       |    2 ++
> >  drivers/mmc/core/mmc.c                        |    6 ++++++
> >  drivers/mmc/host/mxs-mmc.c                    |   22 ++++++++++++++++++++++
> >  include/linux/mmc/host.h                      |    1 +
> >  5 files changed, 36 insertions(+), 2 deletions(-)
> > 
> 
> 
> -- 
> Best regards,
> Marek Vasut

[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Hi Marek,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 6. August 2016 um 15:48
> geschrieben:
> 
> 
> Hi Marek,
> 
> > Marek Vasut <marex@denx.de> hat am 6. August 2016 um 15:10 geschrieben:
> > 
> > 
> > On 08/06/2016 02:55 PM, Stefan Wahren wrote:
> > > Based on these discussions: [1], [2] this patch series implements
> > > DDR support for the MXS MMC host driver. This feature has never been
> > > ported from the vendor kernel.
> > > 
> > > It has been tested on a i.MX28 board with eMMC which is currently
> > > not in mainline (Duckbill 2).
> > > 
> > > * without DDR support
> > > dd if=/dev/zero of=test bs=8k count=10000
> > > 81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
> > > 
> > > * with DDR support:
> > > dd if=/dev/zero of=test bs=8k count=10000
> > > 81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s
> > 
> > Not much of a speedup, how so ?
> 
> i agree. Unfortunately i never had the time for a deeper analysis, but i
> noticed
> big write performance differences between the vendor kernel 2.6.35 and current
> mainline kernel in case of writing 100 MB directly to an eMMC partition
> (factor
> 2). Implementation of DDR mode was the lower fruit.
> 
> I've the suspicion that is related to the usage of DMA engine. Maybe the
> overhead for AC/BC commands is to big. In the Freescale kernel is an
> optimization [1].
> 
> [1] -
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/?h=imx_2.6.35_maintain&id=b09358887fb4b67f6d497fac8cc48475c8bd292d
> 

here are some additional thoughts about the too little performance gain of DDR
mode:

* The DDR mode applies only to the DATA lines not to the CMD line. So the plain
MMC commands can't benefit from that.

* The i.MX28 reference manual mentions a relevant setting:

  17.8.2.2 eMMC DDR operation
  To boost the performance of SSP running in DDR mode and at the maximum
frequency, 
  the DMA interface allows a burst of 4 or 8 32-bit APB transfers per DMA
request.

I tried to change HW_SSP_DDR_CTRL_DMA_BURST_TYPE with no luck (in case of 8
bursts the communication to the MMC get lost). AFAIK this register is never
touched in the vendor kernel.

Regards
Stefan

[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Hi Ulf,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 6. August 2016 um 14:55
> geschrieben:
> 
> 
> Based on these discussions: [1], [2] this patch series implements
> DDR support for the MXS MMC host driver. This feature has never been
> ported from the vendor kernel.
> 
> It has been tested on a i.MX28 board with eMMC which is currently
> not in mainline (Duckbill 2).
> 
> * without DDR support
> dd if=/dev/zero of=test bs=8k count=10000
> 81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
> 
> * with DDR support:
> dd if=/dev/zero of=test bs=8k count=10000
> 81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s
> 
> [1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
> [2] - http://www.spinics.net/lists/linux-mmc/msg34418.html

do you want to add some comments about this series?

Regards
Stefan

> 
> Stefan Wahren (3):
>   DT: bindings: mmc: Add property for 3.3V only support
>   mmc: core: add new cap for 3.3V only DDR MMCs
>   mmc: mxs-mmc: Implement DDR support
> 
>  Documentation/devicetree/bindings/mmc/mmc.txt |    7 +++++--
>  drivers/mmc/core/host.c                       |    2 ++
>  drivers/mmc/core/mmc.c                        |    6 ++++++
>  drivers/mmc/host/mxs-mmc.c                    |   22 ++++++++++++++++++++++
>  include/linux/mmc/host.h                      |    1 +
>  5 files changed, 36 insertions(+), 2 deletions(-)
> 
> -- 
> 1.7.9.5