From: Alan <alan@lxorguk.ukuu.org.uk>
To: Jay Cliburn <jacliburn@bellsouth.net>
Cc: jeff@garzik.org, shemminger@osdl.org, romieu@fr.zoreil.com,
csnook@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver
Date: Mon, 20 Nov 2006 01:15:34 +0000 [thread overview]
Message-ID: <20061120011534.54b1e010@localhost.localdomain> (raw)
In-Reply-To: <20061119202817.GA29736@osprey.hogchain.net>
Would be nice if it used atl_ not at_ so its less likely to cause
namespace clashes.
You have various macros for swaps that are pretty ugly - we have
cpu_to and le/be_to_cpu functions for most swapping cases and these are
generally optimised assembler (eg bswap on x86)
AT_DESC_USED/UNUSED would be better as inline functions but thats not a
serious concern.
Be careful with :1 bitfields when working with hardware - the compiler
has more than one choice about how to pack them.
The irq enable/disable use for locking on vlan appears unsafe. PCI
interrupt delivery is asynchronous which means you can get this happen
card sends PCI interrupt
We call irq_disable
We take lock
We poke bits
We drop lock
PCI interrupt arrives
This really does happen, typically its nasty to debug as well because you
usually only get it on PIII boards on the one in n-zillion times a
message collides and is retransmitted on the APIC bus.
skb->len is unsigned so <= 0 can be == 0. More importantly the subtraction
before the test will wrap and is completely unsafe (see at_xmit_frame)
Alan
next prev parent reply other threads:[~2006-11-20 1:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-19 20:28 [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver Jay Cliburn
2006-11-20 1:15 ` Alan [this message]
2006-11-20 6:34 ` Chris Snook
2006-11-20 9:46 ` Alan
2006-11-20 6:37 ` Chris Snook
2006-11-21 20:25 ` Luca Tettamanti
2006-11-25 22:43 ` Luca Tettamanti
2006-11-26 1:33 ` Jay Cliburn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20061120011534.54b1e010@localhost.localdomain \
--to=alan@lxorguk.ukuu.org.uk \
--cc=csnook@redhat.com \
--cc=jacliburn@bellsouth.net \
--cc=jeff@garzik.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=romieu@fr.zoreil.com \
--cc=shemminger@osdl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).