Matthew Wilcox wrote:
+static int __devinit
+advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+       int err, ioport;
+       struct Scsi_Host *shost;
+
+       err = pci_enable_device(pdev);
+       if (err)
+               goto fail;
+
+       ioport = pci_resource_start(pdev, 0);
+       shost = advansys_board_found(ioport, &pdev->dev, ASC_IS_PCI);
+
+       if (shost) {
+               pci_set_drvdata(pdev, shost);
+               return 0;
+       }
+
+       err = -ENODEV;
+       pci_disable_device(pdev);
+ fail:
+       return err;


minor nit:

it is far more like other Linux drivers to do

        if (!shost) {
                err = -ENODEV;
                goto err_out_shost;
        }

        pci_set_drvdata(pdev, shost);
        return 0;

It keeps the main, non-error path at the lowest indentation level, which is more future-proof (easier to add more clauses later).

It also makes your PCI probe function easier to review, to ensure that the order of initialization matches the reversed-order of shutdown in pci_driver::remove()

        Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to