Re: [PATCH net-next v2 06/10] net: bcmgenet: add main driver file

[Joe Perches <joe@perches.com>]

On Thu, 2014-02-13 at 11:35 +0100, Francois Romieu wrote:
> Florian Fainelli <f.fainelli@gmail.com> :
> [...]
> > diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
[]
> > +static int bcmgenet_set_rx_csum(struct net_device *dev,
> > +				netdev_features_t wanted)
> > +{
> > +	struct bcmgenet_priv *priv = netdev_priv(dev);
> > +	u32 rbuf_chk_ctrl;
> > +	int rx_csum_en;
> > +
> > +	rx_csum_en = !!(wanted & NETIF_F_RXCSUM);
> 
> It's a bool.

It could be a bool.

The struct definition has:

+	unsigned int desc_rxchk_en;

but perhaps a lot of these members could be bool.

It'd be nicer if the variable types were the same.

> > +	spin_lock_bh(&priv->bh_lock);
> > +	rbuf_chk_ctrl = bcmgenet_rbuf_readl(priv, RBUF_CHK_CTRL);
> > +
> > +	/* enable rx checksumming */
> > +	if (!rx_csum_en)
> > +		rbuf_chk_ctrl &= ~RBUF_RXCHK_EN;
> > +	else
> > +		rbuf_chk_ctrl |= RBUF_RXCHK_EN;

This is more normally written with a positive test like:

	if (rx_csum_en)
		rbuf_chk_ctrl |= RBUF_RXCHK_EN;
	else
		rbuf_chk_ctrl &= RBUF_RXCHK_EN;

> > +	priv->desc_rxchk_en = rx_csum_en;