Re: [PATCH] pda_power: clean up irq, timer, return usage

[Anton Vorontsov <cbou@mail.ru>]

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.


I'll correct this, and will apply to battery2-2.6.git.

Thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/