diff mbox

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

Message ID 1470488140-10104-2-git-send-email-stefan.wahren@i2se.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Wahren Aug. 6, 2016, 12:55 p.m. UTC
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(-)

Comments

Rob Herring (Arm) Aug. 10, 2016, 6:44 p.m. UTC | #1
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
--
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
Stefan Wahren Aug. 10, 2016, 8:27 p.m. UTC | #2
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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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
Rob Herring (Arm) Aug. 10, 2016, 9:39 p.m. UTC | #3
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Lin Aug. 11, 2016, 12:48 a.m. UTC | #4
+ 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Adrian Hunter Aug. 18, 2016, 12:25 p.m. UTC | #5
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@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 Aug. 30, 2016, 9:26 a.m. UTC | #6
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
--
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
Stefan Wahren Sept. 2, 2016, 6:50 p.m. UTC | #7
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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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
Rob Herring (Arm) Sept. 8, 2016, 4:44 p.m. UTC | #8
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
--
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/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