Messages in this thread | | | Date | Sat, 01 Dec 2007 18:28:54 -0500 | From | Jeff Garzik <> | Subject | Re: [patch] ata: ahci: Enclosure Management via LED rev2 |
| |
Kristen Carlson Accardi wrote: > Enclosure Management via LED > > This patch implements Enclosure Management via the LED protocol as specified > in AHCI specification. > > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com> > --- > This revision makes the change to the comment requested by Mark Lord, > fixes some bugs in the bit shifting for writing the new led state, > and implements a show function so that led status can be read as > well as written.
Overall looks pretty good, from a technical review perspective.
Two worries:
1) exporting ata_scsi_find_dev(), and assuming a scsi device is attached. the latter can be fixed by a !NULL check (and should be), but its a bit of a layering violation since long term we want to make the SCSI simulator optional for all ATA devices.
2) vaguely related to #1, I'm not so sure the attributes should be implemented directly in ahci. if this __or something like it__ appears on non-Intel hardware, the code should be somewhere more generic.
| |