On Wed, Nov 11, 2015 at 12:34 AM, Måns Rullgård <m...@mansr.com> wrote:
> 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.
But you unconditionally use entire val value which might bring bits
outside of mask.
>> 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.
OK.
>>> + 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.
--
With Best Regards,
Andy Shevchenko
--
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
