Hi Shawn
On Fri, 7 Dec 2012, Shawn Guo wrote:
> Use devm_* managed functions, so that slot-gpio users do not have to
> call mmc_gpio_free_ro/cd to free up resources requested in
> mmc_gpio_request_ro/cd.
>
> Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx>
Thanks for the patch, but I'm not sure this is a good idea. Firstly, using
devm_* allocation functions means, that normally you don't have to free
these resources explicitly any more. So, actually you would have to remove
free_irq() and gpio_free() calls completely from the API instead of
replacing them with devm_* analogs. Secondly, I do use devm_kzalloc() in
the API because I really want that allocated memory to just be allocated
once and stay there until the device is freed. Whereas with with card
detection and write-protect pins - usually you're right, you would only
allocate those once during the lifetime of your host device. But - are we
sure there cannot be exceptions? What if someone decides to switch between
CD-IRQ and CD-polling at runtime? Or do something equally weird with their
GPIOs... At least I personally don't think I have sufficient knowledge of
all possible configurations to make such a decision. Think about pinctrl
etc. What if at runtime the CD pin has to be released by mmc to be used
by some other device? Anyway, I'm fine either way, but I'm just not sure
how reasonable this change is.
Thanks
Guennadi
> ---
> drivers/mmc/core/slot-gpio.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 16a1c0b..8b55748 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -106,7 +106,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
>
> ctx = host->slot.handler_priv;
>
> - ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->ro_label);
> + ret = devm_gpio_request_one(&host->class_dev, gpio, GPIOF_DIR_IN,
> + ctx->ro_label);
> if (ret < 0)
> return ret;
>
> @@ -128,7 +129,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
>
> ctx = host->slot.handler_priv;
>
> - ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->cd_label);
> + ret = devm_gpio_request_one(&host->class_dev, gpio, GPIOF_DIR_IN,
> + ctx->cd_label);
> if (ret < 0)
> /*
> * don't bother freeing memory. It might still get used by other
> @@ -146,7 +148,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
> irq = -EINVAL;
>
> if (irq >= 0) {
> - ret = request_threaded_irq(irq, NULL, mmc_gpio_cd_irqt,
> + ret = devm_request_threaded_irq(&host->class_dev, irq,
> + NULL, mmc_gpio_cd_irqt,
> IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> ctx->cd_label, host);
> if (ret < 0)
> @@ -175,7 +178,7 @@ void mmc_gpio_free_ro(struct mmc_host *host)
> gpio = ctx->ro_gpio;
> ctx->ro_gpio = -EINVAL;
>
> - gpio_free(gpio);
> + devm_gpio_free(&host->class_dev, gpio);
> }
> EXPORT_SYMBOL(mmc_gpio_free_ro);
>
> @@ -188,13 +191,13 @@ void mmc_gpio_free_cd(struct mmc_host *host)
> return;
>
> if (host->slot.cd_irq >= 0) {
> - free_irq(host->slot.cd_irq, host);
> + devm_free_irq(&host->class_dev, host->slot.cd_irq, host);
> host->slot.cd_irq = -EINVAL;
> }
>
> gpio = ctx->cd_gpio;
> ctx->cd_gpio = -EINVAL;
>
> - gpio_free(gpio);
> + devm_gpio_free(&host->class_dev, gpio);
> }
> EXPORT_SYMBOL(mmc_gpio_free_cd);
> --
> 1.7.9.5
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
