* [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API
@ 2015-09-16 19:16 Robert Jarzmik
2015-09-17 8:20 ` Ulf Hansson
0 siblings, 1 reply; 3+ messages in thread
From: Robert Jarzmik @ 2015-09-16 19:16 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, linux-kernel, Robert Jarzmik, Petr Cvek
Move pxamci to mmc slot-gpio API to fix interrupt request.
It fixes the case where the card detection is on a gpio expander, on I2C
for example on zylonite board. In this case, the card detect netsted
interrupt is called from a threaded interrupt. The request_irq() fails,
because a hard irq cannot be a nested interrupt from a threaded
interrupt (set __setup_irq()).
This was tested on zylonite and mioa701 boards.
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Petr Cvek <petr.cvek@tul.cz>
---
Since v1: trade threaded interrupt for slot-gpio API
---
drivers/mmc/host/pxamci.c | 58 ++++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 1420f29628c7..5b8869bc5629 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -28,6 +28,7 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/io.h>
#include <linux/regulator/consumer.h>
#include <linux/gpio.h>
@@ -454,12 +455,8 @@ static int pxamci_get_ro(struct mmc_host *mmc)
{
struct pxamci_host *host = mmc_priv(mmc);
- if (host->pdata && gpio_is_valid(host->pdata->gpio_card_ro)) {
- if (host->pdata->gpio_card_ro_invert)
- return !gpio_get_value(host->pdata->gpio_card_ro);
- else
- return gpio_get_value(host->pdata->gpio_card_ro);
- }
+ if (host->pdata && gpio_is_valid(host->pdata->gpio_card_ro))
+ return mmc_gpio_get_ro(mmc);
if (host->pdata && host->pdata->get_ro)
return !!host->pdata->get_ro(mmc_dev(mmc));
/*
@@ -551,6 +548,7 @@ static void pxamci_enable_sdio_irq(struct mmc_host *host, int enable)
static const struct mmc_host_ops pxamci_ops = {
.request = pxamci_request,
+ .get_cd = mmc_gpio_get_cd,
.get_ro = pxamci_get_ro,
.set_ios = pxamci_set_ios,
.enable_sdio_irq = pxamci_enable_sdio_irq,
@@ -790,37 +788,31 @@ static int pxamci_probe(struct platform_device *pdev)
gpio_power = host->pdata->gpio_power;
}
if (gpio_is_valid(gpio_power)) {
- ret = gpio_request(gpio_power, "mmc card power");
+ ret = devm_gpio_request(&pdev->dev, gpio_power,
+ "mmc card power");
if (ret) {
- dev_err(&pdev->dev, "Failed requesting gpio_power %d\n", gpio_power);
+ dev_err(&pdev->dev, "Failed requesting gpio_power %d\n",
+ gpio_power);
goto out;
}
gpio_direction_output(gpio_power,
host->pdata->gpio_power_invert);
}
- if (gpio_is_valid(gpio_ro)) {
- ret = gpio_request(gpio_ro, "mmc card read only");
- if (ret) {
- dev_err(&pdev->dev, "Failed requesting gpio_ro %d\n", gpio_ro);
- goto err_gpio_ro;
- }
- gpio_direction_input(gpio_ro);
+ if (gpio_is_valid(gpio_ro))
+ ret = mmc_gpio_request_ro(mmc, gpio_ro);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed requesting gpio_ro %d\n", gpio_ro);
+ goto out;
+ } else {
+ mmc->caps |= host->pdata->gpio_card_ro_invert ?
+ MMC_CAP2_RO_ACTIVE_HIGH : 0;
}
- if (gpio_is_valid(gpio_cd)) {
- ret = gpio_request(gpio_cd, "mmc card detect");
- if (ret) {
- dev_err(&pdev->dev, "Failed requesting gpio_cd %d\n", gpio_cd);
- goto err_gpio_cd;
- }
- gpio_direction_input(gpio_cd);
- ret = request_irq(gpio_to_irq(gpio_cd), pxamci_detect_irq,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
- "mmc card detect", mmc);
- if (ret) {
- dev_err(&pdev->dev, "failed to request card detect IRQ\n");
- goto err_request_irq;
- }
+ if (gpio_is_valid(gpio_cd))
+ ret = mmc_gpio_request_cd(mmc, gpio_cd, 0);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed requesting gpio_cd %d\n", gpio_cd);
+ goto out;
}
if (host->pdata && host->pdata->init)
@@ -835,13 +827,7 @@ static int pxamci_probe(struct platform_device *pdev)
return 0;
-err_request_irq:
- gpio_free(gpio_cd);
-err_gpio_cd:
- gpio_free(gpio_ro);
-err_gpio_ro:
- gpio_free(gpio_power);
- out:
+out:
if (host) {
if (host->dma_chan_rx)
dma_release_channel(host->dma_chan_rx);
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API
2015-09-16 19:16 [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API Robert Jarzmik
@ 2015-09-17 8:20 ` Ulf Hansson
2015-09-17 17:05 ` Robert Jarzmik
0 siblings, 1 reply; 3+ messages in thread
From: Ulf Hansson @ 2015-09-17 8:20 UTC (permalink / raw)
To: Robert Jarzmik; +Cc: linux-mmc, linux-kernel@vger.kernel.org, Petr Cvek
On 16 September 2015 at 21:16, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Move pxamci to mmc slot-gpio API to fix interrupt request.
>
> It fixes the case where the card detection is on a gpio expander, on I2C
> for example on zylonite board. In this case, the card detect netsted
> interrupt is called from a threaded interrupt. The request_irq() fails,
> because a hard irq cannot be a nested interrupt from a threaded
> interrupt (set __setup_irq()).
>
> This was tested on zylonite and mioa701 boards.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Petr Cvek <petr.cvek@tul.cz>
> ---
> Since v1: trade threaded interrupt for slot-gpio API
> ---
> drivers/mmc/host/pxamci.c | 58 ++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
> index 1420f29628c7..5b8869bc5629 100644
> --- a/drivers/mmc/host/pxamci.c
> +++ b/drivers/mmc/host/pxamci.c
> @@ -28,6 +28,7 @@
> #include <linux/clk.h>
> #include <linux/err.h>
> #include <linux/mmc/host.h>
> +#include <linux/mmc/slot-gpio.h>
> #include <linux/io.h>
> #include <linux/regulator/consumer.h>
> #include <linux/gpio.h>
> @@ -454,12 +455,8 @@ static int pxamci_get_ro(struct mmc_host *mmc)
> {
> struct pxamci_host *host = mmc_priv(mmc);
>
> - if (host->pdata && gpio_is_valid(host->pdata->gpio_card_ro)) {
> - if (host->pdata->gpio_card_ro_invert)
> - return !gpio_get_value(host->pdata->gpio_card_ro);
> - else
> - return gpio_get_value(host->pdata->gpio_card_ro);
> - }
> + if (host->pdata && gpio_is_valid(host->pdata->gpio_card_ro))
> + return mmc_gpio_get_ro(mmc);
> if (host->pdata && host->pdata->get_ro)
> return !!host->pdata->get_ro(mmc_dev(mmc));
> /*
> @@ -551,6 +548,7 @@ static void pxamci_enable_sdio_irq(struct mmc_host *host, int enable)
>
> static const struct mmc_host_ops pxamci_ops = {
> .request = pxamci_request,
> + .get_cd = mmc_gpio_get_cd,
> .get_ro = pxamci_get_ro,
> .set_ios = pxamci_set_ios,
> .enable_sdio_irq = pxamci_enable_sdio_irq,
> @@ -790,37 +788,31 @@ static int pxamci_probe(struct platform_device *pdev)
> gpio_power = host->pdata->gpio_power;
> }
> if (gpio_is_valid(gpio_power)) {
> - ret = gpio_request(gpio_power, "mmc card power");
> + ret = devm_gpio_request(&pdev->dev, gpio_power,
> + "mmc card power");
> if (ret) {
> - dev_err(&pdev->dev, "Failed requesting gpio_power %d\n", gpio_power);
> + dev_err(&pdev->dev, "Failed requesting gpio_power %d\n",
> + gpio_power);
> goto out;
> }
> gpio_direction_output(gpio_power,
> host->pdata->gpio_power_invert);
> }
> - if (gpio_is_valid(gpio_ro)) {
> - ret = gpio_request(gpio_ro, "mmc card read only");
> - if (ret) {
> - dev_err(&pdev->dev, "Failed requesting gpio_ro %d\n", gpio_ro);
> - goto err_gpio_ro;
> - }
> - gpio_direction_input(gpio_ro);
> + if (gpio_is_valid(gpio_ro))
> + ret = mmc_gpio_request_ro(mmc, gpio_ro);
Would it be possible for you to use the mmc_gpiod_request_ro() instead?
> + if (ret) {
> + dev_err(&pdev->dev, "Failed requesting gpio_ro %d\n", gpio_ro);
> + goto out;
> + } else {
> + mmc->caps |= host->pdata->gpio_card_ro_invert ?
> + MMC_CAP2_RO_ACTIVE_HIGH : 0;
> }
> - if (gpio_is_valid(gpio_cd)) {
> - ret = gpio_request(gpio_cd, "mmc card detect");
> - if (ret) {
> - dev_err(&pdev->dev, "Failed requesting gpio_cd %d\n", gpio_cd);
> - goto err_gpio_cd;
> - }
> - gpio_direction_input(gpio_cd);
>
> - ret = request_irq(gpio_to_irq(gpio_cd), pxamci_detect_irq,
I guess the pxamci_detect_irq() function can be removed within this
patch as well!?
> - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> - "mmc card detect", mmc);
> - if (ret) {
> - dev_err(&pdev->dev, "failed to request card detect IRQ\n");
> - goto err_request_irq;
> - }
> + if (gpio_is_valid(gpio_cd))
> + ret = mmc_gpio_request_cd(mmc, gpio_cd, 0);
Would it be possible for you to use the mmc_gpiod_request_cd() instead?
> + if (ret) {
> + dev_err(&pdev->dev, "Failed requesting gpio_cd %d\n", gpio_cd);
> + goto out;
> }
>
> if (host->pdata && host->pdata->init)
> @@ -835,13 +827,7 @@ static int pxamci_probe(struct platform_device *pdev)
>
> return 0;
>
> -err_request_irq:
> - gpio_free(gpio_cd);
> -err_gpio_cd:
> - gpio_free(gpio_ro);
> -err_gpio_ro:
> - gpio_free(gpio_power);
> - out:
> +out:
> if (host) {
> if (host->dma_chan_rx)
> dma_release_channel(host->dma_chan_rx);
> --
> 2.1.4
>
I believe you have some additional code to remove in pxamci_remove().
Some gpio_free() and free_irq() shouldn't be needed there after this
change.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API
2015-09-17 8:20 ` Ulf Hansson
@ 2015-09-17 17:05 ` Robert Jarzmik
0 siblings, 0 replies; 3+ messages in thread
From: Robert Jarzmik @ 2015-09-17 17:05 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, linux-kernel@vger.kernel.org, Petr Cvek
Ulf Hansson <ulf.hansson@linaro.org> writes:
>> + if (gpio_is_valid(gpio_ro))
>> + ret = mmc_gpio_request_ro(mmc, gpio_ro);
>
> Would it be possible for you to use the mmc_gpiod_request_ro() instead?
I don't think so.
Most of pxamci users are old platform data based machine code, which passes an
integer for the gpio. A full conversion to gpio_desc is another work.
>> - gpio_direction_input(gpio_cd);
>>
>> - ret = request_irq(gpio_to_irq(gpio_cd), pxamci_detect_irq,
>
> I guess the pxamci_detect_irq() function can be removed within this
> patch as well!?
Euh no. The reason is on this line :
host->pdata->init(&pdev->dev, pxamci_detect_irq, mmc);
Machine code is passed this callback to signal a card detection change for
esoteric cases, using this function as a IRQ handler. For example we have
trizeps4_mci_init() in arch/arm/mach-pxa/trizeps4.c.
>> + if (gpio_is_valid(gpio_cd))
>> + ret = mmc_gpio_request_cd(mmc, gpio_cd, 0);
>
> Would it be possible for you to use the mmc_gpiod_request_cd() instead?
Same reason as before I'm afraid.
> I believe you have some additional code to remove in pxamci_remove().
> Some gpio_free() and free_irq() shouldn't be needed there after this
> change.
Yes, good catch. This will be for v3.
Cheers.
--
Robert
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-17 17:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-16 19:16 [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API Robert Jarzmik
2015-09-17 8:20 ` Ulf Hansson
2015-09-17 17:05 ` Robert Jarzmik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox