Re: [PATCH] spi: qup: skip clk_disable_unprepare if the device is already runtime suspended

From: Sudeep Holla
Date: Fri Sep 02 2016 - 06:45:17 EST




On 02/09/16 10:38, Mark Brown wrote:
On Fri, Sep 02, 2016 at 09:42:04AM +0100, Sudeep Holla wrote:
On 01/09/16 21:29, Mark Brown wrote:
On Thu, Aug 25, 2016 at 01:33:28PM +0100, Sudeep Holla wrote:

CPU: 3 PID: 1593 Comm: bash Tainted: G W 4.8.0-rc3 #14
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
PC is at clk_core_unprepare+0x80/0x90
LR is at clk_unprepare+0x28/0x40
pc : [<ffff0000086eecf0>] lr : [<ffff0000086f0c58>] pstate: 60000145

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative then it's
usually better to pull out the relevant sections.

I removed most of the addresses and just retained the symbols(somehow
the last line with pc and lr was left unintentionally). While you may
have the above opinion, other maintainers may differ. In future, I will
try to add it as a note just to describe the issue.

Oh, *that's* why it looked so weird. Removing the addresses doesn't
help here, the issue isn't that the addresses are confusing it's that
you had a tiny commit message dwarfed by the backtrace preamble then a
screenful of call stack which conveyed no meaningful information,
including not just the entire callback path for a suspend (which doesn't
tell us anything really, especially beyond the first frame) and going on
to show the entire call stack from the sysfs write you used to trigger
suspend which is even less relevant.

This gives us 30 lines or so of splat (more than a screenful) for five
lines of actual content with the important bit which describes what the
change is supposed to be doing buried at the bottom. That's a really
bad signal to noise ratio. What would've been better would be
explaining why the change you are making fixes the problem.


Agreed.

--
Regards,
Sudeep