Re: [rfc] separate sharpsl_pm initialization from sysfs code

[Russell King <rmk+lkml@arm.linux.org.uk>]

On Fri, Mar 10, 2006 at 07:07:20PM +0100, Pavel Machek wrote:
> 	/* Register interrupt handler. */
> 	if ((err = request_irq(COLLIE_IRQ_GPIO_AC_IN, sharpsl_ac_isr, SA_INTERRUPT,
> 			       "ACIN", sharpsl_ac_isr))) {
> 		printk("Could not get irq %d.\n", COLLIE_IRQ_GPIO_AC_IN);
> 		return;
> 	}
> 	if ((err = request_irq(COLLIE_IRQ_GPIO_CO, sharpsl_chrg_full_isr, SA_INTERRUPT,
> 			       "CO", sharpsl_chrg_full_isr))) {
> 		free_irq(COLLIE_IRQ_GPIO_AC_IN, sharpsl_ac_isr);
> 		printk("Could not get irq %d.\n", COLLIE_IRQ_GPIO_CO);
> 		return;
> 	}

Shouldn't these be ucb1x00_hook_irq()'s?

Shouldn't you only enable the ADC when you need to use it?

This driver makes no calls to ucb1x00_enable() and ucb1x00_disable().
They're part of power management, and if your driver doesn't appear to
require them to work, that means some other code is buggy (or you're
keeping the ADC always enabled.)

In order to get maximum power savings, you should ensure that drivers
only call ucb1x00_enable() when they need to and ucb1x00_disable() as
soon as they can complete the operation.  Ditto for ucb1x00_adc_enable()
and ucb1x00_adc_disable() - don't use ucb1x00_enable() and
ucb1x00_adc_enable() from a probe/initialisation function and then
leave it alone.  Unless you want to needlessly waste power.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core