netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: mcarlson@broadcom.com
Cc: netdev@vger.kernel.org, andy@greyhouse.net
Subject: Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
Date: Sun, 06 Jun 2010 18:00:29 -0700 (PDT)	[thread overview]
Message-ID: <20100606.180029.42789560.davem@davemloft.net> (raw)
In-Reply-To: <1275794679-11085-1-git-send-email-mcarlson@broadcom.com>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Sat, 5 Jun 2010 20:24:29 -0700

> This patchset adds some bugfixes and adds 5719 device support.

All applied to net-next-2.6 but there are two things I'm very disappointed
with in this series:

1) Naming register bits things like "TX_MBUF_FIX" isn't descriptive,
   and I suspect the actual bit name used in your programming manuals
   is very different and would be more helpful to someone reading the
   code and trying to understand exactly what that bit does.

   How does it change the chips internal MBUF handling behavior?  Does
   it insert a delay in accesses or state changes?  Does it change
   the MBUF arbitration?  What the heck does that bit do exactly?

2) Removing register definitions is something we really shouldn't do.
   Just because you're not using the register any more in the driver,
   doesn't mean you should remove it's definition from tg3.h

   What if some other developer wants to play with that register and
   use it for some other purpose or experiment?

You really have to handle situations like #1 and #2 better especially
since you guys do not public post the full PDF hardware programming
manuals of your chips online for other developers to use.

I wouldn't, therefore, impose these rules on Intel and their drivers
because they do public the programming manuals for their networking
chips.

  reply	other threads:[~2010-06-07  1:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-06  3:24 [PATCH net-next 00/11] tg3: Bugfixes and 5719 support Matt Carlson
2010-06-07  1:00 ` David Miller [this message]
2010-06-07  3:59   ` Matt Carlson
2010-06-07  7:28     ` David Miller
2010-06-07 18:40       ` Matt Carlson
2010-06-07 23:44         ` David 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=20100606.180029.42789560.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=andy@greyhouse.net \
    --cc=mcarlson@broadcom.com \
    --cc=netdev@vger.kernel.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).