Received: with ECARTIS (v1.0.0; list netdev); Tue, 22 Mar 2005 12:52:48 -0800 (PST)
Received: from parcelfarce.linux.theplanet.co.uk (IDENT:93@parcelfarce.linux.theplanet.co.uk [195.92.249.252])
	by oss.sgi.com (8.13.0/8.13.0) with ESMTP id j2MKqgEs002226
	for <netdev@oss.sgi.com>; Tue, 22 Mar 2005 12:52:42 -0800
Received: from cpe-069-134-152-124.nc.rr.com ([69.134.152.124] helo=[10.10.10.88])
	by parcelfarce.linux.theplanet.co.uk with asmtp (TLSv1:AES256-SHA:256)
	(Exim 4.33)
	id 1DDqMi-0001FH-Ap; Tue, 22 Mar 2005 20:52:40 +0000
Message-ID: <4240858B.5080209@pobox.com>
Date: Tue, 22 Mar 2005 15:52:27 -0500
From: Jeff Garzik <jgarzik@pobox.com>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040922
X-Accept-Language: en-us, en
MIME-Version: 1.0
To: Michael Chan <mchan@broadcom.com>, "David S. Miller" <davem@davemloft.net>
CC: netdev@oss.sgi.com
Subject: Re: [PATCH 2.6.11 1/8] tg3: add 5705_plus flag
References: <B1508D50A0692F42B217C22C02D84972020F3E21@NT-IRVA-0741.brcm.ad.broadcom.com>
In-Reply-To: <B1508D50A0692F42B217C22C02D84972020F3E21@NT-IRVA-0741.brcm.ad.broadcom.com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
X-Virus-Scanned: ClamAV 0.83/778/Mon Mar 21 02:48:43 2005 on oss.sgi.com
X-Virus-Status: Clean
X-archive-position: 480
X-ecartis-version: Ecartis v1.0.0
Sender: netdev-bounce@oss.sgi.com
Errors-to: netdev-bounce@oss.sgi.com
X-original-sender: jgarzik@pobox.com
Precedence: bulk
X-list: netdev
Content-Length: 1377
Lines: 45

Michael Chan wrote:
> Add a 5705_plus flag to indicate the device is 5705, 5750, or future chips
> that all share the same basic architecture. This makes it easier to add
> support for future devices.
> 
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com

ACK, and two comments:

1) In general, I encourage changes like this, for both tg3 and other net 
drivers.  It is -much- better to define a feature flag, and test the 
feature flag in the source code, than to define a list of affected 
[chips | arches].  Changes like this

-       if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705 ||
-           GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5750) {
+       if (tp->tg3_flags2 & TG3_FLG2_5705_PLUS) {

can be an example to others for future changes.


2) [administrivia]  Your patches are encoded as base64, which makes 
reviewing and applying your patches difficult.  You are violating clause 
#3, rules one and two:

	http://linux.yyz.us/patch-format.html

For sending patches, I highly recommend using a text file template, and 
/usr/sbin/sendmail (sendmail/postfix/exim mail servers provide this).  See

	http://lkml.org/lkml/2005/2/24/3

for more info.

If you don't want to bother with Unix, I -think- Mozilla Thunderbird for 
Windows will correctly attach patches in a way that is useable.

	http://www.mozilla.org/products/thunderbird/

Regards,

	Jeff