On Thu, 2009-12-03 at 03:12 -0500, Jeff Garzik wrote:

> Looks fine to me.  Two minor comments, which might perhaps be ignored if 
> that is your taste:
> 
> * prefer enums to #define's, for constants

yeah well ... I lifted those definitions from the old driver and didn't
feel like changing them all :-) I might do a separate patch later to
clean that up, we don't actually use a lot of those anymore since I use
pre-calculated tables, though they are good to keep as documentation.

> * prefer direct function call to "ap->ops->foo_bar()", because 
> ap->ops->foo_bar() is guaranteed to be a constant value known to the 
> driver.  The driver is the entity responsible for the function pointer.
>
> Maybe saves a cycle or two.  Not terribly important, but hey, calling 
> ap->ops->sff_exec_command() from pata_macio_bmdma_setup() is a hot path.

Makes sense. I'm tempted to make that a separate patch tho, since I've
already queued up the existing one and it's just a relatively minor
performance optim. 

Cheers,
Ben.


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to