lkml.org 
[lkml]   [2004]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [2.4] forcedeth network driver
Carl-Daniel Hailfinger wrote:
>>>+#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,28))
>>>+static inline void synchronize_irq_wrapper(unsigned int irq) {
>>>synchronize_irq(); }
>>>+#undef synchronize_irq
>>>+#define synchronize_irq(irq) synchronize_irq_wrapper(irq)
>>>+#endif
>>
>>
>>Just introduce a diff between 2.4 and 2.6 versions.
>
>
> The chunk above is the only difference between the 2.4 and the 2.6
> version. Should I remove the #ifdef or change the line calling
> synchronize_irq itself?

Just change the line itself, for the 2.4 and 2.6 kernel.org trees...


>>>+ dprintk(KERN_DEBUG "%s: start_xmit: packet packet %d queued for
>>>transmission.\n",
>>>+ dev->name, np->next_tx);
>>>+ {
>>>+ int j;
>>>+ for (j=0; j<64; j++) {
>>>+ if ((j%16) == 0)
>>>+ dprintk("\n%03x:", j);
>>>+ dprintk(" %02x", ((unsigned char*)skb->data)[j]);
>>>+ }
>>>+ dprintk("\n");
>>>+ }
>>
>>
>>would be nice if this loop was optimized out by the compiler.
>
>
> You mean it should be removed? AFAICS, we have
> #define dprintk(x...) do { } while (0)
> so the compiler _should_ optimize it out if we use at least -O.

<shrug> no biggie either way. it's just the sorta code I would throw
an ifdef or "if (static_constant)" around.


>>>+ if (!is_valid_ether_addr(dev->dev_addr)) {
>>>+ /*
>>>+ * Bad mac address. At least one bios sets the mac address
>>>+ * to 01:23:45:67:89:ab
>>>+ */
>>>+ printk(KERN_ERR "%s: Invalid Mac address detected:
>>>%02x:%02x:%02x:%02x:%02x:%02x\n",
>>>+ pci_name(pci_dev),
>>>+ dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
>>>+ dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
>>>+ printk(KERN_ERR "Please complain to your hardware vendor.
>>>Switching to a random MAC.\n");
>>>+ dev->dev_addr[0] = 0x00;
>>>+ dev->dev_addr[1] = 0x00;
>>>+ dev->dev_addr[2] = 0x6c;
>>>+ get_random_bytes(&dev->dev_addr[3], 3);
>>>+ }
>>
>>
>>I don't like this random MAC address stuff. Regardless of hardware
>>behavior, this breaks MAC address uniqueness.
>>
>>I would much rather that ->open() failed, if a valid MAC address were
>>not present. Then a simple userspace program can be run by the admin
>>(policy!) that sets the MAC address, before bringing up the interface.
>
>
> This was a feature many users asked for. It seems broken hardware is very
> common for this chipset.

That's fine, but it doesn't have to be implemented in the kernel. Users
can just as easily download a generic userspace program that checks your
MAC address, and attempts to assign you one if you don't already have one.

This is a good example of putting policy in the kernel, in fact :)


> Will take a look at other drivers. Do you have any good example?

e100, e1000, tg3, 8139cp, ...

Jeff



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

\
 
 \ /
  Last update: 2005-03-22 14:00    [W:0.079 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site