* [PATCH v2 01/11] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties Guennadi Liakhovetski
` (9 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
tmio-mmc platform flags can be set by various means, including caller
drivers and device-tree bindings, therefore it is better to only check
them in the tmio-mmc driver proper, not in caller drivers themselves.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v2: .write16_hook should also be assigned in the absence of platform data,
e.g. with DT.
drivers/mmc/host/sh_mobile_sdhi.c | 3 +--
drivers/mmc/host/tmio_mmc_pio.c | 3 +++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 524a7f7..e0ca0ab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -153,10 +153,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
+ mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
if (p) {
mmc_data->flags = p->tmio_flags;
- if (mmc_data->flags & TMIO_MMC_HAS_IDLE_WAIT)
- mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
mmc_data->ocr_mask = p->tmio_ocr_mask;
mmc_data->capabilities |= p->tmio_caps;
mmc_data->capabilities2 |= p->tmio_caps2;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 0f992e9..b25adb4 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -928,6 +928,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
int ret;
u32 irq_mask = TMIO_MASK_CMD;
+ if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
+ pdata->write16_hook = NULL;
+
res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res_ctl)
return -EINVAL;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 01/11] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-28 22:23 ` Chris Ball
2013-01-23 15:32 ` [PATCH v2 03/11] mmc: provide a standard MMC device-tree binding parser centrally Guennadi Liakhovetski
` (8 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
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(-)
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
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-23 15:32 ` [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties Guennadi Liakhovetski
@ 2013-01-28 22:23 ` Chris Ball
2013-01-30 15:47 ` Arnd Bergmann
0 siblings, 1 reply; 30+ messages in thread
From: Chris Ball @ 2013-01-28 22:23 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
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.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-28 22:23 ` Chris Ball
@ 2013-01-30 15:47 ` Arnd Bergmann
2013-01-30 16:02 ` Guennadi Liakhovetski
0 siblings, 1 reply; 30+ messages in thread
From: Arnd Bergmann @ 2013-01-30 15:47 UTC (permalink / raw)
To: Chris Ball; +Cc: Guennadi Liakhovetski, linux-mmc, linux-sh, Magnus Damm
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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-30 15:47 ` Arnd Bergmann
@ 2013-01-30 16:02 ` Guennadi Liakhovetski
2013-01-30 16:13 ` Guennadi Liakhovetski
0 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-30 16:02 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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/
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-30 16:02 ` Guennadi Liakhovetski
@ 2013-01-30 16:13 ` Guennadi Liakhovetski
2013-01-30 16:29 ` Arnd Bergmann
0 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-30 16:13 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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/
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-30 16:13 ` Guennadi Liakhovetski
@ 2013-01-30 16:29 ` Arnd Bergmann
2013-01-30 17:03 ` Guennadi Liakhovetski
0 siblings, 1 reply; 30+ messages in thread
From: Arnd Bergmann @ 2013-01-30 16:29 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-30 16:29 ` Arnd Bergmann
@ 2013-01-30 17:03 ` Guennadi Liakhovetski
2013-01-31 0:09 ` Arnd Bergmann
0 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-30 17:03 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-30 17:03 ` Guennadi Liakhovetski
@ 2013-01-31 0:09 ` Arnd Bergmann
2013-01-31 0:20 ` Chris Ball
2013-01-31 6:47 ` Guennadi Liakhovetski
0 siblings, 2 replies; 30+ messages in thread
From: Arnd Bergmann @ 2013-01-31 0:09 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-31 0:09 ` Arnd Bergmann
@ 2013-01-31 0:20 ` Chris Ball
2013-01-31 6:47 ` Guennadi Liakhovetski
1 sibling, 0 replies; 30+ messages in thread
From: Chris Ball @ 2013-01-31 0:20 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Guennadi Liakhovetski, linux-mmc, linux-sh, Magnus Damm
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.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-31 0:09 ` Arnd Bergmann
2013-01-31 0:20 ` Chris Ball
@ 2013-01-31 6:47 ` Guennadi Liakhovetski
2013-01-31 9:00 ` Arnd Bergmann
1 sibling, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-31 6:47 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties
2013-01-31 6:47 ` Guennadi Liakhovetski
@ 2013-01-31 9:00 ` Arnd Bergmann
0 siblings, 0 replies; 30+ messages in thread
From: Arnd Bergmann @ 2013-01-31 9:00 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Chris Ball, linux-mmc, linux-sh, Magnus Damm
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 03/11] mmc: provide a standard MMC device-tree binding parser centrally
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 01/11] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 02/11] mmc: deprecate redundant cd-inverted and wp-inverted DT properties Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 04/11] mmc: (cosmetic) remove "extern" from function declarations Guennadi Liakhovetski
` (7 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
MMC defines a number of standard DT bindings. Having each driver parse
them individually adds code redundancy and is error prone. Provide a
standard function to unify the parsing. After all drivers are converted
to using it instead of their own parsers, this function can be integrated
into mmc_alloc_host().
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/core/host.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
2 files changed, 83 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..e4c1cbd 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -15,6 +15,8 @@
#include <linux/device.h>
#include <linux/err.h>
#include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#include <linux/pagemap.h>
#include <linux/export.h>
#include <linux/leds.h>
@@ -23,6 +25,7 @@
#include <linux/mmc/host.h>
#include <linux/mmc/card.h>
+#include <linux/mmc/slot-gpio.h>
#include "core.h"
#include "host.h"
@@ -294,6 +297,85 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
#endif
+void mmc_of_parse(struct mmc_host *host)
+{
+ struct device_node *np;
+ u32 bus_width;
+ bool legacy_inv_cd, legacy_inv_wp;
+ enum of_gpio_flags flags;
+ int len, ret, gpio;
+
+ if (!host->parent || !host->parent->of_node)
+ return;
+
+ np = host->parent->of_node;
+
+ /* Both CD and WP pins are by default active low */
+
+ legacy_inv_cd = !!of_find_property(np, "cd-inverted", &len);
+ legacy_inv_wp = !!of_find_property(np, "wp-inverted", &len);
+
+ if (legacy_inv_cd || legacy_inv_wp)
+ dev_warn(host->parent,
+ "\"cd-/wp-inverted\" properties are deprecated, use GPIO flags");
+
+ if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0) {
+ dev_err(host->parent,
+ "Compulsory \"bus-width\" property is missing!\n");
+ return;
+ }
+
+ switch (bus_width) {
+ case 8:
+ host->caps |= MMC_CAP_8_BIT_DATA;
+ /* Hosts, capable of 8-bit transfers can also do 4 bits */
+ case 4:
+ host->caps |= MMC_CAP_4_BIT_DATA;
+ break;
+ case 1:
+ break;
+ default:
+ dev_err(host->parent,
+ "Invalid \"bus-width\" value %ud!\n", bus_width);
+ }
+
+ /* Optional property: ignore errors */
+ of_property_read_u32_array(np, "max-frequency", &host->f_max, 1);
+
+ if (of_find_property(np, "non-removable", &len)) {
+ host->caps |= MMC_CAP_NONREMOVABLE;
+ } else {
+ if (of_find_property(np, "broken-cd", &len))
+ host->caps |= MMC_CAP_NEEDS_POLL;
+
+ gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
+ if (gpio_is_valid(gpio)) {
+ if (legacy_inv_cd || !(flags & OF_GPIO_ACTIVE_LOW))
+ host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ ret = mmc_gpio_request_cd(host, gpio);
+ if (ret < 0)
+ dev_err(host->parent,
+ "Failed to request CD GPIO #%d: %d!\n",
+ gpio, ret);
+ else
+ dev_info(host->parent, "Got CD GPIO #%d.\n",
+ gpio);
+ }
+ }
+
+ gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
+ if (gpio_is_valid(gpio)) {
+ if (legacy_inv_wp || !(flags & OF_GPIO_ACTIVE_LOW))
+ host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ ret = mmc_gpio_request_ro(host, gpio);
+ if (ret < 0)
+ dev_err(host->parent,
+ "Failed to request WP GPIO: %d!\n", ret);
+ }
+}
+
+EXPORT_SYMBOL(mmc_of_parse);
+
/**
* mmc_alloc_host - initialise the per-host structure.
* @extra: sizeof private data structure
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..13c0c8d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
extern int mmc_add_host(struct mmc_host *);
extern void mmc_remove_host(struct mmc_host *);
extern void mmc_free_host(struct mmc_host *);
+void mmc_of_parse(struct mmc_host *host);
static inline void *mmc_priv(struct mmc_host *host)
{
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 04/11] mmc: (cosmetic) remove "extern" from function declarations
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (2 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 03/11] mmc: provide a standard MMC device-tree binding parser centrally Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 05/11] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings Guennadi Liakhovetski
` (6 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
The "extern" keyword isn't required in function declarations, remove it.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
include/linux/mmc/host.h | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 13c0c8d..356ae0a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -341,10 +341,10 @@ struct mmc_host {
unsigned long private[0] ____cacheline_aligned;
};
-extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
-extern int mmc_add_host(struct mmc_host *);
-extern void mmc_remove_host(struct mmc_host *);
-extern void mmc_free_host(struct mmc_host *);
+struct mmc_host *mmc_alloc_host(int extra, struct device *);
+int mmc_add_host(struct mmc_host *);
+void mmc_remove_host(struct mmc_host *);
+void mmc_free_host(struct mmc_host *);
void mmc_of_parse(struct mmc_host *host);
static inline void *mmc_priv(struct mmc_host *host)
@@ -358,16 +358,16 @@ static inline void *mmc_priv(struct mmc_host *host)
#define mmc_classdev(x) (&(x)->class_dev)
#define mmc_hostname(x) (dev_name(&(x)->class_dev))
-extern int mmc_suspend_host(struct mmc_host *);
-extern int mmc_resume_host(struct mmc_host *);
+int mmc_suspend_host(struct mmc_host *);
+int mmc_resume_host(struct mmc_host *);
-extern int mmc_power_save_host(struct mmc_host *host);
-extern int mmc_power_restore_host(struct mmc_host *host);
+int mmc_power_save_host(struct mmc_host *host);
+int mmc_power_restore_host(struct mmc_host *host);
-extern void mmc_detect_change(struct mmc_host *, unsigned long delay);
-extern void mmc_request_done(struct mmc_host *, struct mmc_request *);
+void mmc_detect_change(struct mmc_host *, unsigned long delay);
+void mmc_request_done(struct mmc_host *, struct mmc_request *);
-extern int mmc_cache_ctrl(struct mmc_host *, u8);
+int mmc_cache_ctrl(struct mmc_host *, u8);
static inline void mmc_signal_sdio_irq(struct mmc_host *host)
{
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 05/11] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (3 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 04/11] mmc: (cosmetic) remove "extern" from function declarations Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-28 22:25 ` Chris Ball
2013-01-23 15:32 ` [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings Guennadi Liakhovetski
` (5 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
Use mmc_of_parse() to get interface capability flags and used GPIOs from
device-tree bindings.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/host/sh_mmcif.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 741aeb9..4de94a1 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1329,6 +1329,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
ret = -ENOMEM;
goto ealloch;
}
+ mmc_of_parse(mmc);
host = mmc_priv(mmc);
host->mmc = mmc;
host->addr = reg;
@@ -1341,7 +1342,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
mmc->ops = &sh_mmcif_ops;
sh_mmcif_init_ocr(host);
- mmc->caps = MMC_CAP_MMC_HIGHSPEED;
+ mmc->caps |= MMC_CAP_MMC_HIGHSPEED;
if (pd && pd->caps)
mmc->caps |= pd->caps;
mmc->max_segs = 32;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 05/11] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
2013-01-23 15:32 ` [PATCH v2 05/11] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings Guennadi Liakhovetski
@ 2013-01-28 22:25 ` Chris Ball
0 siblings, 0 replies; 30+ messages in thread
From: Chris Ball @ 2013-01-28 22:25 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
Hi Guennadi,
On Wed, Jan 23 2013, Guennadi Liakhovetski wrote:
> Use mmc_of_parse() to get interface capability flags and used GPIOs from
> device-tree bindings.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> drivers/mmc/host/sh_mmcif.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 741aeb9..4de94a1 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1329,6 +1329,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
> ret = -ENOMEM;
> goto ealloch;
> }
> + mmc_of_parse(mmc);
> host = mmc_priv(mmc);
> host->mmc = mmc;
> host->addr = reg;
> @@ -1341,7 +1342,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
> mmc->ops = &sh_mmcif_ops;
> sh_mmcif_init_ocr(host);
>
> - mmc->caps = MMC_CAP_MMC_HIGHSPEED;
> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED;
> if (pd && pd->caps)
> mmc->caps |= pd->caps;
> mmc->max_segs = 32;
This patch doesn't apply because you don't seem to have this patch:
Author: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Date: Wed Dec 12 15:38:10 2012 +0100
Subject: mmc: sh_mmcif: Avoid unnecessary mmc_delay() at mmc_card_sleepawake()
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (4 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 05/11] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-24 15:34 ` Guennadi Liakhovetski
2013-02-01 4:23 ` Simon Horman
2013-01-23 15:32 ` [PATCH v2 07/11] mmc: tmio-mmc: parse " Guennadi Liakhovetski
` (4 subsequent siblings)
10 siblings, 2 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
Define device-tree bindings for the tmio-mmc driver to be able to specify
parameters, currently provided in platform data.
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
Please, comment on this one, since it is defining an ABI
Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt
diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
new file mode 100644
index 0000000..dd8decd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
@@ -0,0 +1,19 @@
+* Toshiba Mobile IO SD/MMC controller
+
+The tmio-mmc driver doesn't probe its devices actively, instead its binding to
+devices is managed by either MFD drivers or by the sh_mobile_sdhi platform
+driver. Those drivers supply the tmio-mmc driver with platform data, that either
+describe hardware capabilities, known to them, or are obtained by them from
+their own platform data or from their DT information. In the latter case all
+compulsory and any optional properties, common to all SD/MMC drivers, as
+described in mmc.txt, should or can be used. Additionally the following optional
+bindings can be used. They set either respective TMIO_MMC_* flags or MMC_CAP_*
+capabilities.
+
+Optional properties:
+- toshiba,mmc-wrprotect-disable : set TMIO_MMC_WRPROTECT_DISABLE flag
+- toshiba,mmc-blksz-2bytes : set TMIO_MMC_BLKSZ_2BYTES
+- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
+ supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
+ TMIO_MMC_SDIO_IRQ is also set
+- toshiba,mmc-has-idle-wait : set TMIO_MMC_HAS_IDLE_WAIT
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-23 15:32 ` [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings Guennadi Liakhovetski
@ 2013-01-24 15:34 ` Guennadi Liakhovetski
2013-01-24 15:39 ` Chris Ball
2013-02-01 4:23 ` Simon Horman
1 sibling, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-24 15:34 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball
After an internal discussion it occurred to us, that this binding
On Wed, 23 Jan 2013, Guennadi Liakhovetski wrote:
[snip]
> +- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
> + supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
> + TMIO_MMC_SDIO_IRQ is also set
actually isn't tmio-mmc specific, so, it can be moved to [1] as
+- cap-mmc-sdio-irq: SDIO IRQ is supported on this hardware
Chris, what do you think?
[1] http://www.spinics.net/lists/linux-mmc/msg18625.html
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-24 15:34 ` Guennadi Liakhovetski
@ 2013-01-24 15:39 ` Chris Ball
2013-01-24 15:58 ` Guennadi Liakhovetski
0 siblings, 1 reply; 30+ messages in thread
From: Chris Ball @ 2013-01-24 15:39 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
Hi,
On Thu, Jan 24 2013, Guennadi Liakhovetski wrote:
> After an internal discussion it occurred to us, that this binding
>
> On Wed, 23 Jan 2013, Guennadi Liakhovetski wrote:
>
> [snip]
>
>> +- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
>> + supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
>> + TMIO_MMC_SDIO_IRQ is also set
>
> actually isn't tmio-mmc specific, so, it can be moved to [1] as
>
> +- cap-mmc-sdio-irq: SDIO IRQ is supported on this hardware
>
> Chris, what do you think?
Sounds right; perhaps we should call it "enable-sdio-irq" for consistency
with the existing "enable-sdio-wakeup" (which sets a pm_caps flag)?
Thanks,
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-24 15:39 ` Chris Ball
@ 2013-01-24 15:58 ` Guennadi Liakhovetski
2013-01-24 16:03 ` Chris Ball
0 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-24 15:58 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, linux-sh, Magnus Damm
On Thu, 24 Jan 2013, Chris Ball wrote:
> Hi,
>
> On Thu, Jan 24 2013, Guennadi Liakhovetski wrote:
> > After an internal discussion it occurred to us, that this binding
> >
> > On Wed, 23 Jan 2013, Guennadi Liakhovetski wrote:
> >
> > [snip]
> >
> >> +- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
> >> + supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
> >> + TMIO_MMC_SDIO_IRQ is also set
> >
> > actually isn't tmio-mmc specific, so, it can be moved to [1] as
> >
> > +- cap-mmc-sdio-irq: SDIO IRQ is supported on this hardware
> >
> > Chris, what do you think?
>
> Sounds right; perhaps we should call it "enable-sdio-irq" for consistency
> with the existing "enable-sdio-wakeup" (which sets a pm_caps flag)?
I tried to keep this binding similar to others, that I proposed in "mmc:
add DT bindings for more MMC capability flags." Actually, the above is
indeed wrong, I would call it "cap-sdio-irq." And in that patch I tried to
keep binding names resembling as closely as possible respective MMC_CAP_*
flags. I think, it would have been better if "enable-sdio-wakeup" and
"keep-power-in-suspend" were also named, following the same rule, but it's
too late now. Anyway, I'm not too concerned about the names. We can use
"enable-sdio-irq" too if you like.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-24 15:58 ` Guennadi Liakhovetski
@ 2013-01-24 16:03 ` Chris Ball
2013-01-30 14:07 ` Guennadi Liakhovetski
0 siblings, 1 reply; 30+ messages in thread
From: Chris Ball @ 2013-01-24 16:03 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
Hi,
On Thu, Jan 24 2013, Guennadi Liakhovetski wrote:
> I tried to keep this binding similar to others, that I proposed in "mmc:
> add DT bindings for more MMC capability flags." Actually, the above is
> indeed wrong, I would call it "cap-sdio-irq." And in that patch I tried to
> keep binding names resembling as closely as possible respective MMC_CAP_*
> flags. I think, it would have been better if "enable-sdio-wakeup" and
> "keep-power-in-suspend" were also named, following the same rule, but it's
> too late now. Anyway, I'm not too concerned about the names. We can use
> "enable-sdio-irq" too if you like.
I see. Okay, let's go with your proposed cap-* for each MMC_CAP_*, and
the pm_caps can stay as they are.
Thanks!
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-24 16:03 ` Chris Ball
@ 2013-01-30 14:07 ` Guennadi Liakhovetski
2013-01-30 14:09 ` Chris Ball
0 siblings, 1 reply; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-30 14:07 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, linux-sh, Magnus Damm
Hi Chris
On Thu, 24 Jan 2013, Chris Ball wrote:
> Hi,
>
> On Thu, Jan 24 2013, Guennadi Liakhovetski wrote:
> > I tried to keep this binding similar to others, that I proposed in "mmc:
> > add DT bindings for more MMC capability flags." Actually, the above is
> > indeed wrong, I would call it "cap-sdio-irq." And in that patch I tried to
> > keep binding names resembling as closely as possible respective MMC_CAP_*
> > flags. I think, it would have been better if "enable-sdio-wakeup" and
> > "keep-power-in-suspend" were also named, following the same rule, but it's
> > too late now. Anyway, I'm not too concerned about the names. We can use
> > "enable-sdio-irq" too if you like.
>
> I see. Okay, let's go with your proposed cap-* for each MMC_CAP_*, and
> the pm_caps can stay as they are.
Thanks, let's do that. But in fact, in a recent discussion it has been
pointed out to me, that this property
+- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
+ supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
+ TMIO_MMC_SDIO_IRQ is also set
should be common for all MMC drivers: it should be possible to decide per
SD interface, whether SDIO IRQ signalling should be enabled. What do you
think? Shall we add a global "cap-sdio-irq" DT property instead of a
toshiba-specific one?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-30 14:07 ` Guennadi Liakhovetski
@ 2013-01-30 14:09 ` Chris Ball
0 siblings, 0 replies; 30+ messages in thread
From: Chris Ball @ 2013-01-30 14:09 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm
Hi,
On Wed, Jan 30 2013, Guennadi Liakhovetski wrote:
>> On Thu, Jan 24 2013, Guennadi Liakhovetski wrote:
>> > I tried to keep this binding similar to others, that I proposed in "mmc:
>> > add DT bindings for more MMC capability flags." Actually, the above is
>> > indeed wrong, I would call it "cap-sdio-irq." And in that patch I tried to
>> > keep binding names resembling as closely as possible respective MMC_CAP_*
>> > flags. I think, it would have been better if "enable-sdio-wakeup" and
>> > "keep-power-in-suspend" were also named, following the same rule, but it's
>> > too late now. Anyway, I'm not too concerned about the names. We can use
>> > "enable-sdio-irq" too if you like.
>>
>> I see. Okay, let's go with your proposed cap-* for each MMC_CAP_*, and
>> the pm_caps can stay as they are.
>
> Thanks, let's do that. But in fact, in a recent discussion it has been
> pointed out to me, that this property
>
> +- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
> + supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
> + TMIO_MMC_SDIO_IRQ is also set
>
> should be common for all MMC drivers: it should be possible to decide per
> SD interface, whether SDIO IRQ signalling should be enabled. What do you
> think? Shall we add a global "cap-sdio-irq" DT property instead of a
> toshiba-specific one?
Yes, please.
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings
2013-01-23 15:32 ` [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings Guennadi Liakhovetski
2013-01-24 15:34 ` Guennadi Liakhovetski
@ 2013-02-01 4:23 ` Simon Horman
1 sibling, 0 replies; 30+ messages in thread
From: Simon Horman @ 2013-02-01 4:23 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Magnus Damm, Chris Ball
On Wed, Jan 23, 2013 at 04:32:33PM +0100, Guennadi Liakhovetski wrote:
> Define device-tree bindings for the tmio-mmc driver to be able to specify
> parameters, currently provided in platform data.
>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> Please, comment on this one, since it is defining an ABI
>
> Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 19 +++++++++++++++++++
> 1 files changed, 19 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> new file mode 100644
> index 0000000..dd8decd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -0,0 +1,19 @@
> +* Toshiba Mobile IO SD/MMC controller
> +
> +The tmio-mmc driver doesn't probe its devices actively, instead its binding to
> +devices is managed by either MFD drivers or by the sh_mobile_sdhi platform
> +driver. Those drivers supply the tmio-mmc driver with platform data, that either
> +describe hardware capabilities, known to them, or are obtained by them from
> +their own platform data or from their DT information. In the latter case all
> +compulsory and any optional properties, common to all SD/MMC drivers, as
> +described in mmc.txt, should or can be used. Additionally the following optional
> +bindings can be used. They set either respective TMIO_MMC_* flags or MMC_CAP_*
> +capabilities.
> +
> +Optional properties:
> +- toshiba,mmc-wrprotect-disable : set TMIO_MMC_WRPROTECT_DISABLE flag
> +- toshiba,mmc-blksz-2bytes : set TMIO_MMC_BLKSZ_2BYTES
> +- toshiba,mmc-cap-sdio-irq : SDIO IRQ signalling should be used, if
> + supported by the hardware, i.e. set MMC_CAP_SDIO_IRQ if
> + TMIO_MMC_SDIO_IRQ is also set
> +- toshiba,mmc-has-idle-wait : set TMIO_MMC_HAS_IDLE_WAIT
FWIW, TMIO_MMC_HAS_IDLE_WAIT appears to be required for SDHI0 to
function on the Marzen board. As I have been doing some work on
bring up the Marzen board using DT I am very happy to see these new
bindings.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 07/11] mmc: tmio-mmc: parse device-tree bindings
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (5 preceding siblings ...)
2013-01-23 15:32 ` [PATCH/RFC v2 06/11] mmc: tmio-mmc: define device-tree bindings Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 08/11] mmc: sh_mobile_sdhi: remove unused .pdata field Guennadi Liakhovetski
` (3 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
Add parsing of common and driver-specific DT bindings to the tmio-mmc
MMC host driver.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v2: check, if DT assigned a card-detect IRQ, in which case don't enable TMIO
native CD IRQ.
drivers/mmc/host/tmio_mmc_pio.c | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index b25adb4..13729b6 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -918,6 +918,24 @@ static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
}
+static void tmio_mmc_of_parse(struct platform_device *pdev,
+ struct tmio_mmc_data *pdata)
+{
+ const struct device_node *np = pdev->dev.of_node;
+ if (!np)
+ return;
+
+ if (of_get_property(np, "toshiba,mmc-wrprotect-disable", NULL))
+ pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
+ if (of_get_property(np, "toshiba,mmc-blksz-2bytes", NULL))
+ pdata->flags |= TMIO_MMC_BLKSZ_2BYTES;
+ if (of_get_property(np, "toshiba,mmc-cap-sdio-irq", NULL) &&
+ pdata->flags & TMIO_MMC_SDIO_IRQ)
+ pdata->capabilities |= MMC_CAP_SDIO_IRQ;
+ if (of_get_property(np, "toshiba,mmc-has-idle-wait", NULL))
+ pdata->flags |= TMIO_MMC_HAS_IDLE_WAIT;
+}
+
int tmio_mmc_host_probe(struct tmio_mmc_host **host,
struct platform_device *pdev,
struct tmio_mmc_data *pdata)
@@ -928,6 +946,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
int ret;
u32 irq_mask = TMIO_MASK_CMD;
+ tmio_mmc_of_parse(pdev, pdata);
+
if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
pdata->write16_hook = NULL;
@@ -939,6 +959,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
if (!mmc)
return -ENOMEM;
+ mmc_of_parse(mmc);
+
pdata->dev = &pdev->dev;
_host = mmc_priv(mmc);
_host->pdata = pdata;
@@ -959,7 +981,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
}
mmc->ops = &tmio_mmc_ops;
- mmc->caps = MMC_CAP_4_BIT_DATA | pdata->capabilities;
+ mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
mmc->caps2 = pdata->capabilities2;
mmc->max_segs = 32;
mmc->max_blk_size = 512;
@@ -971,7 +993,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
mmc->caps & MMC_CAP_NEEDS_POLL ||
- mmc->caps & MMC_CAP_NONREMOVABLE);
+ mmc->caps & MMC_CAP_NONREMOVABLE ||
+ mmc->slot.cd_irq >= 0);
_host->power = false;
pm_runtime_enable(&pdev->dev);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 08/11] mmc: sh_mobile_sdhi: remove unused .pdata field
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (6 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 07/11] mmc: tmio-mmc: parse " Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 09/11] mmc: sh_mobile_sdhi: use managed resource allocations Guennadi Liakhovetski
` (2 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
The struct sh_mobile_sdhi_info::pdata field was only used for platform-
based card detection and isn't used anymore since the migration to GPIO-
based MMC slot functions. Remove it.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/host/sh_mobile_sdhi.c | 4 ----
include/linux/mmc/sh_mobile_sdhi.h | 2 --
2 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index e0ca0ab..17d5778 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -135,7 +135,6 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
mmc_data = &priv->mmc_data;
if (p) {
- p->pdata = mmc_data;
if (p->init) {
ret = p->init(pdev, &sdhi_ops);
if (ret)
@@ -284,9 +283,6 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
int i = 0, irq;
- if (p)
- p->pdata = NULL;
-
tmio_mmc_host_remove(host);
while (1) {
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index b65679f..b76bcf0 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -4,7 +4,6 @@
#include <linux/types.h>
struct platform_device;
-struct tmio_mmc_data;
#define SH_MOBILE_SDHI_IRQ_CARD_DETECT "card_detect"
#define SH_MOBILE_SDHI_IRQ_SDCARD "sdcard"
@@ -26,7 +25,6 @@ struct sh_mobile_sdhi_info {
unsigned long tmio_caps2;
u32 tmio_ocr_mask; /* available MMC voltages */
unsigned int cd_gpio;
- struct tmio_mmc_data *pdata;
void (*set_pwr)(struct platform_device *pdev, int state);
int (*get_cd)(struct platform_device *pdev);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 09/11] mmc: sh_mobile_sdhi: use managed resource allocations
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (7 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 08/11] mmc: sh_mobile_sdhi: remove unused .pdata field Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 10/11] mmc: tmio: remove unused and deprecated symbols Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 11/11] mmc: tmio: add support for the VccQ regulator Guennadi Liakhovetski
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
Use managed allocations to get memory, clock and interrupts . This
significantly simplifies clean up paths.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/mmc/host/sh_mobile_sdhi.c | 57 +++++++++----------------------------
1 files changed, 14 insertions(+), 43 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 17d5778..e095d5c 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -126,7 +126,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
int irq, ret, i = 0;
bool multiplexed_isr = true;
- priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
if (priv == NULL) {
dev_err(&pdev->dev, "kzalloc failed\n");
return -ENOMEM;
@@ -138,11 +138,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
if (p->init) {
ret = p->init(pdev, &sdhi_ops);
if (ret)
- goto einit;
+ return ret;
}
}
- priv->clk = clk_get(&pdev->dev, NULL);
+ priv->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(priv->clk)) {
ret = PTR_ERR(priv->clk);
dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
@@ -197,33 +197,33 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
if (irq >= 0) {
multiplexed_isr = false;
- ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
+ ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_card_detect_irq, 0,
dev_name(&pdev->dev), host);
if (ret)
- goto eirq_card_detect;
+ goto eirq;
}
irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
if (irq >= 0) {
multiplexed_isr = false;
- ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+ ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdio_irq, 0,
dev_name(&pdev->dev), host);
if (ret)
- goto eirq_sdio;
+ goto eirq;
}
irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
if (irq >= 0) {
multiplexed_isr = false;
- ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+ ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdcard_irq, 0,
dev_name(&pdev->dev), host);
if (ret)
- goto eirq_sdcard;
+ goto eirq;
} else if (!multiplexed_isr) {
dev_err(&pdev->dev,
"Principal SD-card IRQ is missing among named interrupts\n");
ret = irq;
- goto eirq_sdcard;
+ goto eirq;
}
if (multiplexed_isr) {
@@ -232,15 +232,15 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
if (irq < 0)
break;
i++;
- ret = request_irq(irq, tmio_mmc_irq, 0,
+ ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_irq, 0,
dev_name(&pdev->dev), host);
if (ret)
- goto eirq_multiplexed;
+ goto eirq;
}
/* There must be at least one IRQ source */
if (!i)
- goto eirq_multiplexed;
+ goto eirq;
}
dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
@@ -250,28 +250,12 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
return ret;
-eirq_multiplexed:
- while (i--) {
- irq = platform_get_irq(pdev, i);
- free_irq(irq, host);
- }
-eirq_sdcard:
- irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
- if (irq >= 0)
- free_irq(irq, host);
-eirq_sdio:
- irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
- if (irq >= 0)
- free_irq(irq, host);
-eirq_card_detect:
+eirq:
tmio_mmc_host_remove(host);
eprobe:
- clk_put(priv->clk);
eclkget:
if (p && p->cleanup)
p->cleanup(pdev);
-einit:
- kfree(priv);
return ret;
}
@@ -279,26 +263,13 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
{
struct mmc_host *mmc = platform_get_drvdata(pdev);
struct tmio_mmc_host *host = mmc_priv(mmc);
- struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
- int i = 0, irq;
tmio_mmc_host_remove(host);
- while (1) {
- irq = platform_get_irq(pdev, i++);
- if (irq < 0)
- break;
- free_irq(irq, host);
- }
-
- clk_put(priv->clk);
-
if (p && p->cleanup)
p->cleanup(pdev);
- kfree(priv);
-
return 0;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 10/11] mmc: tmio: remove unused and deprecated symbols
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (8 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 09/11] mmc: sh_mobile_sdhi: use managed resource allocations Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
2013-01-23 15:32 ` [PATCH v2 11/11] mmc: tmio: add support for the VccQ regulator Guennadi Liakhovetski
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
The tmio_mmc_cd_wakeup() inline function has been deprecated since 3.4 and
is unused since 3.4 too. Remove them.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
include/linux/mfd/tmio.h | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index d83af39..99bf3e66 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -65,12 +65,6 @@
*/
#define TMIO_MMC_SDIO_IRQ (1 << 2)
/*
- * Some platforms can detect card insertion events with controller powered
- * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
- * and cd_flags fields of struct tmio_mmc_data.
- */
-#define TMIO_MMC_HAS_COLD_CD (1 << 3)
-/*
* Some controllers require waiting for the SD bus to become
* idle before writing to some registers.
*/
@@ -117,18 +111,6 @@ struct tmio_mmc_data {
};
/*
- * This function is deprecated and will be removed soon. Please, convert your
- * platform to use drivers/mmc/core/cd-gpio.c
- */
-#include <linux/mmc/host.h>
-static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
-{
- if (pdata)
- mmc_detect_change(dev_get_drvdata(pdata->dev),
- msecs_to_jiffies(100));
-}
-
-/*
* data for the NAND controller
*/
struct tmio_nand_data {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 11/11] mmc: tmio: add support for the VccQ regulator
2013-01-23 15:32 [PATCH v2 00/11] mmc: core and driver DT and related development Guennadi Liakhovetski
` (9 preceding siblings ...)
2013-01-23 15:32 ` [PATCH v2 10/11] mmc: tmio: remove unused and deprecated symbols Guennadi Liakhovetski
@ 2013-01-23 15:32 ` Guennadi Liakhovetski
10 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-23 15:32 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-sh, Magnus Damm, Chris Ball, Guennadi Liakhovetski
Some SD/MMC interfaces use 2 power regulators: one to power the card itself
(Vcc) and another one to pull signal lines up (VccQ). In case of eMMC and
UHS SD cards the regulators also have to be configured to supply different
voltages. The preferred order of turning supply power on and off is to
turn Vcc first on and last off.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v2:
1. delays are necessary for reliable SDIO card probing
2. balance calls to tmio_mmc_power_on() and tmio_mmc_power_off()
drivers/mmc/host/tmio_mmc_pio.c | 56 ++++++++++++++++++++++++++++++++-------
1 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 13729b6..cd3b777 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -43,6 +43,7 @@
#include <linux/platform_device.h>
#include <linux/pm_qos.h>
#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
#include <linux/scatterlist.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
@@ -155,6 +156,7 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, int new_clock)
host->set_clk_div(host->pdev, (clk>>22) & 1);
sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, clk & 0x1ff);
+ msleep(10);
}
static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
@@ -768,16 +770,48 @@ static int tmio_mmc_clk_update(struct mmc_host *mmc)
return ret;
}
-static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
+static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
{
struct mmc_host *mmc = host->mmc;
+ int ret = 0;
+
+ /* .set_ios() is returning void, so, no chance to report an error */
if (host->set_pwr)
- host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
+ host->set_pwr(host->pdev, 1);
+
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+ /*
+ * Attention: empiric value. With a b43 WiFi SDIO card this
+ * delay proved necessary for reliable card-insertion probing.
+ * 100us were not enough. Is this the same 140us delay, as in
+ * tmio_mmc_set_ios()?
+ */
+ udelay(200);
+ }
+ /*
+ * It seems, VccQ should be switched on after Vcc, this is also what the
+ * omap_hsmmc.c driver does.
+ */
+ if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
+ regulator_enable(mmc->supply.vqmmc);
+ udelay(200);
+ }
+}
+
+static void tmio_mmc_power_off(struct tmio_mmc_host *host)
+{
+ struct mmc_host *mmc = host->mmc;
+
+ if (!IS_ERR(mmc->supply.vqmmc))
+ regulator_disable(mmc->supply.vqmmc);
+
if (!IS_ERR(mmc->supply.vmmc))
- /* Errors ignored... */
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
- ios->power_mode ? ios->vdd : 0);
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (host->set_pwr)
+ host->set_pwr(host->pdev, 0);
}
/* Set MMC clock / power.
@@ -828,18 +862,20 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
if (!host->power) {
tmio_mmc_clk_update(mmc);
pm_runtime_get_sync(dev);
- host->power = true;
}
tmio_mmc_set_clock(host, ios->clock);
- /* power up SD bus */
- tmio_mmc_set_power(host, ios);
+ if (!host->power) {
+ /* power up SD card and the bus */
+ tmio_mmc_power_on(host, ios->vdd);
+ host->power = true;
+ }
/* start bus clock */
tmio_mmc_clk_start(host);
} else if (ios->power_mode != MMC_POWER_UP) {
- if (ios->power_mode == MMC_POWER_OFF)
- tmio_mmc_set_power(host, ios);
if (host->power) {
struct tmio_mmc_data *pdata = host->pdata;
+ if (ios->power_mode == MMC_POWER_OFF)
+ tmio_mmc_power_off(host);
tmio_mmc_clk_stop(host);
host->power = false;
pm_runtime_put(dev);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 30+ messages in thread