Re: [PATCH 5/6] mv643xx_eth: Add support for platform device interface

[Dale Farnsworth]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/