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.