netdev
[Top] [All Lists]

Re: [PATCH] and enhancements to pcnet32.c for 2.6.3-rc2

To: Don Fry <brazilnut@xxxxxxxxxx>
Subject: Re: [PATCH] and enhancements to pcnet32.c for 2.6.3-rc2
From: "Randy.Dunlap" <rddunlap@xxxxxxxx>
Date: Tue, 17 Feb 2004 14:54:30 -0800
Cc: tsbogend@xxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <200402172131.i1HLVvv02518@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Organization: OSDL
References: <200402172131.i1HLVvv02518@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Tue, 17 Feb 2004 13:31:57 -0800 (PST) Don Fry <brazilnut@xxxxxxxxxx> wrote:

| Attached is a collection of patches for the pcnet32 driver.
| 
| The maintainer has not responded to the following three patches.

Since this email has received such wide reviews in the past, here
are some more comments on it.

a.  make separate patches based on functional changes, not a collection
of patches.

b.  don't use C++-style "//" comments: Linux style is /* ... */



| @@ -213,6 +223,13 @@
|   *      clean up and using new mii module
|   * v1.27b  Sep 30 2002 Kent Yoder <yoder1@xxxxxxxxxx>
|   *      Added timer for cable connection state changes.
| + * v1.28   11 Feb 2004 Don Fry <brazilnut@xxxxxxxxxx>,
| + *      Jon Mason <jonmason@xxxxxxxxxx>, Chinmay Albal <albal@xxxxxxxxxx>,
| + *      Jim Lewis <jklewis@xxxxxxxxxx>.
| + *      Now uses ethtool_ops, netif_msg_*, and generic_mii_ioctl.
| + *      Loopback test added.  Supports PCI (not PCMCIA) hot remove.
| + *      Fixes "Bus master arbitration failure" and pci_[un]map_single
| + *      length errors.
|   */

This should go in the changeset comments.


| @@ -841,8 +1131,10 @@
|      }
|  
|      /* Check for a valid station address */
| -    if( !is_valid_ether_addr(dev->dev_addr) )
| -     return -EINVAL;
| +    if( !is_valid_ether_addr(dev->dev_addr) ) {

Too ugly.  Use:
        if (!is_xyz()) {


| +     ret = -EINVAL;
| +     goto err_free_irq;
| +    }


--
~Randy

<Prev in Thread] Current Thread [Next in Thread>