Re: [PATCH 2/4] mmc: omap_hsmmc: Use gpio_find_by_chip_name() foromap_hsmmc_gpio_init()

From: Rajendra Nayak
Date: Fri Mar 02 2012 - 02:25:46 EST


On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
Use gpio_find_by_chip_name() to find the GPIO pins as they can
be dynamically allocated on various gpio_chips.

Note that we don't want to touch the platform data as it can
now specify the GPIO offset on a named gpio_chip.

This removes the need to use callbacks to set the GPIO pins
in platform data.

Cc: Chris Ball<cjb@xxxxxxxxxx>
Cc: Grant Likely<grant.likely@xxxxxxxxxxxx>
Cc: Rajendra Nayak<rnayak@xxxxxx>
Signed-off-by: Tony Lindgren<tony@xxxxxxxxxxx>
---

some more comments based on my testing with twl4030-gpio
built as a module..

arch/arm/mach-omap2/hsmmc.c | 3 +
arch/arm/mach-omap2/hsmmc.h | 5 ++
arch/arm/plat-omap/include/plat/mmc.h | 3 +
drivers/gpio/gpio-twl4030.c | 2 +
drivers/mmc/host/omap_hsmmc.c | 109 +++++++++++++++++++++------------
5 files changed, 82 insertions(+), 40 deletions(-)

diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
index a97876d..dda88f7 100644
--- a/arch/arm/mach-omap2/hsmmc.c
+++ b/arch/arm/mach-omap2/hsmmc.c
@@ -323,7 +323,10 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,

@@ -497,55 +502,80 @@ static inline int omap_hsmmc_have_reg(void)

#endif

-static int omap_hsmmc_gpio_init(struct omap_mmc_platform_data *pdata)
+static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host)
{
- int ret;
-
- if (gpio_is_valid(pdata->slots[0].switch_pin)) {
- if (pdata->slots[0].cover)
- pdata->slots[0].get_cover_state =
+ struct omap_mmc_platform_data *pdata = host->pdata;
+ struct omap_mmc_slot_data *slot =&pdata->slots[0];
+ int gpio, ret;
+
+ gpio = slot->switch_pin;
+ if (slot->gpiochip_cd)
+ gpio = gpio_find_by_chip_name(slot->gpiochip_cd, gpio);
+ if (gpio_is_valid(gpio)) {
+ if (slot->cover)
+ slot->get_cover_state =
omap_hsmmc_get_cover_state;
else
- pdata->slots[0].card_detect = omap_hsmmc_card_detect;
- pdata->slots[0].card_detect_irq =
- gpio_to_irq(pdata->slots[0].switch_pin);
- ret = gpio_request(pdata->slots[0].switch_pin, "mmc_cd");
+ slot->card_detect = omap_hsmmc_card_detect;
+ slot->card_detect_irq =
+ gpio_to_irq(gpio);
+ ret = gpio_request(gpio, "mmc_cd");
if (ret)
return ret;
- ret = gpio_direction_input(pdata->slots[0].switch_pin);
+ ret = gpio_direction_input(gpio);
if (ret)
goto err_free_sp;
- } else
- pdata->slots[0].switch_pin = -EINVAL;
+ host->gpio_cd = gpio;
+ } else {
+ if (slot->gpiochip_cd) {
+ pr_warning("MMC %s card detect GPIO chip %s unavailable\n",
+ slot->name, slot->gpiochip_cd);
+ ret = -ENODEV;
+ goto err_free_sp;

This should just return -ENODEV, nothing really to free here.

+ }
+ host->gpio_cd = -EINVAL;
+ }

- if (gpio_is_valid(pdata->slots[0].gpio_wp)) {
- pdata->slots[0].get_ro = omap_hsmmc_get_wp;
- ret = gpio_request(pdata->slots[0].gpio_wp, "mmc_wp");
+ gpio = slot->gpio_wp;
+ if (slot->gpiochip_wp)
+ gpio = gpio_find_by_chip_name(slot->gpiochip_wp, gpio);
+ if (gpio_is_valid(gpio)) {
+ slot->get_ro = omap_hsmmc_get_wp;
+ ret = gpio_request(gpio, "mmc_wp");
if (ret)
goto err_free_cd;
- ret = gpio_direction_input(pdata->slots[0].gpio_wp);
+ ret = gpio_direction_input(gpio);
if (ret)
goto err_free_wp;
- } else
- pdata->slots[0].gpio_wp = -EINVAL;
+ host->gpio_wp = gpio;
+ } else {
+ if (slot->gpiochip_wp) {
+ pr_warning("MMC %s write protect GPIO chip %s unavailable\n",
+ slot->name, slot->gpiochip_wp);
+ ret = -ENODEV;
+ goto err_free_wp;
+ }
+ host->gpio_wp = -EINVAL;
+ }

return 0;

err_free_wp:
- gpio_free(pdata->slots[0].gpio_wp);
+ if (gpio_is_valid(host->gpio_wp))
+ gpio_free(host->gpio_wp);
err_free_cd:
- if (gpio_is_valid(pdata->slots[0].switch_pin))
+ if (gpio_is_valid(host->gpio_cd))
err_free_sp:
- gpio_free(pdata->slots[0].switch_pin);
+ gpio_free(host->gpio_cd);
return ret;
}

-static void omap_hsmmc_gpio_free(struct omap_mmc_platform_data *pdata)
+static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
{
- if (gpio_is_valid(pdata->slots[0].gpio_wp))
- gpio_free(pdata->slots[0].gpio_wp);
- if (gpio_is_valid(pdata->slots[0].switch_pin))
- gpio_free(pdata->slots[0].switch_pin);
+ if (gpio_is_valid(host->gpio_wp))
+ gpio_free(host->gpio_wp);
+ if (gpio_is_valid(host->gpio_cd))
+ gpio_free(host->gpio_cd);
}

/*
@@ -1876,10 +1906,6 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
if (res == NULL)
return -EBUSY;

- ret = omap_hsmmc_gpio_init(pdata);
- if (ret)
- goto err;
-
mmc = mmc_alloc_host(sizeof(struct omap_hsmmc_host),&pdev->dev);
if (!mmc) {
ret = -ENOMEM;
@@ -1903,6 +1929,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, host);

+ ret = omap_hsmmc_gpio_init(host);
+ if (ret)
+ goto err1;
+
mmc->ops =&omap_hsmmc_ops;

/*
@@ -2093,8 +2123,7 @@ err1:
platform_set_drvdata(pdev, NULL);
mmc_free_host(mmc);
err_alloc:
- omap_hsmmc_gpio_free(pdata);
-err:
+ omap_hsmmc_gpio_free(host);

This error handling needs to be fixed up. In case
omap_hsmmc_gpio_init() fails, which already frees up
any requested gpios, omap_hsmmc_gpio_free() again tries
freeing gpios.

regards,
Rajendra

release_mem_region(res->start, resource_size(res));
return ret;
}
@@ -2125,7 +2154,7 @@ static int omap_hsmmc_remove(struct platform_device *pdev)

mmc_free_host(host->mmc);
iounmap(host->base);
- omap_hsmmc_gpio_free(pdev->dev.platform_data);
+ omap_hsmmc_gpio_free(host);
}

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


--
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/