Re: sunhme.c patch for new PCI interface (UNTESTED)

[Jeff Garzik <jgarzik@mandrakesoft.com>]

"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/