diff mbox

[1/3] mmc: mvsdio: Support inverted CD and WP GPIO lines

Message ID 1366660500-26835-2-git-send-email-gmbnomis@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Simon Baatz April 22, 2013, 7:54 p.m. UTC
The slot-gpio helper functions allow to specify whether card detect and
write protect GPIO lines are active high or active low.  Since DT
allows to specify active low/high for GPIO lines as well, we can simply
initialize the flags for slot-gpio using the GPIO flags to support all
cases.

We need to adapt the dts files that use CD/WP via GPIO accordingly.
Only the dts for "MPL CEC4" is affected by this change.

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
---
 arch/arm/boot/dts/kirkwood-mplcec4.dts |    2 +-
 drivers/mmc/host/mvsdio.c              |   14 ++++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

Comments

Chris Ball April 22, 2013, 8:03 p.m. UTC | #1
Hi Simon,

On Mon, Apr 22 2013, Simon Baatz wrote:
> diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> index 145cdaf..d444efd 100644
> --- a/drivers/mmc/host/mvsdio.c
> +++ b/drivers/mmc/host/mvsdio.c
> @@ -691,6 +691,7 @@ static int __init mvsd_probe(struct platform_device *pdev)
>  	struct resource *r;
>  	int ret, irq;
>  	int gpio_card_detect, gpio_write_protect;
> +	enum of_gpio_flags gpio_flags;
>  	struct pinctrl *pinctrl;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -731,8 +732,17 @@ static int __init mvsd_probe(struct platform_device *pdev)
>  		}
>  
>  		host->base_clock = clk_get_rate(host->clk) / 2;
> -		gpio_card_detect = of_get_named_gpio(np, "cd-gpios", 0);
> -		gpio_write_protect = of_get_named_gpio(np, "wp-gpios", 0);
> +		gpio_card_detect = of_get_named_gpio_flags(np, "cd-gpios", 0,
> +						     &gpio_flags);
> +		if (gpio_is_valid(gpio_card_detect) &&
> +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> +			mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +
> +		gpio_write_protect = of_get_named_gpio_flags(np, "wp-gpios", 0,
> +						       &gpio_flags);
> +		if (gpio_is_valid(gpio_write_protect) &&
> +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> +			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>  	} else {
>  		const struct mvsdio_platform_data *mvsd_data;
>  		mvsd_data = pdev->dev.platform_data;

The core function mmc_of_parse() (new in 3.9) will do all of this DT
parsing for you, and will also take care of the driver's later calls
to mmc_gpio_request_{cd,wp}().  Could you try using that function
instead, please?

Thanks,

- Chris.
Andrew Lunn April 22, 2013, 8:37 p.m. UTC | #2
On Mon, Apr 22, 2013 at 04:03:15PM -0400, Chris Ball wrote:
> Hi Simon,
> 
> On Mon, Apr 22 2013, Simon Baatz wrote:
> > diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> > index 145cdaf..d444efd 100644
> > --- a/drivers/mmc/host/mvsdio.c
> > +++ b/drivers/mmc/host/mvsdio.c
> > @@ -691,6 +691,7 @@ static int __init mvsd_probe(struct platform_device *pdev)
> >  	struct resource *r;
> >  	int ret, irq;
> >  	int gpio_card_detect, gpio_write_protect;
> > +	enum of_gpio_flags gpio_flags;
> >  	struct pinctrl *pinctrl;
> >  
> >  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -731,8 +732,17 @@ static int __init mvsd_probe(struct platform_device *pdev)
> >  		}
> >  
> >  		host->base_clock = clk_get_rate(host->clk) / 2;
> > -		gpio_card_detect = of_get_named_gpio(np, "cd-gpios", 0);
> > -		gpio_write_protect = of_get_named_gpio(np, "wp-gpios", 0);
> > +		gpio_card_detect = of_get_named_gpio_flags(np, "cd-gpios", 0,
> > +						     &gpio_flags);
> > +		if (gpio_is_valid(gpio_card_detect) &&
> > +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> > +			mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> > +
> > +		gpio_write_protect = of_get_named_gpio_flags(np, "wp-gpios", 0,
> > +						       &gpio_flags);
> > +		if (gpio_is_valid(gpio_write_protect) &&
> > +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> > +			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> >  	} else {
> >  		const struct mvsdio_platform_data *mvsd_data;
> >  		mvsd_data = pdev->dev.platform_data;
> 
> The core function mmc_of_parse() (new in 3.9) will do all of this DT
> parsing for you, and will also take care of the driver's later calls
> to mmc_gpio_request_{cd,wp}().  Could you try using that function
> instead, please?

Hi Chris, Simon

I have a vague recollection that Thomas Petazzoni already did this
conversion. Take a look in linux-next, or maybe Jason can point you at
a branch.

  Andrew
--
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
Jason Cooper April 22, 2013, 9:09 p.m. UTC | #3
On Mon, Apr 22, 2013 at 10:37:23PM +0200, Andrew Lunn wrote:
> On Mon, Apr 22, 2013 at 04:03:15PM -0400, Chris Ball wrote:
> > Hi Simon,
> > 
> > On Mon, Apr 22 2013, Simon Baatz wrote:
> > > diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> > > index 145cdaf..d444efd 100644
> > > --- a/drivers/mmc/host/mvsdio.c
> > > +++ b/drivers/mmc/host/mvsdio.c
> > > @@ -691,6 +691,7 @@ static int __init mvsd_probe(struct platform_device *pdev)
> > >  	struct resource *r;
> > >  	int ret, irq;
> > >  	int gpio_card_detect, gpio_write_protect;
> > > +	enum of_gpio_flags gpio_flags;
> > >  	struct pinctrl *pinctrl;
> > >  
> > >  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > @@ -731,8 +732,17 @@ static int __init mvsd_probe(struct platform_device *pdev)
> > >  		}
> > >  
> > >  		host->base_clock = clk_get_rate(host->clk) / 2;
> > > -		gpio_card_detect = of_get_named_gpio(np, "cd-gpios", 0);
> > > -		gpio_write_protect = of_get_named_gpio(np, "wp-gpios", 0);
> > > +		gpio_card_detect = of_get_named_gpio_flags(np, "cd-gpios", 0,
> > > +						     &gpio_flags);
> > > +		if (gpio_is_valid(gpio_card_detect) &&
> > > +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> > > +			mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> > > +
> > > +		gpio_write_protect = of_get_named_gpio_flags(np, "wp-gpios", 0,
> > > +						       &gpio_flags);
> > > +		if (gpio_is_valid(gpio_write_protect) &&
> > > +		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
> > > +			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> > >  	} else {
> > >  		const struct mvsdio_platform_data *mvsd_data;
> > >  		mvsd_data = pdev->dev.platform_data;
> > 
> > The core function mmc_of_parse() (new in 3.9) will do all of this DT
> > parsing for you, and will also take care of the driver's later calls
> > to mmc_gpio_request_{cd,wp}().  Could you try using that function
> > instead, please?
> 
> Hi Chris, Simon
> 
> I have a vague recollection that Thomas Petazzoni already did this
> conversion. Take a look in linux-next, or maybe Jason can point you at
> a branch.

Andrew,

Thomas did a board conversion here (slated for v3.11 in /pcie branch):

  cd2bde5 arm: kirkwood: convert db-88f6281 to the Device Tree

And Sebastian did some mvsdio WP and CD pin work here:

  0d0644eARM: Kirkwood: fix unused mvsdio gpio pins

Nothing else really jumps out though.  Where either of these what you
were thinking of?

thx,

Jason.
--
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
Andrew Lunn April 22, 2013, 9:45 p.m. UTC | #4
> > > The core function mmc_of_parse() (new in 3.9) will do all of this DT
> > > parsing for you, and will also take care of the driver's later calls
> > > to mmc_gpio_request_{cd,wp}().  Could you try using that function
> > > instead, please?
> > 
> > Hi Chris, Simon
> > 
> > I have a vague recollection that Thomas Petazzoni already did this
> > conversion. Take a look in linux-next, or maybe Jason can point you at
> > a branch.
> 
> Andrew,
> 
> Thomas did a board conversion here (slated for v3.11 in /pcie branch):
> 
>   cd2bde5 arm: kirkwood: convert db-88f6281 to the Device Tree
> 
> And Sebastian did some mvsdio WP and CD pin work here:
> 
>   0d0644eARM: Kirkwood: fix unused mvsdio gpio pins
> 
> Nothing else really jumps out though.  Where either of these what you
> were thinking of?

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138514.html

and

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138513.html

	Andrew
--
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 April 22, 2013, 9:50 p.m. UTC | #5
Hi,

On Mon, Apr 22 2013, Andrew Lunn wrote:
>> Nothing else really jumps out though.  Where either of these what you
>> were thinking of?
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138514.html
>
> and
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138513.html

These patches move mvsdio to slot-gpio, which is a prerequisite for
using the shared mmc_of_parse(), but they don't go all the way to
using mmc_of_parse().

(Which makes sense; mmc_of_parse() was merged to mmc-next in February,
so it wasn't available for this patchset in December.)

Thanks,

- Chris.
Jason Cooper April 22, 2013, 10:54 p.m. UTC | #6
On Mon, Apr 22, 2013 at 11:45:43PM +0200, Andrew Lunn wrote:
> > > > The core function mmc_of_parse() (new in 3.9) will do all of this DT
> > > > parsing for you, and will also take care of the driver's later calls
> > > > to mmc_gpio_request_{cd,wp}().  Could you try using that function
> > > > instead, please?
> > > 
> > > Hi Chris, Simon
> > > 
> > > I have a vague recollection that Thomas Petazzoni already did this
> > > conversion. Take a look in linux-next, or maybe Jason can point you at
> > > a branch.
> > 
> > Andrew,
> > 
> > Thomas did a board conversion here (slated for v3.11 in /pcie branch):
> > 
> >   cd2bde5 arm: kirkwood: convert db-88f6281 to the Device Tree
> > 
> > And Sebastian did some mvsdio WP and CD pin work here:
> > 
> >   0d0644eARM: Kirkwood: fix unused mvsdio gpio pins
> > 
> > Nothing else really jumps out though.  Where either of these what you
> > were thinking of?
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138514.html
> 
> and
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138513.html

Ahh, they were with the original submission for mvsdio:

  07728b7 mmc: mvsdio: use slot-gpio for card detect gpio
  3724482 mmc: mvsdio: use slot-gpio infrastructure for write protect gpio

I thought you were referring to a newer patch.

thx,

Jason.
--
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
Thomas Petazzoni April 23, 2013, 1:48 a.m. UTC | #7
Dear Chris Ball,

On Mon, 22 Apr 2013 17:50:17 -0400, Chris Ball wrote:

> On Mon, Apr 22 2013, Andrew Lunn wrote:
> >> Nothing else really jumps out though.  Where either of these what you
> >> were thinking of?
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138514.html
> >
> > and
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138513.html
> 
> These patches move mvsdio to slot-gpio, which is a prerequisite for
> using the shared mmc_of_parse(), but they don't go all the way to
> using mmc_of_parse().
> 
> (Which makes sense; mmc_of_parse() was merged to mmc-next in February,
> so it wasn't available for this patchset in December.)

Correct. I used the slot-gpio infrastructure in mvsdio, but I wasn't
aware of the new mmc_of_parse() thing, and I don't have any patch that
converts mvsdio to use it for the moment.

Best regards,

Thomas
Simon Baatz April 23, 2013, 9:22 p.m. UTC | #8
Hi Chris, Guennadi,

On Mon, Apr 22, 2013 at 05:50:17PM -0400, Chris Ball wrote:
> Hi,
> 
> On Mon, Apr 22 2013, Andrew Lunn wrote:
> >> Nothing else really jumps out though.  Where either of these what you
> >> were thinking of?
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138514.html
> >
> > and
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138513.html
> 
> These patches move mvsdio to slot-gpio, which is a prerequisite for
> using the shared mmc_of_parse(), but they don't go all the way to
> using mmc_of_parse().

That's nice functionality, I was not aware of that. I will convert this
to mmc_of_parse() once I find some time.

One question already: Why does mmc_of_parse() not return errors to
the caller?  If, for example, a GPIO line was given for CD in the DT,
but it could not be obtained, shouldn't the caller at least have the
possibility to handle this as an error?  (mvsdio currently
aborts the probe if it can't get the GPIO line).

- Simon
--
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/arch/arm/boot/dts/kirkwood-mplcec4.dts b/arch/arm/boot/dts/kirkwood-mplcec4.dts
index 7588241..bf3a58c 100644
--- a/arch/arm/boot/dts/kirkwood-mplcec4.dts
+++ b/arch/arm/boot/dts/kirkwood-mplcec4.dts
@@ -136,7 +136,7 @@ 
 			pinctrl-0 = <&pmx_sdio &pmx_sdio_cd>;
 			pinctrl-names = "default";
 			status = "okay";
-			cd-gpios = <&gpio1 15 0>;
+			cd-gpios = <&gpio1 15 1>;
 			/* No WP GPIO */
 		};
 	};
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 145cdaf..d444efd 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -691,6 +691,7 @@  static int __init mvsd_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret, irq;
 	int gpio_card_detect, gpio_write_protect;
+	enum of_gpio_flags gpio_flags;
 	struct pinctrl *pinctrl;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -731,8 +732,17 @@  static int __init mvsd_probe(struct platform_device *pdev)
 		}
 
 		host->base_clock = clk_get_rate(host->clk) / 2;
-		gpio_card_detect = of_get_named_gpio(np, "cd-gpios", 0);
-		gpio_write_protect = of_get_named_gpio(np, "wp-gpios", 0);
+		gpio_card_detect = of_get_named_gpio_flags(np, "cd-gpios", 0,
+						     &gpio_flags);
+		if (gpio_is_valid(gpio_card_detect) &&
+		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
+			mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+
+		gpio_write_protect = of_get_named_gpio_flags(np, "wp-gpios", 0,
+						       &gpio_flags);
+		if (gpio_is_valid(gpio_write_protect) &&
+		    !(gpio_flags & OF_GPIO_ACTIVE_LOW))
+			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
 	} else {
 		const struct mvsdio_platform_data *mvsd_data;
 		mvsd_data = pdev->dev.platform_data;