netdev
[Top] [All Lists]

Re: [8/9][PATCH 2.6] Small fixes and clean-up

To: Roger Luethi <rl@xxxxxxxxxxx>
Subject: Re: [8/9][PATCH 2.6] Small fixes and clean-up
From: Jeff Garzik <jgarzik@xxxxxxxxx>
Date: Sat, 19 Jun 2004 17:28:01 -0400
Cc: Andrew Morton <akpm@xxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <20040615174956.GA11359@xxxxxxxxxxxxxx>
References: <20040615174956.GA11359@xxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040510
Roger Luethi wrote:
@@ -678,22 +681,43 @@
io_size = 256;
        phy_id = 0;
-       if (pci_rev < VT6102) {
+       quirks = 0;
+       name = "Rhine";
+       mname = "unknown";
+       if (pci_rev < VTunknown0) {
                quirks = rqRhineI;
                io_size = 128;
-               name = "VT86C100A Rhine";
+               mname = "VT86C100A";
        }
-       else {
+       else if (pci_rev >= VT6102) {
                quirks = rqWOL | rqForceReset;
                if (pci_rev < VT6105) {
                        name = "Rhine II";
                        quirks |= rqStatusWBRace;       /* Rhine-II exclusive */
+                       if (pci_rev < VT8231)
+                               mname = "VT6102";
+                       else if (pci_rev < VT8233)
+                               mname = "VT8231";
+                       else if (pci_rev < VT8235)
+                               mname = "VT8233";
+                       else if (pci_rev < VT8237)
+                               mname = "VT8235";
+                       else if (pci_rev < VTunknown1)
+                               mname = "VT8237";
                }
                else {
                        name = "Rhine III";
                        phy_id = 1;     /* Integrated PHY, phy_id fixed to 1 */
                        if (pci_rev >= VT6105_B0)
                                quirks |= rq6patterns;
+                       if (pci_rev < VT6105L)
+                               mname = "VT6105";
+                       else if (pci_rev < VT6107)
+                               mname = "VT6105L";
+                       else if (pci_rev < VT6105M)
+                               mname = "VT6107";
+                       else if (pci_rev >= VT6105M)
+                               mname = "Management Adapter VT6105M";
                }
        }

In Linux lists of model names are discouraged. It's not terribly bad in via-rhine, but overall these things wind up getting patches quite often, and become a maintenance annoyance.

It's up to you as maintainer, but I would recommend removing the string completely. For dmesg/printk purposes, the user only needs to know they have a 'via-rhine' controller.


-       dev = alloc_etherdev(sizeof(*rp));
-       if (dev == NULL) {
+       dev = alloc_etherdev(sizeof(struct rhine_private));
+       if (!dev) {
                rc = -ENOMEM;
-               printk(KERN_ERR "init_ethernet failed for card #%d\n",
-                      card_idx);
+               printk(KERN_ERR "alloc_etherdev failed\n");

this error message change seems like a step backwards... print out pci_name() or _something_ to let the user know which card failed.


                goto err_out;
        }
        SET_MODULE_OWNER(dev);
        SET_NETDEV_DEV(dev, &pdev->dev);
- rc = pci_request_regions(pdev, shortname);
+       rc = pci_request_regions(pdev, DRV_NAME);
        if (rc)
                goto err_out_free_netdev;
@@ -768,9 +791,8 @@
        rp = netdev_priv(dev);
        rp->quirks = quirks;
+ /* Get chip registers into a sane state */
        rhine_power_init(dev);
-
-       /* Reset the chip to erase previous misconfiguration. */
        rhine_hw_init(dev, pioaddr);
for (i = 0; i < 6; i++)
@@ -778,16 +800,11 @@
if (!is_valid_ether_addr(dev->dev_addr)) {
                rc = -EIO;
-               printk(KERN_ERR "Invalid MAC address for card #%d\n", card_idx);
+               printk(KERN_ERR "Invalid MAC address\n");
                goto err_out_unmap;

ditto


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