| Message ID | 1366660500-26835-2-git-send-email-gmbnomis@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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.
> > > 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
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.
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.
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
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
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;
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(-)