Andy Shevchenko <andy.shevche...@gmail.com> writes:
>> +static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
>> + u32 mask, u32 val)
>> +{
>> + u32 old = nb8800_readb(priv, reg);
>> + u32 new = (old & ~mask) | val;
>
> Shoudn't be "… | (val & mask);" ?
No, it's meant to replace the bits in mask with the corresponding bits
from val.
> + empty line?
>
>> + if (new != old)
>> + nb8800_writeb(priv, reg, new);
>> +}
>> +
[...]
>> +static void nb8800_receive(struct net_device *dev, int i, int len)
>
> unsigned int i ?
> len as well?
Does it matter? The values are nowhere near overflowing signed int.
[...]
>> + /* If a packet arrived after we last checked but
>> + * before writing RX_ITR, the interrupt will be
>> + * delayed, so we retrieve it now. */
>
> Block comments usually
> /*
> * text
> */
Documentation/CodingStyle says net/ and drivers/net/ are special, though
currently a mix of styles can be found. Personally, I don't
particularly care.
> Can be longer lines?
Still won't fit on two lines.
>> + if (priv->rx_descs[next].report)
>> + goto again;
>> +
>> + napi_complete_done(napi, work);
>> + }
>> +
>> + return work;
>> +}
>> +
>> +static void nb8800_tx_dma_start(struct net_device *dev)
>> +{
>> + struct nb8800_priv *priv = netdev_priv(dev);
>> + struct nb8800_tx_buf *txb;
>> + u32 txc_cr;
>> +
>> + txb = &priv->tx_bufs[priv->tx_queue];
>> + if (!txb->ready)
>> + return;
>> +
>> + txc_cr = nb8800_readl(priv, NB8800_TXC_CR);
>> + if (txc_cr & TCR_EN)
>> + return;
>> +
>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>> + wmb(); /* ensure desc addr is written before starting DMA */
>
> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?
Possibly.
>> + nb8800_writel(priv, NB8800_TXC_CR, txc_cr | TCR_EN);
>> +
>> + priv->tx_queue = (priv->tx_queue + txb->chain_len) % TX_DESC_COUNT;
>> +}
--
Måns Rullgård
m...@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
