Matthew Wilcox wrote:
+static void __devexit advansys_remove(struct Scsi_Host *shost)
+{
+ int i;
+ scsi_remove_host(shost);
+ advansys_release(shost);
+
+ for (i = 0; i < asc_board_count; i++) {
+ if (asc_host[i] == shost) {
+ asc_host[i] = NULL;
+ break;
+ }
+ }
+}
1) IMO this should be in the PCI patch
+static int __devinit advansys_eisa_probe(struct device *dev)
+{
+ int i, ioport;
+ int err = -ENODEV;
+ struct eisa_device *edev = to_eisa_device(dev);
+
+ struct eisa_scsi_data *data = kzalloc(sizeof(*data), GFP_KERNEL);
nits:
2) check for NULL
3) maybe it's just me, but the mixing initializers and allocation code
is way too close to C99/C++ code/decl mixing. I would declare 'data',
then initialize it on an separate line. But again, maybe that's just me.
+ ioport = edev->base_addr + 0xc30;
+
+ for (i = 0; i < 2; i++, ioport += 0x20) {
+ if (!AscFindSignature(ioport))
+ continue;
+ inw(ioport + 4);
4) would be nice to have a comment noting what this inw() does.
5) I would suggest putting a "remove inpw/outpw pointless wrappers"
cleanup patch before patches #2 .. #N.
+ data->host[i] = advansys_board_found(ioport, dev, ASC_IS_EISA);
+ if (data->host[i])
+ err = 0;
+ }
+
+ if (err) {
+ kfree(data);
+ } else {
+ dev_set_drvdata(dev, data);
+ }
+
+ return err;
+}
+
+static __devexit int advansys_eisa_remove(struct device *dev)
+{
+ int i, ioport;
+ struct eisa_scsi_data *data = dev_get_drvdata(dev);
+
+ for (i = 0; i < 2; i++) {
+ struct Scsi_Host *shost = data->host[i];
+ if (!shost)
+ continue;
+ ioport = shost->io_port;
+ advansys_remove(data->host[i]);
+ }
+
+ return 0;
6) set drvdata to NULL
+static struct eisa_driver advansys_eisa_driver = {
+ .id_table = advansys_eisa_table,
+ .driver = {
+ .name = "advansys",
+ .probe = advansys_eisa_probe,
+ .remove = __devexit_p(advansys_eisa_remove),
+ }
7) values much more readable when tab-aligned
-
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
