Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210129082726.19406-1-ruifeng.zhang0110@gmail.com> <YBPNAoitmgnTxiqQ@kroah.com>
In-Reply-To: <YBPNAoitmgnTxiqQ@kroah.com>
From:   Ruifeng Zhang <ruifeng.zhang0110@gmail.com>
Date:   Thu, 4 Feb 2021 17:06:25 +0800
Message-ID: <CAG7+-3NGUgryomKB=_W=p9nADPJVnns_x9oWpAFCEei7RiyrOA@mail.gmail.com>
Subject: Re: [PATCH] RFC syscore: add suspend type to syscore
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>, ruifeng.zhang1@unisoc.com,
        linux-kernel@vger.kernel.org, chunyan.zhang@unisoc.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Greg KH <gregkh@linuxfoundation.org> =E4=BA=8E2021=E5=B9=B41=E6=9C=8829=E6=
=97=A5=E5=91=A8=E4=BA=94 =E4=B8=8B=E5=8D=884:53=E5=86=99=E9=81=93=EF=BC=9A
>
> On Fri, Jan 29, 2021 at 04:27:26PM +0800, Ruifeng Zhang wrote:
> > From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
> >
> > Suspend type contains s2ram and s2idle, but syscore is only
> > available for S2RAM.
>
> Who else needs this?
In the s2idle suspend and resume, some vendors want to do some
things, for example the vendor implemented the watchdog driver.

The GKI requires that no modification of the kernel source is allowed,
so an syscore_s2idle is added for use.

The reason device_suspend was not chosen was that I wanted it to
monitor for longer periods, such as between device_suspend and
syscore_suspend.
>
> > S2idle requires a similar feature, so a new parameter
> > "enum suspend_type" is added to distinguish it.
>
> Who requires this export?
>
> I don't see a user of this new code/api in this patch, so why would it
> be accepted?
>
> Also, you are doing many different things in the same patch, please
> break this up into a patch series where you only do one logical change
> at a time.
I think it's only one things in patch
0001-RFC-syscore-add-suspend-type-to-syscore.patch,
add a new s2ildle type for syscore.
>
> thanks,
>
> greg k-h

From 1abd09045639dafdbf713514d4f1323b572dd2ec Mon Sep 17 00:00:00 2001
From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
Date: Thu, 4 Feb 2021 13:29:56 +0800
Subject: [PATCH 2/2] RFC time: add syscore suspend ops to s2idle

Some vendors need do more things when s2idle.

The required GKI does not allow modification of the
kernel source code, so provide the syscore operation
interface.

Signed-off-by: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
---
 kernel/time/tick-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 9d3a22510bab..8c4509250456 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -11,6 +11,7 @@
 #include <linux/err.h>
 #include <linux/hrtimer.h>
 #include <linux/interrupt.h>
+#include <linux/list.h>
 #include <linux/nmi.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
@@ -528,6 +529,7 @@ void tick_freeze(void)
        trace_suspend_resume(TPS("timekeeping_freeze"),
                     smp_processor_id(), true);
        system_state =3D SYSTEM_SUSPEND;
+       syscore_suspend(SUSPEND_S2IDLE);
        sched_clock_suspend();
        timekeeping_suspend();
    } else {
@@ -553,6 +555,7 @@ void tick_unfreeze(void)
    if (tick_freeze_depth =3D=3D num_online_cpus()) {
        timekeeping_resume();
        sched_clock_resume();
+       syscore_resume(SUSPEND_S2IDLE);
        system_state =3D SYSTEM_RUNNING;
        trace_suspend_resume(TPS("timekeeping_freeze"),
                     smp_processor_id(), false);
--
2.17.1

Greg KH <gregkh@linuxfoundation.org> =E4=BA=8E2021=E5=B9=B41=E6=9C=8829=E6=
=97=A5=E5=91=A8=E4=BA=94 =E4=B8=8B=E5=8D=884:53=E5=86=99=E9=81=93=EF=BC=9A
>
> On Fri, Jan 29, 2021 at 04:27:26PM +0800, Ruifeng Zhang wrote:
> > From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
> >
> > Suspend type contains s2ram and s2idle, but syscore is only
> > available for S2RAM.
>
> Who else needs this?
>
> > S2idle requires a similar feature, so a new parameter
> > "enum suspend_type" is added to distinguish it.
>
> Who requires this export?
>
> I don't see a user of this new code/api in this patch, so why would it
> be accepted?
>
> Also, you are doing many different things in the same patch, please
> break this up into a patch series where you only do one logical change
> at a time.
>
> thanks,
>
> greg k-h