On Mon, 29 Oct 2007 22:51:42 +0100
Florian Fainelli [off-list ref] wrote:

This patch adds support for the RDC R6040 MAC we can find in the RDC R-321x System-on-chips.
This driver really needs improvements especially on the NAPI part which probably does not
fully use the new NAPI structure.
You will need the RDC PCI identifiers if you want to test this driver which are the following ones :

RDC_PCI_VENDOR_ID = 0x17f3
RDC_PCI_DEVICE_ID_RDC_R6040 = 0x6040

Thank you very much in advance for your comments.

Signed-off-by: Sten Wang <redacted>
Signed-off-by: Daniel Gimpelevich <redacted>
Signed-off-by: Florian Fainelli <redacted>

**** BUG *** Don't call kfree() to free the network device; use free_netdev()

* Don't define use uppercase for variable names (NUM_MAC_TABLE)

* Use get_random_ether_addr() rather than a hardcoded table of mac addresses.

* checkpatch complains about some extra blanks, and several lines > 80 chars.

* use ethtool stubs for check_link

* add ethtool get_settings to allow use by bonding/bridging, etc.

* this is unusual coding style:
+	do {} while ((i++ < 2048) && (inw(ioaddr + 0x04) & 0x1));

* add a blank line after declarations and before code in a function

* use of global NAPI_status should be replaced by putting it in priv

* the handling of shared IRQ is wrong.
     - need to check for status == 0 || status == 0xffff and return IRQ_NONE

* don't call napi_disable() with irq's disabled in r6040_close

* poll routine shouldn't call dev_kfree_skb_irq() to free Tx buffers because
   that means going through TX softirq, just call dev_kfree_skb()

* the down routine calls pci_unmap_single with wrong length when handling
   TX buffers.

* pci id table can be cleaned up:
static struct pci_device_id r6040_pci_tbl[] = {
	{ PCI_DEVICE(PCI_VENDOR_ID_RDC, 0x6040) },
	{ PCI_DEVICE(PCI_VENDOR_VIA, 0x3065) },
	{ 0 }
};

* use netdev_priv() consistently rather than dev->priv.
   Yes they are the same now, but that will be fixed in future.

* eliminate check for dev being NULL in IRQ handler.

* reorder functions to eliminate need for forward declarations

* get rid of R6040_PCI_CMD and pci_flags field it is unused.
  
* do you really have to have the whole chip_info at all? The only usage
   seems to be to validate the pci region size.  Do you have platforms with
   busted BIOS that set it wrong or something??



---

WARNING: no space between function name and open parenthesis '('
#1071: FILE: drivers/net/r6040.c:958:
+static int __init r6040_init (void)

WARNING: no space between function name and open parenthesis '('
#1073: FILE: drivers/net/r6040.c:960:
+	return pci_register_driver (&r6040_driver);

WARNING: no space between function name and open parenthesis '('
#1077: FILE: drivers/net/r6040.c:964:
+static void __exit r6040_cleanup (void)

WARNING: no space between function name and open parenthesis '('
#1079: FILE: drivers/net/r6040.c:966:
+	pci_unregister_driver (&r6040_driver);

total: 0 errors, 36 warnings, 1001 lines checked
Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.