On 06/09/2016 08:50 PM, Don Brace wrote: > These patches are based on Linus's tree > > - add smartpqi to kernel.org > - remove PCI IDs from aacraid driver > - Depends on adoption of smartpqi driver > > Changes since initial upload > - Forgot to give correct ownership to the author. > Changes since V1 > - Corrected make ARCH=i386 kbuild test robot issue. > > --- > > Don Brace (1): > aacraid: remove wildcard for series 9 controllers > > Kevin Barnett (1): > smartpqi: initial commit of Microsemi smartpqi driver > > > drivers/scsi/smartpqi/Kconfig | 50 > drivers/scsi/smartpqi/Makefile | 3 > drivers/scsi/smartpqi/smartpqi.h | 1135 ++++ > drivers/scsi/smartpqi/smartpqi_init.c | 6817 ++++++++++++++++++++++++ > drivers/scsi/smartpqi/smartpqi_sas_transport.c | 350 + > drivers/scsi/smartpqi/smartpqi_sis.c | 394 + > drivers/scsi/smartpqi/smartpqi_sis.h | 32 > 7 files changed, 8781 insertions(+) > create mode 100644 drivers/scsi/smartpqi/Kconfig > create mode 100644 drivers/scsi/smartpqi/Makefile > create mode 100644 drivers/scsi/smartpqi/smartpqi.h > create mode 100644 drivers/scsi/smartpqi/smartpqi_init.c > create mode 100644 drivers/scsi/smartpqi/smartpqi_sas_transport.c > create mode 100644 drivers/scsi/smartpqi/smartpqi_sis.c > create mode 100644 drivers/scsi/smartpqi/smartpqi_sis.h > Some general comments: - Wouldn't it be better to split off CISS specific definitions from smartpqi.h into a separate header file? I guess this even could be shared with hpsa, right? - In the same vein: Maybe it's worth considering splitting off the PQI specific ones into a separate header file, too, and just keeping the smartpqi.h file for the driver specific definitions. - The device rescan / device mapping functionality is positively horrible. I still think it was a mistake having that in hpsa, but I'd be loath to see this one repeated in smartpqi. Why can't you use the LUN numbers directly? It's _quite_ easy to have the RAID and passthrough devices on a separate SCSI bus, so that shouldn't be an issue. - The 'devices' sysfs attribute is a sysfs abuse. As per definition each sysfs attribute should be a single entry. If you want/need to have a listing consider adding a sysfs directory for the devices. But then again, if you drop the device rescan/mapping functionality and use the LUN numbers directly none of these would matter and the 'devices' attribute could be dropped, too. - Statistics. Using atomics for each variable and enabling them by default is not a good idea. I'd prefer to have it optional; have you checked using debugfs for this? - 'host_wellness' is cute. Do we get a Spa to go with it? - pqi_alloc_io_request() has this weird 'while' loop. I was under the impression that the block layer would guarantee tag uniqueness, so why don't you use the request tag to identify the command from the request pool (and split the number of tags per hardware queue)? Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@xxxxxxx +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html