On 06/07/2014 05:55 AM, Kalle Valo wrote:
Ben Greear <greearb@xxxxxxxxxxxxxxx> writes:
On 06/05/2014 11:10 PM, Kalle Valo wrote:
Ben Greear <greearb@xxxxxxxxxxxxxxx> 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.
Ok, I'll move the data structs to the debug logic for next version of the patch.
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.
Surely we do not want to impede traffic flow just to dump debug info?
And, it is easier to review a specific spinlock rather than use one big
global-ish lock and have to review every use of that lock for issues.
My -v2 had a new spinlock for this new debug data. I'll move it into the
debug struct with the other new data fields for next version.
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
