....
 

Guardian Digital Inc. > InfoCenter > Mailing List Archives > Linux Kernel

Linux Kernel Mailing List Archive

From: Dale Farnsworth (dale@farnsworth.org)
Date: Wed Dec 15 2004 - 13:32:40 EST


On Tue, Dec 14, 2004 at 11:19:24PM +0000, Christoph Hellwig wrote:
> > +#undef MV_READ
> > +#define MV_READ(offset) \
> > + readl(mv64x60_eth_shared_base - MV64340_ETH_SHARED_REGS + offset)
> > +
> > +#undef MV_WRITE
> > +#define MV_WRITE(offset, data) \
> > + writel((u32)data, \
> > + mv64x60_eth_shared_base - MV64340_ETH_SHARED_REGS + offset)
> > +
>
> please use different accessors. Best static inlines without shouting names.

The existing drivers uses MV_READ/MV_WRITE throughout. I agree it's
ugly but I kept them to minimize the patch size. I plan to submit a
patch to rename them and make these static inline functions as soon as
this set of patches is "in the queue".

> > + */
> > +static void eth_port_uc_addr_get(struct net_device *dev, unsigned char *MacAddr)
> > +{
> > + struct mv64340_private *mp = netdev_priv(dev);
> > + unsigned int port_num = mp->port_num;
> > + u32 MacLow;
> > + u32 MacHigh;
> > +
> > + MacLow = MV_READ(MV64340_ETH_MAC_ADDR_LOW(port_num));
> > + MacHigh = MV_READ(MV64340_ETH_MAC_ADDR_HIGH(port_num));
> > +
> > + MacAddr[5] = (MacLow) & 0xff;
> > + MacAddr[4] = (MacLow >> 8) & 0xff;
> > + MacAddr[3] = (MacHigh) & 0xff;
> > + MacAddr[2] = (MacHigh >> 8) & 0xff;
> > + MacAddr[1] = (MacHigh >> 16) & 0xff;
> > + MacAddr[0] = (MacHigh >> 24) & 0xff;
>
> Please avoid mixed UpperLower case variable names. Also make sure to use
> tabs for indentation again.

I copied this from an existing driver, but I agree and will change.

Thanks,
-Dale
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



[ About Guardian Digital ] - [ Press Center ] - [ Contact Us ] - [ System Activation ] - [ Reseller Info ] - [ Online Store ] - [ Site Map ]
Copyright (c) 2000 - 2004 Guardian Digital, Inc. Linux Lockbox and EnGarde are Trademarks of Guardian Digital, Inc.