Messages in this thread |  | | Date | Fri, 17 Nov 2000 16:42:16 -0500 | From | Jeff Garzik <> | Subject | Re: sunhme.c patch for new PCI interface (UNTESTED) |
| |
"Adam J. Richter" wrote: > -static struct happy_meal *root_happy_dev = NULL; > - > #ifdef CONFIG_SBUS > +static struct happy_meal *root_happy_dev = NULL; > static struct quattro *qfe_sbus_list = NULL; > #endif
don't initialize static to zero/null explicitly.. > - if (dev == NULL) { > - dev = init_etherdev(0, sizeof(struct happy_meal)); > - } else { > - dev->priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); > - if (dev->priv == NULL) > - return -ENOMEM; > - } > + dev = init_etherdev(0, sizeof(struct happy_meal));
allocation failure not checked
> -static int __init happy_meal_pci_init(struct net_device *dev, struct pci_dev *pdev) > +static int __devinit happy_meal_pci_probe(struct pci_dev *pdev, > + const struct pci_device_id *id)
Are you aware of any hotplug sunhme hardware? If no, don't change it to __devinit...
> - if (dev == NULL) { > - dev = init_etherdev(0, sizeof(struct happy_meal)); > - } else { > - dev->priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); > - if (dev->priv == NULL) > - return -ENOMEM; > - } > + dev = init_etherdev(0, sizeof(struct happy_meal));
check for failure.
also, ether_setup() call is not needed if you always call init_etherdev(NULL, ...).
> +static void __devexit happy_meal_pci_remove (struct pci_dev *pdev) > +{ > + struct happy_meal *hp = pdev->driver_data; > + > + pci_free_consistent(hp->happy_dev, > + PAGE_SIZE, > + hp->happy_block, > + hp->hblock_dvma); > + unregister_netdev(hp->dev); > + kfree(hp->dev); > +}
zero pdev->driver_data. If this driver has to be portable, use pci_{get,set}_drvdata() instead of directly accessing ::driver_data.
> +static struct pci_device_id happymeal_pci_ids[] __devinitdata = {
again, no __devxxx if not hotplug.
> #ifdef CONFIG_PCI > - cards += happy_meal_pci_probe(dev); > + return pci_module_init(&happy_meal_pci_driver); > +#else > + return (cards > 0) ? 0 : -ENODEV; > #endif
ifdef not needed
> +#ifdef CONFIG_PCI > + pci_unregister_driver (&happy_meal_pci_driver); > +#endif
ifdef not needed.
-- Jeff Garzik | Building 1024 | The chief enemy of creativity is "good" sense MandrakeSoft | -- Picasso - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |