Subject: [PATCH v2 6/7] ARM: OMAP3: Use .teardown of twl4030-gpio to clean board requests

From: [email-masked] (Igor Grinberg)
Date: Thu, 23 Feb 2012 16:55:35 +0200

Hi Rajendra,

On 02/23/12 13:40, Rajendra Nayak wrote:
> All OMAP3 boards which register a .setup function with twl4030
> gpio driver do not seem to have a .teardown hook implemented.
> .setup mainly requests a few gpios and also in most cases
> does a omap_hsmmc_deferred_add(). Have a .teardown do a gpio_free()
> and of the requested gpios and also do a omap_hsmmc_deferred_del().
> This helps in case the twl4030 gpio driver is built as a module and
> added and removed multiple times. Without the .teardown a multiple
> insmod/rmmod can result in gpio request failues and also WARN messages
> stating addition of already registered mmc devices.
> 
> Reported-by: Russell King <[email-masked]>
> Signed-off-by: Rajendra Nayak <[email-masked]>
> ---
>  arch/arm/mach-omap2/board-3430sdp.c          |   10 ++++++++++
>  arch/arm/mach-omap2/board-cm-t35.c           |    8 ++++++++
>  arch/arm/mach-omap2/board-devkit8000.c       |   10 ++++++++++
>  arch/arm/mach-omap2/board-igep0020.c         |   14 ++++++++++++++
>  arch/arm/mach-omap2/board-ldp.c              |    9 +++++++++
>  arch/arm/mach-omap2/board-omap3beagle.c      |   12 ++++++++++++
>  arch/arm/mach-omap2/board-omap3evm.c         |   10 ++++++++++
>  arch/arm/mach-omap2/board-omap3pandora.c     |    9 +++++++++
>  arch/arm/mach-omap2/board-omap3stalker.c     |   11 +++++++++++
>  arch/arm/mach-omap2/board-omap3touchbook.c   |   10 ++++++++++
>  arch/arm/mach-omap2/board-overo.c            |    8 ++++++++
>  arch/arm/mach-omap2/board-rx51-peripherals.c |    8 ++++++++
>  arch/arm/mach-omap2/board-zoom-peripherals.c |    9 +++++++++
>  arch/arm/mach-omap2/hsmmc.c                  |   11 +++++++++++
>  arch/arm/mach-omap2/hsmmc.h                  |    4 ++++
>  15 files changed, 143 insertions(+), 0 deletions(-)

[...]

> diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
> index 14df109..55e0180 100644
> --- a/arch/arm/mach-omap2/board-cm-t35.c
> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> @@ -475,11 +475,19 @@ static int cm_t35_twl_gpio_setup(struct device *dev, unsigned gpio,
>  	return 0;
>  }
>  
> +static int cm_t35_twl_gpio_teardown(struct device *dev, unsigned gpio,
> +				 unsigned ngpio)
> +{
> +	omap_hsmmc_deferred_del(mmc);
> +	gpio_free(gpio + 2);
> +	return 0;
> +}

Can we have an empty line here?

>  static struct twl4030_gpio_platform_data cm_t35_gpio_data = {
>  	.gpio_base	= OMAP_MAX_GPIO_LINES,
>  	.irq_base	= TWL4030_GPIO_IRQ_BASE,
>  	.irq_end	= TWL4030_GPIO_IRQ_END,
>  	.setup          = cm_t35_twl_gpio_setup,
> +	.teardown	= cm_t35_twl_gpio_teardown,
>  };
>  
>  static struct twl4030_platform_data cm_t35_twldata = {

[...]

> diff --git a/arch/arm/mach-omap2/board-igep0020.c b/arch/arm/mach-omap2/board-igep0020.c
> index 11a6aa4..1d0850f 100644
> --- a/arch/arm/mach-omap2/board-igep0020.c
> +++ b/arch/arm/mach-omap2/board-igep0020.c
> @@ -435,12 +435,26 @@ static int igep_twl_gpio_setup(struct device *dev,
>  	return 0;
>  };
>  
> +static int igep_twl_gpio_teardown(struct device *dev,
> +		unsigned gpio, unsigned ngpio)
> +{
> +	omap_hsmmc_deferred_del(mmc);
> +	gpio_free(gpio + TWL4030_GPIO_MAX + 1);
> +	if (machine_is_igep0030())
> +		return 0;
> +
> +	gpio_free(gpio + 1);
> +	gpio_free(gpio + TWL4030_GPIO_MAX);

gpio_free_array()?

> +	return 0;
> +}
> +
>  static struct twl4030_gpio_platform_data igep_twl4030_gpio_pdata = {
>  	.gpio_base	= OMAP_MAX_GPIO_LINES,
>  	.irq_base	= TWL4030_GPIO_IRQ_BASE,
>  	.irq_end	= TWL4030_GPIO_IRQ_END,
>  	.use_leds	= true,
>  	.setup		= igep_twl_gpio_setup,
> +	.teardown	= igep_twl_gpio_teardown,
>  };
>  
>  static int igep2_enable_dvi(struct omap_dss_device *dssdev)

[...]

> diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
> index 8b6065b..e1f496d 100644
> --- a/arch/arm/mach-omap2/board-overo.c
> +++ b/arch/arm/mach-omap2/board-overo.c
> @@ -419,12 +419,20 @@ static int overo_twl_gpio_setup(struct device *dev,
>  	return 0;
>  }
>  
> +static int overo_twl_gpio_teardown(struct device *dev,
> +		unsigned gpio, unsigned ngpio)
> +{
> +	omap_hsmmc_deferred_del(mmc);

If Overo will not have omap_hsmmc_deferred_add() (as for my comment to
the previous patch), then no need for the omap_hsmmc_deferred_del().

> +	return 0;
> +}
> +
>  static struct twl4030_gpio_platform_data overo_gpio_data = {
>  	.gpio_base	= OMAP_MAX_GPIO_LINES,
>  	.irq_base	= TWL4030_GPIO_IRQ_BASE,
>  	.irq_end	= TWL4030_GPIO_IRQ_END,
>  	.use_leds	= true,
>  	.setup		= overo_twl_gpio_setup,
> +	.teardown	= overo_twl_gpio_teardown,
>  };
>  
>  static struct regulator_init_data overo_vmmc1 = {

[...]

> diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
> index 51e3a2d..0f256ca 100644
> --- a/arch/arm/mach-omap2/hsmmc.c
> +++ b/arch/arm/mach-omap2/hsmmc.c
> @@ -541,6 +541,17 @@ void omap_hsmmc_deferred_add(struct omap2_hsmmc_info *controllers)
>  	}
>  }
>  
> +void omap_hsmmc_deferred_del(struct omap2_hsmmc_info *controllers)
> +{
> +	struct platform_device *pdev;
> +	for (; controllers->mmc; controllers++) {
> +		if (!controllers->deferred)
> +			continue;
> +		pdev = controllers->pdev;
> +		omap_device_unregister(pdev);

Just:

if (controllers->deferred)
	omap_device_unregister(controllers->pdev);

and no need for the for loop brackets?

> +	}
> +}
> +
>  void omap_hsmmc_init(struct omap2_hsmmc_info *controllers)
>  {
>  	u32 reg;

[...]

- -- 
Regards,
Igor.