Re: Missing memory barriers

[Kalle Valo <kvalo@qca.qualcomm.com>]

Avery Pennarun <apenwarr@gmail.com> writes:

> On Thu, Feb 27, 2014 at 7:48 AM, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Avery Pennarun <apenwarr@gmail.com> writes:
>>> - there are definitely some missing memory barriers in here; in a few
>>> cases you can clearly see a write getting done before the read that
>>> came before it.  Looking at the definitions for iowrite32 and
>>> ioread32, and for rmb() and wmb(), we can see that the use of rmb()
>>> and wmb() do not work properly (at least on ARM) when you care about
>>> the ordering between reads and writes.  However, I don't think this
>>> actually causes the problem.
>>
>> Can you tell more about this, please? Did you find out where we are
>> actually doing it wrong?
>
> Sure.  It's been a while since I wrote the above and it was with an
> older version of the ath10k driver, but basically what happens is as
> follows.

[...]

> The bad news is that, while inserting the extra barrier did clean up
> my bus trace, it didn't fix the underlying problem.  When the chip
> dies due to this cold reset operation, the inability to read the
> status register is only a symptom, not the cause.  In the end it's
> harmless that we end up doing the first read before the write
> operation finishes.  What happens isn't what the code says, but I
> don't think that matters in this case.

Thanks for the excellent write up, I understand this better now. I
wasn't expecting that this would fix the cold reset issue, but these
kind of issues should be good to fix anyway. You never know what kind of
bugs they might cause in the future.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k