diff mbox

[v2,02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties

Message ID 1358955158-1510-3-git-send-email-g.liakhovetski@gmx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Guennadi Liakhovetski Jan. 23, 2013, 3:32 p.m. UTC
GPIO DT bindings have a standard way to specify GPIO polarity - the
OF_GPIO_ACTIVE_LOW flag. Use that instead of custom cd-inverted and
wp-inverted properties.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Chris Ball Jan. 28, 2013, 10:23 p.m. UTC | #1
Hi Guennadi,

On Wed, Jan 23 2013, Guennadi Liakhovetski wrote:
> +cd-inverted and wp-inverted properties are deprecated ans shouldn't be used,
> +instead pleaseuse the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings. Note,
> +that the default (as defined by the SDHCI standard) CD and WP polarity is
> +active-low, so, OF_GPIO_ACTIVE_LOW should normally be set, and only be left
> +clear, if the polarity is inverted.

Please use this text for your next version, fixing typos and a newline:

cd-inverted and wp-inverted properties are deprecated and shouldn't be used,
instead please use the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings.
Note that the default (as defined by the SDHCI standard) CD and WP polarity
is active-low, so OF_GPIO_ACTIVE_LOW should normally be set, and only be
left clear if the polarity is inverted.

Thanks,

- Chris.
Arnd Bergmann Jan. 30, 2013, 3:47 p.m. UTC | #2
On Monday 28 January 2013, Chris Ball wrote:
> On Wed, Jan 23 2013, Guennadi Liakhovetski wrote:
> > +cd-inverted and wp-inverted properties are deprecated ans shouldn't be used,
> > +instead pleaseuse the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings. Note,
> > +that the default (as defined by the SDHCI standard) CD and WP polarity is
> > +active-low, so, OF_GPIO_ACTIVE_LOW should normally be set, and only be left
> > +clear, if the polarity is inverted.
> 
> Please use this text for your next version, fixing typos and a newline:
> 
> cd-inverted and wp-inverted properties are deprecated and shouldn't be used,
> instead please use the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings.
> Note that the default (as defined by the SDHCI standard) CD and WP polarity
> is active-low, so OF_GPIO_ACTIVE_LOW should normally be set, and only be
> left clear if the polarity is inverted.

Hmm, I wonder if this is possible in general. A lot of the GPIO bindings
allow passing flags, but I think that some of them do not, for historic
reasons. If we want to deprecate the behavior in eMMC, we should also
ensure that all gpio drivers are extended to support gpio specifiers
with flags. It should be possible to extend all drivers in a compatible
way, but someone has to do that.

When we introduced the MMC binding, the situation was already like this,
and it seemed easier to leave the {wp,cd}-inverted properties as optional.

	Arnd
--
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
Guennadi Liakhovetski Jan. 30, 2013, 4:02 p.m. UTC | #3
Hi Arnd

Thanks for your input.

On Wed, 30 Jan 2013, Arnd Bergmann wrote:

> On Monday 28 January 2013, Chris Ball wrote:
> > On Wed, Jan 23 2013, Guennadi Liakhovetski wrote:
> > > +cd-inverted and wp-inverted properties are deprecated ans shouldn't be used,
> > > +instead pleaseuse the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings. Note,
> > > +that the default (as defined by the SDHCI standard) CD and WP polarity is
> > > +active-low, so, OF_GPIO_ACTIVE_LOW should normally be set, and only be left
> > > +clear, if the polarity is inverted.
> > 
> > Please use this text for your next version, fixing typos and a newline:
> > 
> > cd-inverted and wp-inverted properties are deprecated and shouldn't be used,
> > instead please use the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings.
> > Note that the default (as defined by the SDHCI standard) CD and WP polarity
> > is active-low, so OF_GPIO_ACTIVE_LOW should normally be set, and only be
> > left clear if the polarity is inverted.
> 
> Hmm, I wonder if this is possible in general. A lot of the GPIO bindings
> allow passing flags, but I think that some of them do not, for historic
> reasons. If we want to deprecate the behavior in eMMC, we should also
> ensure that all gpio drivers are extended to support gpio specifiers
> with flags. It should be possible to extend all drivers in a compatible
> way, but someone has to do that.
> 
> When we introduced the MMC binding, the situation was already like this,
> and it seemed easier to leave the {wp,cd}-inverted properties as optional.

This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
gpio "flags" cell and has to fall-back to always use "*-inverted" 
properties. Same holds for any other multi-arch driver, using GPIOs. So, 
we're stuck with this?

But in fact, we're not dropping support for those flags. We're just 
encouraging any new mmc drivers to use GPIO flags. Maybe we should just 
change the wording from "deprecated" to "discouraged" and replace the 
warning with a debug message?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Guennadi Liakhovetski Jan. 30, 2013, 4:13 p.m. UTC | #4
On Wed, 30 Jan 2013, Guennadi Liakhovetski wrote:

> Hi Arnd
> 
> Thanks for your input.
> 
> On Wed, 30 Jan 2013, Arnd Bergmann wrote:
> 
> > On Monday 28 January 2013, Chris Ball wrote:
> > > On Wed, Jan 23 2013, Guennadi Liakhovetski wrote:
> > > > +cd-inverted and wp-inverted properties are deprecated ans shouldn't be used,
> > > > +instead pleaseuse the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings. Note,
> > > > +that the default (as defined by the SDHCI standard) CD and WP polarity is
> > > > +active-low, so, OF_GPIO_ACTIVE_LOW should normally be set, and only be left
> > > > +clear, if the polarity is inverted.
> > > 
> > > Please use this text for your next version, fixing typos and a newline:
> > > 
> > > cd-inverted and wp-inverted properties are deprecated and shouldn't be used,
> > > instead please use the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings.
> > > Note that the default (as defined by the SDHCI standard) CD and WP polarity
> > > is active-low, so OF_GPIO_ACTIVE_LOW should normally be set, and only be
> > > left clear if the polarity is inverted.
> > 
> > Hmm, I wonder if this is possible in general. A lot of the GPIO bindings
> > allow passing flags, but I think that some of them do not, for historic
> > reasons. If we want to deprecate the behavior in eMMC, we should also
> > ensure that all gpio drivers are extended to support gpio specifiers
> > with flags. It should be possible to extend all drivers in a compatible
> > way, but someone has to do that.
> > 
> > When we introduced the MMC binding, the situation was already like this,
> > and it seemed easier to leave the {wp,cd}-inverted properties as optional.
> 
> This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
> gpio "flags" cell and has to fall-back to always use "*-inverted" 
> properties. Same holds for any other multi-arch driver, using GPIOs. So, 
> we're stuck with this?

BTW, just verified in the current "next": all platforms, using cd-inverted 
or wp-inverted in the mainline

arch/arm/boot/dts/ccu9540.dts
arch/arm/boot/dts/ea3250.dts
arch/arm/boot/dts/phy3250.dts
arch/arm/boot/dts/snowball.dts
arch/arm/boot/dts/u9540.dts

use GPIO controllers with 2 or 3 cells.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Arnd Bergmann Jan. 30, 2013, 4:29 p.m. UTC | #5
On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
> > gpio "flags" cell and has to fall-back to always use "*-inverted" 
> > properties. Same holds for any other multi-arch driver, using GPIOs. So, 
> > we're stuck with this?

But the SDHCI driver itself would not interpret the flags anyway, would it?
From the code, I understand that of_get_named_gpio() would return a gpio
line with the polarity already inverted if it's specified that way, and
the SDHCI_QUIRK_INVERTED_WRITE_PROTECT flag can be used to invert it
independent of it. So you could actually express the same thing by either
putting the inversion into the gpio specifier or into the *-inverted
properties.

If you actually provide both, that would have the same meaning as not
inverting.

> BTW, just verified in the current "next": all platforms, using cd-inverted 
> or wp-inverted in the mainline
> 
> arch/arm/boot/dts/ccu9540.dts
> arch/arm/boot/dts/ea3250.dts
> arch/arm/boot/dts/phy3250.dts
> arch/arm/boot/dts/snowball.dts
> arch/arm/boot/dts/u9540.dts
> 
> use GPIO controllers with 2 or 3 cells.

What about those that don't use a GPIO line at all but instead use a
built-in write-protect and card-detect registers of the SDHCI controller?

The Freescale ESDHC on powerpc mpc83xx seems to do that, and require 
the wp-inverted flag.

	Arnd
--
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
Guennadi Liakhovetski Jan. 30, 2013, 5:03 p.m. UTC | #6
On Wed, 30 Jan 2013, Arnd Bergmann wrote:

> On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > > This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
> > > gpio "flags" cell and has to fall-back to always use "*-inverted" 
> > > properties. Same holds for any other multi-arch driver, using GPIOs. So, 
> > > we're stuck with this?
> 
> But the SDHCI driver itself would not interpret the flags anyway, would it?

Currently each driver, including SDHCI, does it on its own, the goal is to 
do this centrally. And yes, in our specific MMC case, if we want to 
support GPIO controllers with 1 cell, no mmc DT node, using GPIOs for WP 
or CD should be using the inversion flag - even where possible.

> From the code, I understand that of_get_named_gpio() would return a gpio
> line with the polarity already inverted if it's specified that way,

Sorry, don't understand. of_get_named_gpio() just returns a GPIO number, 
not GPIO level. It doesn't even actually request the GPIO for the specific 
user. Or do you mean, that gpio_chip::of_xlate() should request the chip 
to invert the GPIO from now on?

> and
> the SDHCI_QUIRK_INVERTED_WRITE_PROTECT flag can be used to invert it
> independent of it.

Right, not sure about that flag. Isn't it used for SDHCI-native WP?

> So you could actually express the same thing by either
> putting the inversion into the gpio specifier or into the *-inverted
> properties.
> 
> If you actually provide both, that would have the same meaning as not
> inverting.

Hm, that'd be a weird way to configure the pin, perhaps.

> > BTW, just verified in the current "next": all platforms, using cd-inverted 
> > or wp-inverted in the mainline
> > 
> > arch/arm/boot/dts/ccu9540.dts
> > arch/arm/boot/dts/ea3250.dts
> > arch/arm/boot/dts/phy3250.dts
> > arch/arm/boot/dts/snowball.dts
> > arch/arm/boot/dts/u9540.dts
> > 
> > use GPIO controllers with 2 or 3 cells.
> 
> What about those that don't use a GPIO line at all but instead use a
> built-in write-protect and card-detect registers of the SDHCI controller?

The mmc.txt document specifies cd-inverted and wp-inverted _explicitly_ 
for use with GPIOs. If we want to use them with built-in CD and WP pins, 
we have to modify those definitions too.

Thanks
Guennadi

> The Freescale ESDHC on powerpc mpc83xx seems to do that, and require 
> the wp-inverted flag.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Arnd Bergmann Jan. 31, 2013, 12:09 a.m. UTC | #7
On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> On Wed, 30 Jan 2013, Arnd Bergmann wrote:
> 
> > On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > > > This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
> > > > gpio "flags" cell and has to fall-back to always use "*-inverted" 
> > > > properties. Same holds for any other multi-arch driver, using GPIOs. So, 
> > > > we're stuck with this?
> > 
> > But the SDHCI driver itself would not interpret the flags anyway, would it?
> 
> Currently each driver, including SDHCI, does it on its own, the goal is to 
> do this centrally. And yes, in our specific MMC case, if we want to 
> support GPIO controllers with 1 cell, no mmc DT node, using GPIOs for WP 
> or CD should be using the inversion flag - even where possible.
> 
> > From the code, I understand that of_get_named_gpio() would return a gpio
> > line with the polarity already inverted if it's specified that way,
> 
> Sorry, don't understand. of_get_named_gpio() just returns a GPIO number, 
> not GPIO level. It doesn't even actually request the GPIO for the specific 
> user. Or do you mean, that gpio_chip::of_xlate() should request the chip 
> to invert the GPIO from now on?

I'm not saying that it should, I just thought that it did that already,
but I may be wrong with that. Where does the polarity of a gpio
line normally get set?

> > and
> > the SDHCI_QUIRK_INVERTED_WRITE_PROTECT flag can be used to invert it
> > independent of it.
> 
> Right, not sure about that flag. Isn't it used for SDHCI-native WP?

It's used in the sdhci_check_ro() function, independent of whether
the bit comes from the GPIO or from the SDHCI_PRESENT_STATE
register.

It gets set based on the presence of the wp-inverted or the
older sdhci,wp-inverted property, which both get interpreted
in the same way.

> > So you could actually express the same thing by either
> > putting the inversion into the gpio specifier or into the *-inverted
> > properties.
> > 
> > If you actually provide both, that would have the same meaning as not
> > inverting.
> 
> Hm, that'd be a weird way to configure the pin, perhaps.

Yes, it's silly and I don't see a reason why you'd do that, but I think
it should still be considered valid.

> > > BTW, just verified in the current "next": all platforms, using cd-inverted 
> > > or wp-inverted in the mainline
> > > 
> > > arch/arm/boot/dts/ccu9540.dts
> > > arch/arm/boot/dts/ea3250.dts
> > > arch/arm/boot/dts/phy3250.dts
> > > arch/arm/boot/dts/snowball.dts
> > > arch/arm/boot/dts/u9540.dts
> > > 
> > > use GPIO controllers with 2 or 3 cells.
> > 
> > What about those that don't use a GPIO line at all but instead use a
> > built-in write-protect and card-detect registers of the SDHCI controller?
> 
> The mmc.txt document specifies cd-inverted and wp-inverted _explicitly_ 
> for use with GPIOs. If we want to use them with built-in CD and WP pins, 
> we have to modify those definitions too.

Yes, that makes sense. The way that the specific esdhc binding is written
does not refer to gpio and could be interpreted the way that it is used
in the driver (for both gpio and internal wp), but it's certainly a
good idea to clarify it in the common binding.

	Arnd
--
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
Chris Ball Jan. 31, 2013, 12:20 a.m. UTC | #8
Hi,

On Wed, Jan 30 2013, Arnd Bergmann wrote:
>> > From the code, I understand that of_get_named_gpio() would return a gpio
>> > line with the polarity already inverted if it's specified that way,
>> 
>> Sorry, don't understand. of_get_named_gpio() just returns a GPIO number, 
>> not GPIO level. It doesn't even actually request the GPIO for the specific 
>> user. Or do you mean, that gpio_chip::of_xlate() should request the chip 
>> to invert the GPIO from now on?
>
> I'm not saying that it should, I just thought that it did that already,
> but I may be wrong with that. Where does the polarity of a gpio
> line normally get set?

of_get_named_gpio() intentionally discards the specified flags, but
of_get_named_gpio_flags(.., .., .., &gpio_flags) retrieves them, and
gpio_flags will == OF_GPIO_ACTIVE_LOW if that's how the gpio's been
specified in the DT.

- Chris.
Guennadi Liakhovetski Jan. 31, 2013, 6:47 a.m. UTC | #9
On Thu, 31 Jan 2013, Arnd Bergmann wrote:

> On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > On Wed, 30 Jan 2013, Arnd Bergmann wrote:
> > 
> > > On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > > > > This means, that a multi-platform driver like, e.g. SDHCI cannot use the 
> > > > > gpio "flags" cell and has to fall-back to always use "*-inverted" 
> > > > > properties. Same holds for any other multi-arch driver, using GPIOs. So, 
> > > > > we're stuck with this?
> > > 
> > > But the SDHCI driver itself would not interpret the flags anyway, would it?
> > 
> > Currently each driver, including SDHCI, does it on its own, the goal is to 
> > do this centrally. And yes, in our specific MMC case, if we want to 
> > support GPIO controllers with 1 cell, no mmc DT node, using GPIOs for WP 
> > or CD should be using the inversion flag - even where possible.
> > 
> > > From the code, I understand that of_get_named_gpio() would return a gpio
> > > line with the polarity already inverted if it's specified that way,
> > 
> > Sorry, don't understand. of_get_named_gpio() just returns a GPIO number, 
> > not GPIO level. It doesn't even actually request the GPIO for the specific 
> > user. Or do you mean, that gpio_chip::of_xlate() should request the chip 
> > to invert the GPIO from now on?
> 
> I'm not saying that it should, I just thought that it did that already,
> but I may be wrong with that. Where does the polarity of a gpio
> line normally get set?

Well, don't think we had a generic way to handle this until now. Each 
driver handles GPIOs in its own way. The GPIO API only provides functions 
to reserve GPIOs, specify their direction (out / in) and set or get level 
respectively. Besides you could set some flags like GPIOF_OPEN_DRAIN etc. 
After a driver, say, reads low level of a GPIO it has to decide itself 
whether it's the "on" or the "off" level, for which many drivers often use 
their own means to specify GPIO's "polarity."

> > > and
> > > the SDHCI_QUIRK_INVERTED_WRITE_PROTECT flag can be used to invert it
> > > independent of it.
> > 
> > Right, not sure about that flag. Isn't it used for SDHCI-native WP?
> 
> It's used in the sdhci_check_ro() function, independent of whether
> the bit comes from the GPIO or from the SDHCI_PRESENT_STATE
> register.

Right, but the MMC core already has a generic MMC_CAP2_RO_ACTIVE_HIGH 
flag, so, this flag is redundant. Perhaps, the SDHCI OF glue will have to 
set that flag if the MMC core reports MMC_CAP2_RO_ACTIVE_HIGH.

> It gets set based on the presence of the wp-inverted or the
> older sdhci,wp-inverted property, which both get interpreted
> in the same way.

Right, which is already a mess...

> > > So you could actually express the same thing by either
> > > putting the inversion into the gpio specifier or into the *-inverted
> > > properties.
> > > 
> > > If you actually provide both, that would have the same meaning as not
> > > inverting.
> > 
> > Hm, that'd be a weird way to configure the pin, perhaps.
> 
> Yes, it's silly and I don't see a reason why you'd do that, but I think
> it should still be considered valid.

We can do that, sure, but I'm not sure we really have to apply both to end 
up with a non-inverted pin, or just interpret it as resundant information 
and invert once.

> > > > BTW, just verified in the current "next": all platforms, using cd-inverted 
> > > > or wp-inverted in the mainline
> > > > 
> > > > arch/arm/boot/dts/ccu9540.dts
> > > > arch/arm/boot/dts/ea3250.dts
> > > > arch/arm/boot/dts/phy3250.dts
> > > > arch/arm/boot/dts/snowball.dts
> > > > arch/arm/boot/dts/u9540.dts
> > > > 
> > > > use GPIO controllers with 2 or 3 cells.
> > > 
> > > What about those that don't use a GPIO line at all but instead use a
> > > built-in write-protect and card-detect registers of the SDHCI controller?
> > 
> > The mmc.txt document specifies cd-inverted and wp-inverted _explicitly_ 
> > for use with GPIOs. If we want to use them with built-in CD and WP pins, 
> > we have to modify those definitions too.
> 
> Yes, that makes sense. The way that the specific esdhc binding is written
> does not refer to gpio and could be interpreted the way that it is used
> in the driver (for both gpio and internal wp), but it's certainly a
> good idea to clarify it in the common binding.

Ok, as soon as we agree on handling of the double inversion, I'll prepare 
and post an updated patch series.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Arnd Bergmann Jan. 31, 2013, 9 a.m. UTC | #10
On Thursday 31 January 2013, Guennadi Liakhovetski wrote:
> On Thu, 31 Jan 2013, Arnd Bergmann wrote:
> > On Wednesday 30 January 2013, Guennadi Liakhovetski wrote:
> > > On Wed, 30 Jan 2013, Arnd Bergmann wrote:
> > 
> > I'm not saying that it should, I just thought that it did that already,
> > but I may be wrong with that. Where does the polarity of a gpio
> > line normally get set?
> 
> Well, don't think we had a generic way to handle this until now. Each 
> driver handles GPIOs in its own way. The GPIO API only provides functions 
> to reserve GPIOs, specify their direction (out / in) and set or get level 
> respectively. Besides you could set some flags like GPIOF_OPEN_DRAIN etc. 
> After a driver, say, reads low level of a GPIO it has to decide itself 
> whether it's the "on" or the "off" level, for which many drivers often use 
> their own means to specify GPIO's "polarity."

Right, Chris also mentioned the of_get_named_gpio_flags() method that
drivers would need to use to get the flags, although currently
most of them don't.

> 
> > > > and
> > > > the SDHCI_QUIRK_INVERTED_WRITE_PROTECT flag can be used to invert it
> > > > independent of it.
> > > 
> > > Right, not sure about that flag. Isn't it used for SDHCI-native WP?
> > 
> > It's used in the sdhci_check_ro() function, independent of whether
> > the bit comes from the GPIO or from the SDHCI_PRESENT_STATE
> > register.
> 
> Right, but the MMC core already has a generic MMC_CAP2_RO_ACTIVE_HIGH 
> flag, so, this flag is redundant. Perhaps, the SDHCI OF glue will have to 
> set that flag if the MMC core reports MMC_CAP2_RO_ACTIVE_HIGH.

Makes sense. It seems that the MMC_CAP2_RO_ACTIVE_HIGH flag is only
evaluated by the generic mmc_gpio_get_ro() helper function you wrote
and that is currently used in only one host driver, while the
SDHCI_QUIRK_INVERTED_WRITE_PROTECT obviously is only used in SDHCI.
It certainly makes sense to unify those flags.

> > It gets set based on the presence of the wp-inverted or the
> > older sdhci,wp-inverted property, which both get interpreted
> > in the same way.
> 
> Right, which is already a mess...

Yes. It was worse before I introduced the common mmc binding, when
each driver had its own method. I left the sdhci specific one there
because there are (old) preexising users of that on powerpc that
would suffer from an incompatible change to the binding, but I
changed some of those that I knew had few existing users.

> > > > So you could actually express the same thing by either
> > > > putting the inversion into the gpio specifier or into the *-inverted
> > > > properties.
> > > > 
> > > > If you actually provide both, that would have the same meaning as not
> > > > inverting.
> > > 
> > > Hm, that'd be a weird way to configure the pin, perhaps.
> > 
> > Yes, it's silly and I don't see a reason why you'd do that, but I think
> > it should still be considered valid.
> 
> We can do that, sure, but I'm not sure we really have to apply both to end 
> up with a non-inverted pin, or just interpret it as resundant information 
> and invert once.

Fine with me, too. It's not a case I'm worried about for the practical
applications. My thought was that it would be just as easy and more logical
to combine the two using XOR as it is using OR logic wherever they get
interpreted, but if it's easier to use OR, then I don't object.

	Arnd
--
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 34f28ed..e180892 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -19,11 +19,19 @@  Only one of the properties in this section should be supplied:
 Optional properties:
 - wp-gpios: Specify GPIOs for write protection, see gpio binding
 - cd-inverted: when present, polarity on the cd gpio line is inverted
+  (deprecated)
 - wp-inverted: when present, polarity on the wp gpio line is inverted
+  (deprecated)
 - 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.
 
+cd-inverted and wp-inverted properties are deprecated ans shouldn't be used,
+instead pleaseuse the OF_GPIO_ACTIVE_LOW flag in respective GPIO bindings. Note,
+that the default (as defined by the SDHCI standard) CD and WP polarity is
+active-low, so, OF_GPIO_ACTIVE_LOW should normally be set, and only be left
+clear, if the polarity is inverted.
+
 Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion