Re: Sort of patch: Ethernet driver for National Semiconductor DP83815 for linux-2.2.16

From: Alan Cox (alan@lxorguk.ukuu.org.uk)
Date: Fri Jul 28 2000 - 08:32:51 EST


> I have received a patch from Dave Gotwisner at Wyse Technologies
> for an ethernet driver for the National Semiconductor dp83815 chip

Well parts of it appear to be uncredited Don Becker work, and the rest of
it is just ugly. Don's natsemi.c may well be a better choice ? - see
his driver archives.

> +2.2.2 Unsupported Features
> + o Newer MP kernels 2.2.xx

So its a non candidate.

> + * SWAP_BUS_TO_CPU_XX and SWAP_CPU_TO_BUS_XX macros swap 16 and 32 bit values
> + * between the PCI bus' little endian byte-order and the CPU's native

cpu_to_le16 etc...

> +typedef volatile u8 * bus_addr; /* BUS physical address */

Should be unsigned long

> +typedef u8 bool;

int is faster

> +#define FALSE 0
> +
> +bool dp_full_duplex; /* flag for full duplex mode */

Should be static

> + pcibios_read_config_byte (bus, func, PCI_INTERRUPT_LINE, &irq);
> + pcibios_read_config_dword (bus, func, PCI_BASE_ADDRESS_0, &iobase);
> + iobase &= PCI_BASE_ADDRESS_IO_MASK;

Wont work for 2.2

> + if (dev->priv == NULL)
> + dev->priv = kmalloc(sizeof(struct dp83815_priv), GFP_KERNEL);
> + if (dev->priv == NULL)
> + return -ENOMEM;

Fails to free dev

> + /* If address is not set, set up a default one */
> + if ((dev->dev_addr[0] == 0) && (dev->dev_addr[1] == 0) &&
> + (dev->dev_addr[2] == 0) && (dev->dev_addr[3] == 0) &&
> + (dev->dev_addr[4] == 0) && (dev->dev_addr[5] == 0))
> + {
> + u32 random = jiffies;
> + u8 * ptr = (u8 *) &random;

This is suicidal. Several identical boxes powering on together will pick
the same address. If the address isnt set fail it

> +/* io_read_16 - read the 16 bit value stored at given io address */
> +static u16 io_read_16 (u16 io_port)
> +{
> + return DP_INW (io_port);
> +}

Uninlined i/o operations ??

Can you also bang it into the shape the CodingStyle document asks - indent
to tab etc.

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jul 31 2000 - 21:00:27 EST