On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:Clean up pda_power interrupt handling:
Nice, thanks! Just few cosmetic comments.
Prior to this patch, the driver would pass information it needed
to the interrupt handler dev_id pointer, and then prompt forget it
ever did so, recreating that same information after a couple passes
through the timer-based state machine.
This patch removes the redundant checks by passing the
pda_power_supply[] pointer through the state machine. The current
code passed 'irq' through the state machine, as an index to recreate
the pointer, when we could more simply pass around the pointer itself.
Additionally, bogus "return;" statements were removed.
My preference is to use "return;" statements in functions returning
void, even if functions is very small. It's just sugar for my eyes,
to really see exit points. . Without returns I've feeling that
something is missing there. Yes, my oddity.
Plus, so far CodingStyle does not say anything about non obligatory
return statements.
These should be "fixed" too, though:
~/linux-2.6$ grep -h "return;" -A1 -r drivers/ arch/ | grep "^}$" | \
wc -l
1354
Obviously, drivers/ata is almost pure (3). ;-)
Either way, I prefer to leave alone these "return;"s, until CodingStyle
permits them.
This patch makes it easier to remove the 'irq' argument in the future,
in addition to cleaning up the driver today.
P.S. Where are the MAINTAINERS entries for this driver, and
drivers/power in general?
I'll send patch shortly.
+ void *power_supply = (void *) power_supply_ptr;^
I guess common practice is not to put space here.
+ charger_timer.data = (unsigned long) power_supply;
And here.