[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<DU0PR04MB9417FFF8A9FB5018125C524688082@DU0PR04MB9417.eurprd04.prod.outlook.com>
Date: Tue, 16 Apr 2024 12:34:14 +0000
From: Peng Fan <peng.fan@....com>
To: Cristian Marussi <cristian.marussi@....com>
CC: "Peng Fan (OSS)" <peng.fan@....nxp.com>, "sudeep.holla@....com"
<sudeep.holla@....com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] firmware: arm_scmi: power_control: support suspend
command
> Subject: Re: [PATCH] firmware: arm_scmi: power_control: support suspend
> command
>
> On Tue, Apr 16, 2024 at 07:01:42AM +0000, Peng Fan wrote:
> > Hi Cristian,
> >
>
> Hi,
>
> > > Subject: Re: [PATCH] firmware: arm_scmi: power_control: support
> > > suspend command
> > >
> > > On Mon, Apr 15, 2024 at 06:11:41PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@....com>
> > > >
> > > > Support System suspend notification. Using a work struct to call
> > > > pm_suspend. There is no way to pass suspend level to pm_suspend,
> > > > so use MEM as of now.
> > > >
> > >
> > > Hi Peng,
> > >
> > > > Signed-off-by: Peng Fan <peng.fan@....com>
> > > > ---
> > > > .../firmware/arm_scmi/scmi_power_control.c | 20
> ++++++++++++++++++-
> > > > 1 file changed, 19 insertions(+), 1 deletion(-)
> > > >
> > >
> > > This LGTM in general, the only obsservation here is that while on
> > > shutdown by issuing a orderly_reboot() we in fact ask PID_1 to start
> > > a shutdown from
> >
> > Would you please share why PID_1 is involved here? orderly_reboot is
> > just schedule a work.
>
> For the shutdown case, orderly_reboot related scheduled work ends up in a
> call to /sbin/reboot via usermodehelper kernel facilities, so that, depending
> on what PID_1 you have and how it is configured, PID_1 does have a chance
> to perform some additional task (if configured) before triggering the real final
> shutdown....this is done because the SCMI Notification is supposed to be a
> graceful request, so we dont just kernel_poweroff or similar.
>
> As an example with SystemD PID_1 /sbin/reboot is just a link to systemctl
> and you can place whatever you want in
>
> /usr/lib/systemd/system-shutdown/
>
> and that it will executed by systemctl before kernel shutdown is really
> triggered.
Thanks for explaining the details. I see that in kernel/reboot.c
>
> > > userspace, when triggering a system suspend with pm_suspend() we do
> > > not involve userspace in the process, but I dont think there is another way
> indeed.
> >
> > Userspace process should not be involved, otherwise the freezing
> > process will never finish, and suspend will abort.
> >
>
> Similarly in the suspend case when you initiate it from userspace (systemcl
> suspend) you can place something in /usr/lib/systemd/system-sleep/ and
> have it executed before suspend is passwed on to the kernel, BUT in our case
> we are not passing through userspace and it seems there is no way to do it,
> indeed, like we do for shutdown with orderly_reboot(). Moreover userspace,
> as Sudeep mentioned in the other thread, could be configured to trigger a
> different type of suspend, it it was involved at all in this process.
>
> But, as said, I dont think there is a way to trigger a userspace suspennd from
> jernel like we do for shutdown/reboot...I'll try to investigate a bit more.
Thanks for helping.
>
> Anyway the change seems good to me as I said.
ok, I will post v2 with commit log updated later, waiting some feedback from
others.
Thanks
Peng.
>
> Thanks,
> Cristian
Powered by blists - more mailing lists