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. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
