[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20100607.002805.232920368.davem@davemloft.net>
Date: Mon, 07 Jun 2010 00:28:05 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: mcarlson@...adcom.com
Cc: netdev@...r.kernel.org, andy@...yhouse.net
Subject: Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: "Matt Carlson" <mcarlson@...adcom.com>
Date: Sun, 6 Jun 2010 20:59:34 -0700
> On Sun, Jun 06, 2010 at 06:00:29PM -0700, David Miller wrote:
> The programming manual says this bit prevents a tx state machine lockup
> due to tx mbuf corruption. The conditions under which the tx mbufs get
> corrupted is complicated, but the net effect of this bit is that the
> RDMA engine acts a little more conservatively.
That last phrase is a good description and could have been used to
better name the register bit macro in question. You're basically
confirming that you named the register bit too tersely.
> The problem is that register definitions can change from asic rev to
> asic rev.
Frankly, this kind of justification really ticks me off.
How the heck does that make a difference? Describe which chip revs
the register bit is valid for in a comment for crying out loud.
Document your hardware properly, not selectively or where you see
it fit to do so.
It's bad enough that you guys don't publish your hardware specs.
As a consequence as much knowledge as possible must go into the
driver sources. Any piece of information you take away is bad.
There are tons of chip register bits in this driver already which are
only valid on certain hardware revs. In fact, there are many. Yet
they are all still there in tg3.h whether they are used by the driver
or not. In fact, if I recall correctly, the DMA burst size controls
are pretty much different on every single rev of the chip.
Documenting where for which chips a register bit is valid is
pervasively done elsewhere, and is nothing new. Your driver and your
hardware is not special.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists