On Tue, Mar 22, 2005 at 03:52:27PM -0500, Jeff Garzik wrote:
> 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@xxxxxxxxxxxx
>
> 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.
Except this isn't really a feature flag, per se. That would be
something like TG3_HAS_FOO, in other words "what particular feature
this if statement actually cares about". What is currently 5705_PLUS
would then be a bunch of flags HAS_SENSIBLE_X | HAS_FAST_Y | HAS_WORKING_Z.
The "family flag" approach is fine until you run into something that
has X and Y but not Z.
--
Mathematics is the supreme nostalgia of our time.
|