On 2024/8/27 10:59, Brian Norris wrote:
On Mon, Aug 26, 2024 at 6:33 PM Jon Lin <jon.lin@xxxxxxxxxxxxxx> wrote:
On 2024/8/27 6:27, Brian Norris wrote:
It seems like you'd really be served well by
pm_runtime_force_{suspend,resume}() here, and in fact, that's what this
driver used to use before the breaking change (commit
e882575efc77). Why aren't you just going back to using it? (This is the
kind of thing I might expect in your commit message -- reasoning as to
why you're doing what you're doing.)
And in fact, I already submitted a patch that resolves the above problem
and does exactly that:
https://lore.kernel.org/all/20240823214235.1718769-1-briannorris@xxxxxxxxxxxx/
[PATCH] spi: rockchip: Resolve unbalanced runtime PM / system PM handling
Do you see any problem with it?
I have reviewed your submission and although the code has been
simplified, the execution efficiency has decreased. So although it is a
commonly used processing solution for SPI Upstream, I still hope to
retain a more efficiency approach as I submitted.
What do you mean by "efficiency"? You mean because there's
indirection, via the PM runtime framework? If so, I doubt that's a
priority for this piece of functionality -- simplicity is more
important than a function call or two when talking about system
suspend.
Your code is fine, and I have tested it locally. The interface
simplification is indeed in line with the direction of community
development, and your solution can be used as a solution.
Additionally, simplicity has additional benefits -- it heads off
questions that your more complex code doesn't address. For example,
are runtime PM and system PM mutually exclusive? Do we have to
coordinate with any pending autosuspend? (Reading through
https://docs.kernel.org/power/runtime_pm.html, I believe these are not
actually concerns, but it's really not obvious and takes a bit of
reading.) But your patch makes it more likely that runtime and system
PM states get out of sync.
Anyway, if the patches really are equivalent, I suppose it can be the
maintainer's choice as to which is preferable.
Brian
