Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: Måns Rullgård
Date: Tue Nov 10 2015 - 18:07:33 EST

Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes:

> On Wed, Nov 11, 2015 at 12:34 AM, MÃns RullgÃrd <mans@xxxxxxxxx> wrote:
>> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> 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.
> But you unconditionally use entire val value which might bring bits
> outside of mask.

Very well, I'll apply the mask to both then.

>>>> + 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.
> Standalone wmb() doesn't make sense.

It does if you need to enforce ordering between normal and I/O memory.
In fact, since the descriptor is filled in using normal memory accesses,
my understanding is that mmiowb() would be insufficient here. The
comment could be improved, however.

MÃns RullgÃrd
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at