linux-kernel - Re: [PATCH] spi: rockchip: Avoid redundant clock disable in pm operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+ASDXMjN7dVUQWgfMULcD2KgF448-q=Ue2+MYUftK6Ra8MWhw@mail.gmail.com>
Date: Mon, 26 Aug 2024 19:59:53 -0700
From: Brian Norris <briannorris@...omium.org>
To: Jon Lin <jon.lin@...k-chips.com>
Cc: broonie@...nel.org, linux-rockchip@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, heiko@...ech.de, 
	linux-arm-kernel@...ts.infradead.org, linux-spi@...r.kernel.org
Subject: Re: [PATCH] spi: rockchip: Avoid redundant clock disable in pm operation

On Mon, Aug 26, 2024 at 6:33 PM Jon Lin <jon.lin@...k-chips.com> 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@chromium.org/
> > [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.

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