Re: [PATCH] gpio: omap-gpio: add support for pm_runtime autosuspend

[Tim Niemeyer <tim.niemeyer@corscience.de>]

Am Freitag, den 26.10.2012, 11:03 +0300 schrieb Felipe Balbi:
> Hi,
> 
> On Fri, Oct 26, 2012 at 09:55:30AM +0200, Tim Niemeyer wrote:
> > Adds support for configuring the omap-gpio driver use autosuspend for
> > runtime power management. This can reduce the latency in using it by
> > not suspending the device immediately on idle. If another access takes
> > place before the autosuspend timeout (2 secs), the call to resume the
> > device can return immediately saving some save/ restore cycles.
> > 
> > I use a gpio to monitor a spi transfer which occurs every 250µs. The
> > suspend overhead is to high, so almost every second transfer is lost.
> > This patch fixes that.
> > 
> > Signed-off-by: Tim Niemeyer <tim.niemeyer@corscience.de>
> > ---
> >  drivers/gpio/gpio-omap.c |   23 ++++++++++++++++++-----
> >  1 files changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index 94cbc84..92f48cb 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -31,6 +31,7 @@
> >  #include <asm/mach/irq.h>
> >  
> >  #define OFF_MODE	1
> > +#define GPIO_AUTOSUSPEND_TIMEOUT                2000
> >  
> >  static LIST_HEAD(omap_gpio_list);
> >  
> > @@ -628,8 +629,10 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
> >  	 * If this is the last gpio to be freed in the bank,
> >  	 * disable the bank module.
> >  	 */
> > -	if (!bank->mod_usage)
> > -		pm_runtime_put(bank->dev);
> > +	if (!bank->mod_usage) {
> 
> while at that I would drop this bank->mod_usage nonsense and let
> power core handle reference counting.
I looked at it, but i'm unsure about the GPIO_MOD_CTRL_BIT. The
bank->mod_usage counter prevents omap_gpio_free() to disable the hole
bank while another gpio is still in use.

> > +		pm_runtime_mark_last_busy(bank->dev);
> > +		pm_runtime_put_autosuspend(bank->dev);
> > +	}
> >  }
> >  
> >  /*
> > @@ -715,7 +718,8 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> >  exit:
> >  	if (!unmasked)
> >  		chained_irq_exit(chip, desc);
> > -	pm_runtime_put(bank->dev);
> > +	pm_runtime_mark_last_busy(bank->dev);
> > +	pm_runtime_put_autosuspend(bank->dev);
> >  }
> >  
> >  static void gpio_irq_shutdown(struct irq_data *d)
> > @@ -1132,6 +1136,8 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, bank);
> >  
> > +	pm_runtime_use_autosuspend(bank->dev);
> > +	pm_runtime_set_autosuspend_delay(bank->dev, GPIO_AUTOSUSPEND_TIMEOUT);
> >  	pm_runtime_enable(bank->dev);
> >  	pm_runtime_irq_safe(bank->dev);
> >  	pm_runtime_get_sync(bank->dev);
> > @@ -1146,7 +1152,8 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
> >  	if (bank->loses_context)
> >  		bank->get_context_loss_count = pdata->get_context_loss_count;
> >  
> > -	pm_runtime_put(bank->dev);
> > +	pm_runtime_mark_last_busy(bank->dev);
> > +	pm_runtime_put_autosuspend(bank->dev);
> >  
> >  	list_add_tail(&bank->node, &omap_gpio_list);
> >  
> > @@ -1333,7 +1340,13 @@ void omap2_gpio_prepare_for_idle(int pwr_mode)
> >  
> >  		bank->power_mode = pwr_mode;
> >  
> > -		pm_runtime_put_sync_suspend(bank->dev);
> > +		/* direct pm_runtime on pwroff */
> > +		if (pwr_mode)
> 
> you also need braces here.
Yes. I resend this later.

> > +			pm_runtime_put_sync_suspend(bank->dev);
> > +		else {
> > +			pm_runtime_mark_last_busy(bank->dev);
> > +			pm_runtime_put_sync_autosuspend(bank->dev);
> > +		}
> >  	}
> >  }
> >  
> > -- 
> > 1.7.2.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Tim Niemeyer

Corscience GmbH & Co. KG
Henkestr. 91
D-91052 Erlangen
Germany

e-mail: tim.niemeyer@corscience.de
Internet: www.corscience.de

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html