From: "Eric Lemoine" <[EMAIL PROTECTED]>
Date: Fri, 10 Nov 2006 14:28:41 +0100
> On 11/10/06, David Miller <[EMAIL PROTECTED]> wrote:
> >
> > Please use GREG_STAT_* instead of magic constants for the
> > interrupt mask and ACK register writes. In fact, there are
> > some questionable values you use, in particular this one:
> >
> > +static inline void gem_ack_int(struct gem *gp)
> > +{
> > + writel(0x3f, gp->regs + GREG_IACK);
> > +}
>
> This code clears bits 0 through 6, which GREG_IACK is for.
>
> > There is no bit defined in GREG_STAT_* for 0x08, but you
> > set it in this magic bitmask. It is another reason not
> > to use magic constants like this :-)
>
> Yes, the bit 4 isn't used, but I assumed clearing it shouldn't be prolem.
>
> Would you prefer that:
>
> #define GREG_STAT_IACK 0x3f
>
> static inline void gem_ack_int(struct gem *gp)
> {
> writel(GREG_STAT_IACK, gp->regs + GREG_IACK);
> }
>
> or that:
>
> #define GREG_STAT_IACK (GREG_STAT_TXINTME | GREG_STAT_TXALL | \
> GREG_STAT_RXDONE | GREG_STAT_RXNOBUF)
>
> to avoid bit 4, and bit 3 (TX_DONE) which is masked.
>
> Personnaly, I like the first way best.
I think the second way is better because it clearly states
your intentions. The GREG_STAT_IACK 0x3f macro is still
"magic" because it doesn't say what the bits actually mean.
-
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
