On Wed, 2007-10-31 at 14:30 -0700, Kok, Auke wrote:
> Joe Perches wrote:
> that's not a bad idea, however see below:
> can't we keep the macro here (and just make it call the function instead of
> expanding). the resulting code is much more lenghty and contains all these 
> logic
> traps that the previous code didn't have.
> just have the macro expand to `if (reg_pattern_test(...)) return 1)` and you 
> don't
> need to change any of the calling lines.

You could define something like:

#define REG_PATTERN_TEST(reg, mask, write) \
        if (reg_pattern_test(adapter, data, \
                             E1000_REG(&adapter->hw, reg), \
                             mask, write)) \
                return 1;

But isn't the macro with an embedded return a bit too obfuscating?

> > +#define E1000_READ_REG_ARRAY(a, reg, offset)               \
> > +   (readl((a)->hw_addr +                           \
> > +          (((a)->mac_type >= e1000_82543)          \
> > +           ? E1000_##reg : E1000_82542_##reg) +    \
> > +          ((offset) << 2)))
> 
> did you have to change these macro's ?

No.  Your choice to keep/remove.
I did want to use the E1000_REG or a new E1000_REG_ADDR macro.

> also, I'm a bit inclined to prefer a patch for e1000e for now as we're about 
> to
> move the pci-express hardware over, but we can certainly merge something like 
> this
> in e1000 after the move as well.

Simple enough.

When is e1000e scheduled to be part of defconfig?

cheers,  Joe

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to