Re: [PATCH v2] mmc: pxamci: fix card detect with slot-gpio API

[Ulf Hansson]

On 16 September 2015 at 21:16, Robert Jarzmik <robert.jarzmik@xxxxxxx> 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@xxxxxxx>
> Cc: Petr Cvek <petr.cvek@xxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/