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

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

[Rajendra Nayak]

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@laptop.org>
> Cc: Grant Likely<grant.likely@secretlab.ca>
> Cc: Rajendra Nayak<rnayak@ti.com>
> Signed-off-by: Tony Lindgren<tony@atomide.com>
> ---

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);
>

Re: [PATCH 0/4] Start getting rid of pdata callbacks with gpio_find_by_chip_name()

[Rajendra Nayak]

Hi Tony,

On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
> Hi all,
>
> This series adds gpio_find_by_name() that allows finding
> GPIOs on specific gpio_chips. As the GPIO numbers can be
> dynamic, it's hard to find the GPIO numbers from drivers
> using them directly.
>
> So far we've dealt with this using platform specific callbacks,
> but that is messy. This series removes the needs for these
> callbacks for omap hsmmc driver. Further callbacks can be
> removed people are OK with adding gpio_find_by_name().
>
> This series is based on the omap fixes-non-critical that's
> needed for the arch/arm/mach-omap2 parts of this series.

I tested these on my beagle/panda/omap4sdp and they seem to
work fine, also fixing the broken panda card detect (due to
missing card_detect_irq in the board file). There are still
issues however when I build twl4030-gpio as a module, which I
already commented on, and the fact that the init sequence now
works by luck :)
The other issue also is that the multiple insmod/rmmod test
suggested by Russell still fails, since the second time around
the gpio_requests in the board callback fail because they are
not freed when you do a module unload/unbind.
That would need this patch from me to add the .teardown
hooks
http://marc.info/?l=linux-omap&m=133007767831297&w=2

regards,
Rajendra

>
> Regards,
>
> Tony
>
> ---
>
> Tony Lindgren (4):
>        gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
>        mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()
>        mmc: omap_hsmmc: Use GPIO offset for external GPIO chips
>        mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect
>
>
>   arch/arm/mach-omap2/board-3430sdp.c          |   13 +-
>   arch/arm/mach-omap2/board-4430sdp.c          |   45 --------
>   arch/arm/mach-omap2/board-cm-t35.c           |    8 -
>   arch/arm/mach-omap2/board-devkit8000.c       |    7 -
>   arch/arm/mach-omap2/board-igep0020.c         |    8 -
>   arch/arm/mach-omap2/board-omap3beagle.c      |    9 +-
>   arch/arm/mach-omap2/board-omap3evm.c         |    8 -
>   arch/arm/mach-omap2/board-omap3pandora.c     |   13 +-
>   arch/arm/mach-omap2/board-omap3stalker.c     |    8 -
>   arch/arm/mach-omap2/board-omap3touchbook.c   |    7 -
>   arch/arm/mach-omap2/board-omap4panda.c       |   52 ----------
>   arch/arm/mach-omap2/board-zoom-peripherals.c |    7 -
>   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/gpio/gpiolib.c                       |   47 +++++++++
>   drivers/mfd/twl6030-irq.c                    |   33 +++---
>   drivers/mmc/host/omap_hsmmc.c                |  140 +++++++++++++++++++-------
>   include/asm-generic/gpio.h                   |    3 -
>   20 files changed, 204 insertions(+), 217 deletions(-)
>

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Tony Lindgren]

Hi,

Correcting a typo in the LKML address..

* Grant Likely <grant.likely@secretlab.ca> [120302 01:00]:
> On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > Currently there is no way for drivers to request a GPIO on a particular
> > gpio chip. This makes it hard to support multiple GPIO controllers
> > with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.
> > 
> > Make it easier for device drivers to find GPIOs on a specific gpio_chip
> > by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().
> > 
> > Note that as gpiochip_find() is already exported, we may as well
> > export gpiochip_find_by_name() too.
> 
> How is the device going to know the name of the gpio controller?  I
> don't particularly like interfaces that find devices by-names since
> I don't think device names can be relied to be stable when devices
> are instantiated from device tree data.

The gpio_chip name + gpio offset is coming from pdata until things
are converted over to use DT.

For DT, this should not be needed, this is needed for removing callbacks
in pdata so we can clean up some drivers and keep them working with
both pdata and DT until things are converted over to DT.

> > +static int match_name(struct gpio_chip *chip, void *data)
> 
> Even though this is a static, please keep the prefix to avoid
> namespace conflicts.  gpiochip_match_name()
> 
> > +{
> > +	const char *name = data;
> 
> This is unnecessary; the void* can be passed directly to sysfs_streq...
> but why is sysfs_streq being used here instead of strcmp?  This is 
> not sysfs related code.

That comes from the bus code, will take a look.
 
> > +
> > +	return sysfs_streq(name, chip->label);
> > +}
> > +
> > +/**
> > + * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
> > + * @name: name of the gpio_chip
> > + *
> > + * Similar to bus_find_device_by_name. It returns a reference to the
> > + * first gpio_chip with matching name. It ignores NULL and empty names.
> > + * If you need to do something more complex, then use gpiochip_find.
> > + */
> > +struct gpio_chip *gpiochip_find_by_name(const char *name)
> > +{
> > +	if (!name || !strcmp(name, ""))
> > +		return NULL;
> > +
> > +	return gpiochip_find((void *)name, match_name);
> 
> Nasty cast.  Can the signature for gpiochip_find be changed to accept
> a (const void *)?

I think so, this too comes from the bus code.

Regards,

Tony 

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

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [120301 21:23]:
> 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.
> 
> While one of the reasons for those callbacks was to set the GPIO
> pins in platform data, I guess the other and more important one
> was to make sure the init sequencing between twl4030-gpio and the
> mmc device depending on it was done rightly. Doesn't this patch now
> leave the sequencing to work by luck (in the absence of something
> like deferred probe being in place) like is the case of twl6030 and
> mmc init sequence on OMAP4 already?

Nope :) The difference is that  we can exit and produce a sensible
error to the user about the particular gpio_chip not being available.

Regards,

Tony

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

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [120301 22:54]:
> On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
> >+		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.

Thanks, will correct.
 
> >@@ -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.

Hmm that sounds like a separate patch that should be a fixed
before this series?

Regards,

Tony

Re: [PATCH 3/4] mmc: omap_hsmmc: Use GPIO offset for external GPIO chips

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [120301 21:31]:
> On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
> >--- a/arch/arm/mach-omap2/board-3430sdp.c
> >+++ b/arch/arm/mach-omap2/board-3430sdp.c
> >@@ -231,14 +231,16 @@ static struct omap2_hsmmc_info mmc[] = {
> >  		 * so the SIM card isn't used; else 4 bits.
> >  		 */
> >  		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
> >+		.gpiochip_cd	= "twl4030_gpio",
> >+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
> >  		.gpio_wp	= 4,
> >-		.deferred	= true,
> 
> Shouldn't this patch completely get rid of all the 'deferred'
> infrastructure that was put in place, including the
> omap_hsmmc_late_init() function, since there is no need for it
> anymore?

Yes, that was needed as a fix so unfortunately there's a little
bit going back and forth. But now we can get rid of other stuff
too in addition to deferred omap_hsmmc_late_init(), we can remove
init and cleanup callbacks for hsmmc. I'll do another patch for
that.

> >--- a/arch/arm/mach-omap2/board-omap3pandora.c
> >+++ b/arch/arm/mach-omap2/board-omap3pandora.c
> >@@ -270,19 +270,19 @@ static struct omap2_hsmmc_info omap3pandora_mmc[] = {
> >  	{
> >  		.mmc		= 1,
> >  		.caps		= MMC_CAP_4_BIT_DATA,
> >-		.gpio_cd	= -EINVAL,
> >+		.gpiochip_cd	= "twl4030_gpio",
> >+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
> >  		.gpio_wp	= 126,
> >  		.ext_clock	= 0,
> >-		.deferred	= true,
> >  	},
> >  	{
> >  		.mmc		= 2,
> >  		.caps		= MMC_CAP_4_BIT_DATA,
> >-		.gpio_cd	= -EINVAL,
> >+		.gpiochip_cd	= "twl4030_gpio",
> >+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
> 
> This one should be gpio_cd = 1,

Thanks, will correct.

Regards,

Tony 

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [120301 21:38]:
> On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
> >--- a/arch/arm/mach-omap2/board-omap4panda.c
> >+++ b/arch/arm/mach-omap2/board-omap4panda.c
> >@@ -153,8 +153,8 @@ static struct omap2_hsmmc_info mmc[] = {
> >  	{
> >  		.mmc		= 1,
> >  		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
> >-		.gpio_wp	= -EINVAL,
> >  		.gpio_cd	= -EINVAL,
> >+		.gpio_wp	= -EINVAL,
> 
> stray change.

Hmm I'd like to keep this change before this ordering gets copied
to ten other board files that we may have in testing-board for a while..
I'll add a comment to the patch description about that.

> >--- a/drivers/mmc/host/omap_hsmmc.c
> >+++ b/drivers/mmc/host/omap_hsmmc.c
> >@@ -34,6 +34,7 @@
> >  #include<linux/gpio.h>
> >  #include<linux/regulator/consumer.h>
> >  #include<linux/pm_runtime.h>
> >+#include<linux/i2c/twl.h>
> >  #include<plat/dma.h>
> >  #include<mach/hardware.h>
> >  #include<plat/board.h>
> >@@ -578,6 +579,32 @@ static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
> >  		gpio_free(host->gpio_cd);
> >  }
> >
> >+#ifdef CONFIG_TWL4030_CORE
> >+static int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
> >+{
> >+	struct omap_mmc_platform_data *pdata = host->pdata;
> >+	struct omap_mmc_slot_data *slot =&pdata->slots[0];
> >+	int irq;
> >+
> >+	if (gpio_is_valid(host->gpio_cd) || host->id)
> 
> I have a series, which I am asking Chris to pull, which completely
> gets rid of all host->id based hard-codings' in the driver.
> Isn't there a better way to do this than rely on the device instance?

Yes I was thinking about that too. I guess we need to pass some
flag in pdata for twl6030 card detect. I was thinking about using
the gpiochip_cd, but twl6030 is not gpio based card detect, so
a separate flag is better.

BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails
to detect any cards, and then fails to unload. This works on omap3
just fine. Any ideas why that would be?

Regards,

Tony

Re: [PATCH 0/4] Start getting rid of pdata callbacks with gpio_find_by_chip_name()

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [120302 00:35]:
> Hi Tony,
> 
> On Friday 02 March 2012 12:25 AM, Tony Lindgren wrote:
> >Hi all,
> >
> >This series adds gpio_find_by_name() that allows finding
> >GPIOs on specific gpio_chips. As the GPIO numbers can be
> >dynamic, it's hard to find the GPIO numbers from drivers
> >using them directly.
> >
> >So far we've dealt with this using platform specific callbacks,
> >but that is messy. This series removes the needs for these
> >callbacks for omap hsmmc driver. Further callbacks can be
> >removed people are OK with adding gpio_find_by_name().
> >
> >This series is based on the omap fixes-non-critical that's
> >needed for the arch/arm/mach-omap2 parts of this series.
> 
> I tested these on my beagle/panda/omap4sdp and they seem to
> work fine, also fixing the broken panda card detect (due to
> missing card_detect_irq in the board file). There are still
> issues however when I build twl4030-gpio as a module, which I
> already commented on, and the fact that the init sequence now
> works by luck :)

Hmm it should not be luck based, loading omap_hsmmc module
should fail with a sensible error if the configured card detect
or write protect is not available.

I guess this is with twl6030 non-gpio based card detect?

If so, I'll add something to pass the twl6030 card detect from
pdata so we can fail with a sensible error in that case too.

Also, sounds like twl as module and mmc built in case won't
work without deferred probe. But at least there is a sensible
error for that. And maybe we can prevent that in Kconfig.

> The other issue also is that the multiple insmod/rmmod test
> suggested by Russell still fails, since the second time around
> the gpio_requests in the board callback fail because they are
> not freed when you do a module unload/unbind.
> That would need this patch from me to add the .teardown
> hooks
> http://marc.info/?l=linux-omap&m=133007767831297&w=2

Yes let's add the teardown patch as a fix for now, but let's
plan on getting rid of the twl_setup callback function
completely.

With these patches LCD and WLAN too can request the twl gpios
directly from the driver based on gpio_chip + gpio offset.

Regards,

Tony

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [120302 08:31]:
> * Grant Likely <grant.likely@secretlab.ca> [120302 01:00]:
> > On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > > +
> > > +/**
> > > + * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
> > > + * @name: name of the gpio_chip
> > > + *
> > > + * Similar to bus_find_device_by_name. It returns a reference to the
> > > + * first gpio_chip with matching name. It ignores NULL and empty names.
> > > + * If you need to do something more complex, then use gpiochip_find.
> > > + */
> > > +struct gpio_chip *gpiochip_find_by_name(const char *name)
> > > +{
> > > +	if (!name || !strcmp(name, ""))
> > > +		return NULL;
> > > +
> > > +	return gpiochip_find((void *)name, match_name);
> > 
> > Nasty cast.  Can the signature for gpiochip_find be changed to accept
> > a (const void *)?
> 
> I think so, this too comes from the bus code.

Looks like it can't be const as of_node_to_gpiochip uses it with
a struct device_node * that for of_get_named_gpio_flags comes from
of_parse_phandle_with_args.

Regards,

Tony

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

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [120302 08:38]:
> * Rajendra Nayak <rnayak@ti.com> [120301 22:54]:
>
> > >@@ -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.
> 
> Hmm that sounds like a separate patch that should be a fixed
> before this series?

Nope, I was confused. The error handling needs to change
as the omap_hsmmc_gpio_init() got moved later so host is
initialized. Will fix.

Regards,

Tony

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Grant Likely]

On Fri, 2 Mar 2012 10:08:48 -0800, Tony Lindgren <tony@atomide.com> wrote:
> * Tony Lindgren <tony@atomide.com> [120302 08:31]:
> > * Grant Likely <grant.likely@secretlab.ca> [120302 01:00]:
> > > On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > > > +
> > > > +/**
> > > > + * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
> > > > + * @name: name of the gpio_chip
> > > > + *
> > > > + * Similar to bus_find_device_by_name. It returns a reference to the
> > > > + * first gpio_chip with matching name. It ignores NULL and empty names.
> > > > + * If you need to do something more complex, then use gpiochip_find.
> > > > + */
> > > > +struct gpio_chip *gpiochip_find_by_name(const char *name)
> > > > +{
> > > > +	if (!name || !strcmp(name, ""))
> > > > +		return NULL;
> > > > +
> > > > +	return gpiochip_find((void *)name, match_name);
> > > 
> > > Nasty cast.  Can the signature for gpiochip_find be changed to accept
> > > a (const void *)?
> > 
> > I think so, this too comes from the bus code.
> 
> Looks like it can't be const as of_node_to_gpiochip uses it with
> a struct device_node * that for of_get_named_gpio_flags comes from
> of_parse_phandle_with_args.

Really? It sees to work fine here:

---

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d773540..e633a2a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
  * non-zero, this function will return to the caller and not iterate over any
  * more gpio_chips.
  */
-struct gpio_chip *gpiochip_find(void *data,
-				int (*match)(struct gpio_chip *chip, void *data))
+struct gpio_chip *gpiochip_find(const void *data,
+				int (*match)(struct gpio_chip *chip,
+					     const void *data))
 {
 	struct gpio_chip *chip = NULL;
 	unsigned long flags;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index e034b38..bba8121 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
 }
 
 /* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
 {
 	return chip->of_node == data;
 }
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 1ff4e22..5f52690 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
 /* add/remove chips */
 extern int gpiochip_add(struct gpio_chip *chip);
 extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
-						     void *data));
+						     const void *data));
 
 
 /* Always use the library code for GPIO management calls,

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Tony Lindgren]

* Grant Likely <grant.likely@secretlab.ca> [120302 10:16]:
> On Fri, 2 Mar 2012 10:08:48 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > * Tony Lindgren <tony@atomide.com> [120302 08:31]:
> > > * Grant Likely <grant.likely@secretlab.ca> [120302 01:00]:
> > > > On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > > > > +
> > > > > +/**
> > > > > + * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
> > > > > + * @name: name of the gpio_chip
> > > > > + *
> > > > > + * Similar to bus_find_device_by_name. It returns a reference to the
> > > > > + * first gpio_chip with matching name. It ignores NULL and empty names.
> > > > > + * If you need to do something more complex, then use gpiochip_find.
> > > > > + */
> > > > > +struct gpio_chip *gpiochip_find_by_name(const char *name)
> > > > > +{
> > > > > +	if (!name || !strcmp(name, ""))
> > > > > +		return NULL;
> > > > > +
> > > > > +	return gpiochip_find((void *)name, match_name);
> > > > 
> > > > Nasty cast.  Can the signature for gpiochip_find be changed to accept
> > > > a (const void *)?
> > > 
> > > I think so, this too comes from the bus code.
> > 
> > Looks like it can't be const as of_node_to_gpiochip uses it with
> > a struct device_node * that for of_get_named_gpio_flags comes from
> > of_parse_phandle_with_args.
> 
> Really? It sees to work fine here:

Hmm right you are. I must have missed adding the const to
of_gpiochip_is_match, that's good news :)

Want to do that as a separate patch or want me to fold it in?

Tony
 
> ---
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d773540..e633a2a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
>   * non-zero, this function will return to the caller and not iterate over any
>   * more gpio_chips.
>   */
> -struct gpio_chip *gpiochip_find(void *data,
> -				int (*match)(struct gpio_chip *chip, void *data))
> +struct gpio_chip *gpiochip_find(const void *data,
> +				int (*match)(struct gpio_chip *chip,
> +					     const void *data))
>  {
>  	struct gpio_chip *chip = NULL;
>  	unsigned long flags;
> diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
> index e034b38..bba8121 100644
> --- a/drivers/of/gpio.c
> +++ b/drivers/of/gpio.c
> @@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
>  }
>  
>  /* Private function for resolving node pointer to gpio_chip */
> -static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
> +static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
>  {
>  	return chip->of_node == data;
>  }
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 1ff4e22..5f52690 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
>  /* add/remove chips */
>  extern int gpiochip_add(struct gpio_chip *chip);
>  extern int __must_check gpiochip_remove(struct gpio_chip *chip);
> -extern struct gpio_chip *gpiochip_find(void *data,
> +extern struct gpio_chip *gpiochip_find(const void *data,
>  					int (*match)(struct gpio_chip *chip,
> -						     void *data));
> +						     const void *data));
>  
>  
>  /* Always use the library code for GPIO management calls,
> 

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[Rajendra Nayak]

On Friday 02 March 2012 10:52 PM, Tony Lindgren wrote:
> BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails
> to detect any cards, and then fails to unload. This works on omap3
> just fine. Any ideas why that would be?

Yeah, looks like thats broken. I am not sure whats going wrong though.
I just enabled CONFIG_MMC_DEBUG and saw these logs below.

Venkat/Balaji, care to look at this one?

# insmod omap_hsmmc.ko
[   43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
[   43.364105] omap_hsmmc omap_hsmmc.0: enabled
[   44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0 
timing 0
[   44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[   44.474365] omap_hsmmc omap_hsmmc.0: disabled
[   44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
[   44.482208] omap_hsmmc omap_hsmmc.4: enabled
[   44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
[   44.552276] omap_hsmmc omap_hsmmc.0: enabled
[   44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[   44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0 
timing 0
[   44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[   44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 
width 0 timing 0
[   44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
[   44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
[   44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
[   45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0 
timing 0
[   45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
[   45.591491] omap_hsmmc omap_hsmmc.4: disabled
#
#
# rmmod omap_hsmmc
[  607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
[  607.308044] omap_hsmmc omap_hsmmc.4: enabled
[  607.312591] omap_hsmmc omap_hsmmc.4: disabled
[  607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
[  607.322875] omap_hsmmc omap_hsmmc.4: enabled

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[T Krishnamoorthy, Balaji]

On Mon, Mar 5, 2012 at 2:46 PM, Rajendra Nayak <rnayak@ti.com> wrote:
> On Friday 02 March 2012 10:52 PM, Tony Lindgren wrote:
>>
>> BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails
>> to detect any cards, and then fails to unload. This works on omap3
>> just fine. Any ideas why that would be?
>
>
> Yeah, looks like thats broken. I am not sure whats going wrong though.
> I just enabled CONFIG_MMC_DEBUG and saw these logs below.
>
> Venkat/Balaji, care to look at this one?
>

Let me check this

> # insmod omap_hsmmc.ko
> [   43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
> [   43.364105] omap_hsmmc omap_hsmmc.0: enabled
> [   44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
> timing 0
> [   44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
> [   44.474365] omap_hsmmc omap_hsmmc.0: disabled
> [   44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
> [   44.482208] omap_hsmmc omap_hsmmc.4: enabled
> [   44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
> [   44.552276] omap_hsmmc omap_hsmmc.0: enabled
> [   44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
> [   44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0
> timing 0
> [   44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
> [   44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 width
> 0 timing 0
> [   44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
> [   44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
> [   44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
> [   45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
> timing 0
> [   45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
> [   45.591491] omap_hsmmc omap_hsmmc.4: disabled
> #
> #
> # rmmod omap_hsmmc
> [  607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
> [  607.308044] omap_hsmmc omap_hsmmc.4: enabled
> [  607.312591] omap_hsmmc omap_hsmmc.4: disabled
> [  607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
> [  607.322875] omap_hsmmc omap_hsmmc.4: enabled
>

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[T Krishnamoorthy, Balaji]

On Mon, Mar 5, 2012 at 3:55 PM, T Krishnamoorthy, Balaji
<balajitk@ti.com> wrote:
> On Mon, Mar 5, 2012 at 2:46 PM, Rajendra Nayak <rnayak@ti.com> wrote:
>> On Friday 02 March 2012 10:52 PM, Tony Lindgren wrote:
>>>
>>> BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails
>>> to detect any cards, and then fails to unload. This works on omap3
>>> just fine. Any ideas why that would be?
>>
>>
>> Yeah, looks like thats broken. I am not sure whats going wrong though.
>> I just enabled CONFIG_MMC_DEBUG and saw these logs below.
>>
>> Venkat/Balaji, care to look at this one?
>>
>
> Let me check this

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
Addng the offset to platform_device resource structure
increments the start address for every insmod operation.
MMC command fails on re-insertion as module due incorrect register base.
Fix this by updating the ioremap base address only.

Signed-off-by: Balaji T K <balajitk@ti.com>
---
Note:  eMMC detection is still failing on resertion due to card vcc
power off on rmmod

 drivers/mmc/host/omap_hsmmc.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index fd0c661..4e1f8f6 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1870,8 +1870,6 @@ static int __init omap_hsmmc_probe(struct
platform_device *pdev)
 	if (res == NULL || irq < 0)
 		return -ENXIO;

-	res->start += pdata->reg_offset;
-	res->end += pdata->reg_offset;
 	res = request_mem_region(res->start, resource_size(res), pdev->name);
 	if (res == NULL)
 		return -EBUSY;
@@ -1896,7 +1894,7 @@ static int __init omap_hsmmc_probe(struct
platform_device *pdev)
 	host->irq	= irq;
 	host->id	= pdev->id;
 	host->slot_id	= 0;
-	host->mapbase	= res->start;
+	host->mapbase	= res->start + pdata->reg_offset;
 	host->base	= ioremap(host->mapbase, SZ_4K);
 	host->power_mode = MMC_POWER_OFF;
 	host->next_data.cookie = 1;
-- 
1.7.0.4

>
>> # insmod omap_hsmmc.ko
>> [   43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
>> [   43.364105] omap_hsmmc omap_hsmmc.0: enabled
>> [   44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
>> timing 0
>> [   44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
>> [   44.474365] omap_hsmmc omap_hsmmc.0: disabled
>> [   44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
>> [   44.482208] omap_hsmmc omap_hsmmc.4: enabled
>> [   44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
>> [   44.552276] omap_hsmmc omap_hsmmc.0: enabled
>> [   44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
>> [   44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0
>> timing 0
>> [   44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
>> [   44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 width
>> 0 timing 0
>> [   44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
>> [   44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
>> [   44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
>> [   45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
>> timing 0
>> [   45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
>> [   45.591491] omap_hsmmc omap_hsmmc.4: disabled
>> #
>> #
>> # rmmod omap_hsmmc
>> [  607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
>> [  607.308044] omap_hsmmc omap_hsmmc.4: enabled
>> [  607.312591] omap_hsmmc omap_hsmmc.4: disabled
>> [  607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
>> [  607.322875] omap_hsmmc omap_hsmmc.4: enabled
>>



-- 
Thanks and Regards,
Balaji T K

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[Chris Ball]

Hi Balaji,

On Wed, Mar 07 2012, T Krishnamoorthy, Balaji wrote:
> OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
> Addng the offset to platform_device resource structure
> increments the start address for every insmod operation.
> MMC command fails on re-insertion as module due incorrect register base.
> Fix this by updating the ioremap base address only.
>
> Signed-off-by: Balaji T K <balajitk@ti.com>

Is this a regression, or has it never worked in mainline?

> Note:  eMMC detection is still failing on resertion due to card vcc
> power off on rmmod

And this?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[Tony Lindgren]

* Chris Ball <cjb@laptop.org> [120307 07:11]:
> Hi Balaji,
> 
> On Wed, Mar 07 2012, T Krishnamoorthy, Balaji wrote:
> > OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
> > Addng the offset to platform_device resource structure
> > increments the start address for every insmod operation.
> > MMC command fails on re-insertion as module due incorrect register base.
> > Fix this by updating the ioremap base address only.
> >
> > Signed-off-by: Balaji T K <balajitk@ti.com>
> 
> Is this a regression, or has it never worked in mainline?

"Features that never worked originally" so this can wait for the
merge window. It happens on re-inserting of the card.
 
> > Note:  eMMC detection is still failing on resertion due to card vcc
> > power off on rmmod
> 
> And this?

That sounds like a separate issue that needs to be fixed.

Regards,

Tony

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

[T Krishnamoorthy, Balaji]

On Wed, Mar 7, 2012 at 9:12 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi Balaji,
>
> On Wed, Mar 07 2012, T Krishnamoorthy, Balaji wrote:
>> OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
>> Addng the offset to platform_device resource structure
>> increments the start address for every insmod operation.
>> MMC command fails on re-insertion as module due incorrect register base.
>> Fix this by updating the ioremap base address only.
>>
>> Signed-off-by: Balaji T K <balajitk@ti.com>
>
> Is this a regression, or has it never worked in mainline?

Not a regression introduced in current merge window.
It happens on re-insertion of module.
will post a patch with $SUBJECT

>
>> Note:  eMMC detection is still failing on resertion due to card vcc
>> power off on rmmod
>
> And this?

This issue was hidden and uncovered after this fix.
This problem is Vcc being powered off without sleep command.

>
> Thanks,
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Grant Likely]

On Fri, 2 Mar 2012 11:06:15 -0800, Tony Lindgren <tony@atomide.com> wrote:
> * Grant Likely <grant.likely@secretlab.ca> [120302 10:16]:
> > On Fri, 2 Mar 2012 10:08:48 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > > * Tony Lindgren <tony@atomide.com> [120302 08:31]:
> > > > * Grant Likely <grant.likely@secretlab.ca> [120302 01:00]:
> > > > > On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > > > > > +
> > > > > > +/**
> > > > > > + * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
> > > > > > + * @name: name of the gpio_chip
> > > > > > + *
> > > > > > + * Similar to bus_find_device_by_name. It returns a reference to the
> > > > > > + * first gpio_chip with matching name. It ignores NULL and empty names.
> > > > > > + * If you need to do something more complex, then use gpiochip_find.
> > > > > > + */
> > > > > > +struct gpio_chip *gpiochip_find_by_name(const char *name)
> > > > > > +{
> > > > > > +	if (!name || !strcmp(name, ""))
> > > > > > +		return NULL;
> > > > > > +
> > > > > > +	return gpiochip_find((void *)name, match_name);
> > > > > 
> > > > > Nasty cast.  Can the signature for gpiochip_find be changed to accept
> > > > > a (const void *)?
> > > > 
> > > > I think so, this too comes from the bus code.
> > > 
> > > Looks like it can't be const as of_node_to_gpiochip uses it with
> > > a struct device_node * that for of_get_named_gpio_flags comes from
> > > of_parse_phandle_with_args.
> > 
> > Really? It sees to work fine here:
> 
> Hmm right you are. I must have missed adding the const to
> of_gpiochip_is_match, that's good news :)
> 
> Want to do that as a separate patch or want me to fold it in?

I've got it as a separate commit in my gpio/next branch.

g.

> > ---
> > 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index d773540..e633a2a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
> >   * non-zero, this function will return to the caller and not iterate over any
> >   * more gpio_chips.
> >   */
> > -struct gpio_chip *gpiochip_find(void *data,
> > -				int (*match)(struct gpio_chip *chip, void *data))
> > +struct gpio_chip *gpiochip_find(const void *data,
> > +				int (*match)(struct gpio_chip *chip,
> > +					     const void *data))
> >  {
> >  	struct gpio_chip *chip = NULL;
> >  	unsigned long flags;
> > diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
> > index e034b38..bba8121 100644
> > --- a/drivers/of/gpio.c
> > +++ b/drivers/of/gpio.c
> > @@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
> >  }
> >  
> >  /* Private function for resolving node pointer to gpio_chip */
> > -static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
> > +static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
> >  {
> >  	return chip->of_node == data;
> >  }
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index 1ff4e22..5f52690 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
> >  /* add/remove chips */
> >  extern int gpiochip_add(struct gpio_chip *chip);
> >  extern int __must_check gpiochip_remove(struct gpio_chip *chip);
> > -extern struct gpio_chip *gpiochip_find(void *data,
> > +extern struct gpio_chip *gpiochip_find(const void *data,
> >  					int (*match)(struct gpio_chip *chip,
> > -						     void *data));
> > +						     const void *data));
> >  
> >  
> >  /* Always use the library code for GPIO management calls,
> > 

-- 
email sent from notmuch.vim plugin

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

[Tony Lindgren]

* Grant Likely <grant.likely@secretlab.ca> [120308 17:08]:
> On Fri, 2 Mar 2012 11:06:15 -0800, Tony Lindgren <tony@atomide.com> wrote:
> > 
> > Want to do that as a separate patch or want me to fold it in?
> 
> I've got it as a separate commit in my gpio/next branch.

OK great, below is my patch updated against gpio/next if you
want to pick it up.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Fri, 2 Mar 2012 11:30:23 -0800
Subject: [PATCH] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Currently there is no way for drivers to request a GPIO on a particular
gpio chip using platform data. This makes it hard to support multiple
GPIO controllers with platform driver with dynamic GPIO and interrupt
numbering, such as with CONFIG_SPARSE_IRQ.

Let's make it easier for platform device drivers to find GPIOs on a
specific gpio_chip by adding two functions: gpiochip_find_by_name()
and gpio_find_by_chip_name().

Note that these functions should not be used with device tree as
pointed out by Grant Likely <grant.likely@secretlab.ca> as the names
are instantiated from device tree.

As gpiochip_find() is already exported, we may as well export
gpiochip_find_by_name() too.

Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.walleij@stericsson.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1178,6 +1178,51 @@ struct gpio_chip *gpiochip_find(const void *data,
 }
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
+static int gpiochip_match_name(struct gpio_chip *chip, const void *name)
+{
+	return !strcmp(name, chip->label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find(name, gpiochip_match_name);
+}
+EXPORT_SYMBOL_GPL(gpiochip_find_by_name);
+
+/**
+ * gpio_find_by_chip_name() - find a gpio on a specific gpio_chip
+ * @chip_name: name of the gpio_chip
+ * @gpio_offset: offset of the gpio on the gpio_chip
+ *
+ * Note that gpiochip_find_by_name returns the first matching
+ * gpio_chip name. For more complex matching, see gpio_find.
+ */
+int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset)
+{
+	struct gpio_chip *chip;
+	int gpio;
+
+	chip = gpiochip_find_by_name(chip_name);
+	if (!chip)
+		return -ENODEV;
+
+	gpio = chip->base + gpio_offset;
+
+	return gpio;
+}
+EXPORT_SYMBOL_GPL(gpio_find_by_chip_name);
+
 /* These "optional" allocation calls help prevent drivers from stomping
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -145,7 +145,8 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
 extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
 						     const void *data));
-
+extern struct gpio_chip *gpiochip_find_by_name(const char *name);
+extern int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset);
 
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.