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

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

Ben Greear <greearb@candelatech.com> writes:

> On 06/05/2014 11:10 PM, Kalle Valo wrote:
>> Ben Greear <greearb@candelatech.com> writes:
>> 
>>> I did not do this because I figure we will want ethtool support w/out
>>> forcing debugfs to be enabled someday soon.
>> 
>> When we add ethtool support, it's easy to move these back. And then we
>> need to move the code out from debug.c anyway.
>
> Well, it seems like needless churn and makes it harder to follow
> 'git blame' when you move big chunks around.

I don't care about 'git blame', the actual code is far more important to
me. If git blame is so important, someone else can improve 'git blame'
to detect moving code chunks.

> We would not have to move the code out of debug.c, but if you do want
> it moved, lets pick that place now and just put it there to begin
> with.

Code in debug.c should have the data it owns in struct ath10k_debug.

>>> 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.
>
> I think it has to be a spin-lock because the crash dump is gathered
> in the irq handler, so I can't use a mutex as far as I know...
>
> I'll work on adding such a lock today.

I asked why add a _new_ spinlock as ar->data_lock is already a spinlock.

-- 
Kalle Valo

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