Re: [PATCH 1/4] ath10k: provide firmware crash info via debugfs.

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

Michal Kazior <michal.kazior@tieto.com> writes:

> On 6 June 2014 08:10, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Ben Greear <greearb@candelatech.com> writes:
> [...]
>>> I'm a bit leery of adding spin-locks in the dump routine just for
>>> this, but I can add and use a new spin-lock if you prefer.
>>
>> Why a new spinlock? I didn't review the locking requirements, but I
>> would first check ar->data_lock can be used.
>>
>>> If so, any idea if we can do the reads of the target's memory while
>>> holding a spin-lock, or would I need some temporary buffers and only
>>> lock while copying that in to the storage in the 'ar'?
>>
>> I don't see why you would need special locks for reading target's
>> memory. If there is something needed, pci.c should handle that. Michal?
>
> By definition the diagnostic window access must be serialized. We
> don't do this with locks now but rely ON driver states/sequences. We
> might have some problems lurking there already but I'd need to analyze
> it to tell for sure.

Should that serialisation happen within pci.c?

> Calling pci_diag_* functions while holding a spinlock should otherwise
> be fine because these functions use mdelay() and poll the diagnostic
> window copy engine pipe.
>
> Using ar->data_lock for a pci transport specific requirement (the
> diagnostic window) seems wrong though.

I agree. I did not mean using data_lock to serialise the pci code.

-- 
Kalle Valo