* Re: [PATCH] RFC syscore: add suspend type to syscore [not found] ` <YBPNAoitmgnTxiqQ@kroah.com> @ 2021-02-04 9:06 ` Ruifeng Zhang 2021-02-04 10:32 ` Greg KH 2021-02-04 13:38 ` Rafael J. Wysocki 0 siblings, 2 replies; 7+ messages in thread From: Ruifeng Zhang @ 2021-02-04 9:06 UTC (permalink / raw) To: Greg KH; +Cc: Rafael J. Wysocki, ruifeng.zhang1, linux-kernel, chunyan.zhang Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > 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 = SYSTEM_SUSPEND; + syscore_suspend(SUSPEND_S2IDLE); sched_clock_suspend(); timekeeping_suspend(); } else { @@ -553,6 +555,7 @@ void tick_unfreeze(void) if (tick_freeze_depth == num_online_cpus()) { timekeeping_resume(); sched_clock_resume(); + syscore_resume(SUSPEND_S2IDLE); system_state = SYSTEM_RUNNING; trace_suspend_resume(TPS("timekeeping_freeze"), smp_processor_id(), false); -- 2.17.1 Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] RFC syscore: add suspend type to syscore 2021-02-04 9:06 ` [PATCH] RFC syscore: add suspend type to syscore Ruifeng Zhang @ 2021-02-04 10:32 ` Greg KH 2021-02-04 13:38 ` Rafael J. Wysocki 1 sibling, 0 replies; 7+ messages in thread From: Greg KH @ 2021-02-04 10:32 UTC (permalink / raw) To: Ruifeng Zhang Cc: Rafael J. Wysocki, ruifeng.zhang1, linux-kernel, chunyan.zhang On Thu, Feb 04, 2021 at 05:06:25PM +0800, Ruifeng Zhang wrote: > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > 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. We can not add things to the kernel for code that is not in the kernel tree itself, you know this. Please don't try to go around this well-known rule. > The GKI requires that no modification of the kernel source is allowed, > so an syscore_s2idle is added for use. I have no idea what "GKI" is with regards to the kernel project, sorry. > 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. Why does that matter? What do you do with that information? > > > 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, I do not understand what you mean here, emails do not name patches :) > add a new s2ildle type for syscore. But why is that needed? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RFC syscore: add suspend type to syscore 2021-02-04 9:06 ` [PATCH] RFC syscore: add suspend type to syscore Ruifeng Zhang 2021-02-04 10:32 ` Greg KH @ 2021-02-04 13:38 ` Rafael J. Wysocki 2021-02-05 10:27 ` Ruifeng Zhang 1 sibling, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2021-02-04 13:38 UTC (permalink / raw) To: Ruifeng Zhang Cc: Greg KH, Rafael J. Wysocki, ruifeng.zhang1, Linux Kernel Mailing List, Chunyan Zhang On Thu, Feb 4, 2021 at 10:07 AM Ruifeng Zhang <ruifeng.zhang0110@gmail.com> wrote: > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > 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. Do that in the platform operations then. Adding the syscore stuff to the suspend-to-idle flow is not an option, sorry. > The GKI requires that no modification of the kernel source is allowed, > so an syscore_s2idle is added for use. The GKI rules are not a mainline kernel concern. > 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. No, this absolutely is a bad idea. > 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 = SYSTEM_SUSPEND; > + syscore_suspend(SUSPEND_S2IDLE); > sched_clock_suspend(); > timekeeping_suspend(); > } else { > @@ -553,6 +555,7 @@ void tick_unfreeze(void) > if (tick_freeze_depth == num_online_cpus()) { > timekeeping_resume(); > sched_clock_resume(); > + syscore_resume(SUSPEND_S2IDLE); > system_state = SYSTEM_RUNNING; > trace_suspend_resume(TPS("timekeeping_freeze"), > smp_processor_id(), false); > -- > 2.17.1 > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RFC syscore: add suspend type to syscore 2021-02-04 13:38 ` Rafael J. Wysocki @ 2021-02-05 10:27 ` Ruifeng Zhang 2021-02-05 11:39 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Ruifeng Zhang @ 2021-02-05 10:27 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Greg KH, ruifeng.zhang1, Linux Kernel Mailing List, Chunyan Zhang, ke.wang, nianfu.bai, orson.zhai Rafael J. Wysocki <rafael@kernel.org> 于2021年2月4日周四 下午9:38写道: > > On Thu, Feb 4, 2021 at 10:07 AM Ruifeng Zhang > <ruifeng.zhang0110@gmail.com> wrote: > > > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > > > 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. > > Do that in the platform operations then. > > Adding the syscore stuff to the suspend-to-idle flow is not an option, sorry. Excause me, I really still want to know the reason. My requirement is that the watchdog need disable when the system s2idle. If don't, the watchdog will bark when system resume. > > > The GKI requires that no modification of the kernel source is allowed, > > so an syscore_s2idle is added for use. > > The GKI rules are not a mainline kernel concern. > > > 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. > > No, this absolutely is a bad idea. > > > 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 = SYSTEM_SUSPEND; > > + syscore_suspend(SUSPEND_S2IDLE); > > sched_clock_suspend(); > > timekeeping_suspend(); > > } else { > > @@ -553,6 +555,7 @@ void tick_unfreeze(void) > > if (tick_freeze_depth == num_online_cpus()) { > > timekeeping_resume(); > > sched_clock_resume(); > > + syscore_resume(SUSPEND_S2IDLE); > > system_state = SYSTEM_RUNNING; > > trace_suspend_resume(TPS("timekeeping_freeze"), > > smp_processor_id(), false); > > -- > > 2.17.1 > > > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RFC syscore: add suspend type to syscore 2021-02-05 10:27 ` Ruifeng Zhang @ 2021-02-05 11:39 ` Rafael J. Wysocki 2021-02-05 20:03 ` Ruifeng Zhang 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2021-02-05 11:39 UTC (permalink / raw) To: Ruifeng Zhang Cc: Rafael J. Wysocki, Greg KH, ruifeng.zhang1, Linux Kernel Mailing List, Chunyan Zhang, ke.wang, nianfu.bai, orson.zhai On Fri, Feb 5, 2021 at 11:28 AM Ruifeng Zhang <ruifeng.zhang0110@gmail.com> wrote: > > Rafael J. Wysocki <rafael@kernel.org> 于2021年2月4日周四 下午9:38写道: > > > > On Thu, Feb 4, 2021 at 10:07 AM Ruifeng Zhang > > <ruifeng.zhang0110@gmail.com> wrote: > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > > > > > 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. > > > > Do that in the platform operations then. > > > > Adding the syscore stuff to the suspend-to-idle flow is not an option, sorry. > Excause me, I really still want to know the reason. The conditions to run syscore operations are: with one CPU online and with disabled interrupts on that CPU. They are not satisfied in the suspend-to-idle flow. Moreover, none of the existing syscore operations need to be executed for suspend-to-idle except for the timekeeping suspend, but this is done for a special reason. > My requirement is that the watchdog need disable when the system s2idle. > If don't, the watchdog will bark when system resume. So disabled it from s2idle_ops->prepare() or suspend_ops->prepare_late() if device suspend is too early for you. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RFC syscore: add suspend type to syscore 2021-02-05 11:39 ` Rafael J. Wysocki @ 2021-02-05 20:03 ` Ruifeng Zhang 0 siblings, 0 replies; 7+ messages in thread From: Ruifeng Zhang @ 2021-02-05 20:03 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Greg KH, ruifeng.zhang1, Linux Kernel Mailing List, Chunyan Zhang, ke.wang, nianfu.bai, orson.zhai Rafael J. Wysocki <rafael@kernel.org> 于2021年2月5日周五 下午7:39写道: > > On Fri, Feb 5, 2021 at 11:28 AM Ruifeng Zhang > <ruifeng.zhang0110@gmail.com> wrote: > > > > Rafael J. Wysocki <rafael@kernel.org> 于2021年2月4日周四 下午9:38写道: > > > > > > On Thu, Feb 4, 2021 at 10:07 AM Ruifeng Zhang > > > <ruifeng.zhang0110@gmail.com> wrote: > > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2021年1月29日周五 下午4:53写道: > > > > > > > > > > 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. > > > > > > Do that in the platform operations then. > > > > > > Adding the syscore stuff to the suspend-to-idle flow is not an option, sorry. > > Excause me, I really still want to know the reason. > > The conditions to run syscore operations are: with one CPU online and > with disabled interrupts on that CPU. They are not satisfied in the > suspend-to-idle flow. Moreover, none of the existing syscore > operations need to be executed for suspend-to-idle except for the > timekeeping suspend, but this is done for a special reason. > > > My requirement is that the watchdog need disable when the system s2idle. > > If don't, the watchdog will bark when system resume. > > So disabled it from s2idle_ops->prepare() or > suspend_ops->prepare_late() if device suspend is too early for you. I will seriously consider your suggestions, thank you very much. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CAJZ5v0iS92CN5MKHu8tOpQR6mOWu=4=PLqN84qXG+v+64Ro99Q@mail.gmail.com>]
* Re: [PATCH] RFC syscore: add suspend type to syscore [not found] ` <CAJZ5v0iS92CN5MKHu8tOpQR6mOWu=4=PLqN84qXG+v+64Ro99Q@mail.gmail.com> @ 2021-02-04 9:09 ` Ruifeng Zhang 0 siblings, 0 replies; 7+ messages in thread From: Ruifeng Zhang @ 2021-02-04 9:09 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Greg Kroah-Hartman, ruifeng.zhang1, Linux Kernel Mailing List, Chunyan Zhang Rafael J. Wysocki <rafael@kernel.org> 于2021年1月29日周五 下午9:50写道: > > On Fri, Jan 29, 2021 at 9:27 AM Ruifeng Zhang > <ruifeng.zhang0110@gmail.com> wrote: > > > > From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com> > > > > Suspend type contains s2ram and s2idle, but syscore is only > > available for S2RAM. > > > > S2idle requires a similar feature, > > No, it doesn't. Why s2idle don't requires this feature ? I'm need use it for vendor watchdog driver. Rafael J. Wysocki <rafael@kernel.org> 于2021年1月29日周五 下午9:50写道: > > On Fri, Jan 29, 2021 at 9:27 AM Ruifeng Zhang > <ruifeng.zhang0110@gmail.com> wrote: > > > > From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com> > > > > Suspend type contains s2ram and s2idle, but syscore is only > > available for S2RAM. > > > > S2idle requires a similar feature, > > No, it doesn't. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-05 20:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210129082726.19406-1-ruifeng.zhang0110@gmail.com> [not found] ` <YBPNAoitmgnTxiqQ@kroah.com> 2021-02-04 9:06 ` [PATCH] RFC syscore: add suspend type to syscore Ruifeng Zhang 2021-02-04 10:32 ` Greg KH 2021-02-04 13:38 ` Rafael J. Wysocki 2021-02-05 10:27 ` Ruifeng Zhang 2021-02-05 11:39 ` Rafael J. Wysocki 2021-02-05 20:03 ` Ruifeng Zhang [not found] ` <CAJZ5v0iS92CN5MKHu8tOpQR6mOWu=4=PLqN84qXG+v+64Ro99Q@mail.gmail.com> 2021-02-04 9:09 ` Ruifeng Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).