netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
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
Date: Tue, 22 Mar 2005 15:52:27 -0500	[thread overview]
Message-ID: <4240858B.5080209@pobox.com> (raw)
In-Reply-To: <B1508D50A0692F42B217C22C02D84972020F3E21@NT-IRVA-0741.brcm.ad.broadcom.com>

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

  reply	other threads:[~2005-03-22 20:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-03-21  7:14 [PATCH 2.6.11 1/8] tg3: add 5705_plus flag Michael Chan
2005-03-22 20:52 ` Jeff Garzik [this message]
2005-03-24  1:51   ` Matt Mackall
2005-03-23 19:03 ` David S. Miller

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=4240858B.5080209@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=davem@davemloft.net \
    --cc=mchan@broadcom.com \
    --cc=netdev@oss.sgi.com \
    /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).