The suspend_restore_csrs is called in both __hibernate_cpu_resume
and the `else` of subsequent swsusp_arch_suspend.
Removing the first call makes both suspend_{save,restore}_csrs
left in swsusp_arch_suspend for clean code.
Signed-off-by: Song Shuai <[email protected]>
Signed-off-by: Song Shuai <[email protected]>
---
arch/riscv/kernel/hibernate-asm.S | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
index 5c76671c7e15..d698dd7df637 100644
--- a/arch/riscv/kernel/hibernate-asm.S
+++ b/arch/riscv/kernel/hibernate-asm.S
@@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
REG_L a0, hibernate_cpu_context
- suspend_restore_csrs
suspend_restore_regs
/* Return zero value. */
--
2.20.1
looks good to me.
Thanks
Regards
Jee Heng
> -----Original Message-----
> From: Song Shuai <[email protected]>
> Sent: Friday, May 19, 2023 6:29 PM
> To: [email protected]; [email protected]; [email protected]; [email protected]; Mason Huo
> <[email protected]>; Leyfoon Tan <[email protected]>; [email protected]; JeeHeng Sia
> <[email protected]>
> Cc: [email protected]; [email protected]; Song Shuai <[email protected]>
> Subject: [PATCH] riscv: hibernation: Remove duplicate call of suspend_restore_csrs
>
> The suspend_restore_csrs is called in both __hibernate_cpu_resume
> and the `else` of subsequent swsusp_arch_suspend.
>
> Removing the first call makes both suspend_{save,restore}_csrs
> left in swsusp_arch_suspend for clean code.
>
> Signed-off-by: Song Shuai <[email protected]>
> Signed-off-by: Song Shuai <[email protected]>
> ---
> arch/riscv/kernel/hibernate-asm.S | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
> index 5c76671c7e15..d698dd7df637 100644
> --- a/arch/riscv/kernel/hibernate-asm.S
> +++ b/arch/riscv/kernel/hibernate-asm.S
> @@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
>
> REG_L a0, hibernate_cpu_context
>
> - suspend_restore_csrs
> suspend_restore_regs
Good catch. This function is invoked twice to restore the CSRs. I am good with removing this function from here.
>
> /* Return zero value. */
> --
> 2.20.1
Hey,
On Fri, May 19, 2023 at 11:14:27AM +0000, JeeHeng Sia wrote:
> > The suspend_restore_csrs is called in both __hibernate_cpu_resume
> > and the `else` of subsequent swsusp_arch_suspend.
> >
> > Removing the first call makes both suspend_{save,restore}_csrs
> > left in swsusp_arch_suspend for clean code.
> >
> > Signed-off-by: Song Shuai <[email protected]>
> > Signed-off-by: Song Shuai <[email protected]>
BTW, why the two email addresses? The tinylab one here seems entirely
redundant - unless it is your employer, in which case should it be the
only SoB address & also in the author field?
Also, Fixes tag?
> > ---
> > arch/riscv/kernel/hibernate-asm.S | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
> > index 5c76671c7e15..d698dd7df637 100644
> > --- a/arch/riscv/kernel/hibernate-asm.S
> > +++ b/arch/riscv/kernel/hibernate-asm.S
> > @@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
> >
> > REG_L a0, hibernate_cpu_context
> >
> > - suspend_restore_csrs
> > suspend_restore_regs
> Good catch. This function is invoked twice to restore the CSRs.
> I am good with removing this function from here.
If that's a review, then please either give an R-b or A-b tag :)
Thanks,
Conor.
On Fri, May 19, 2023 at 12:28:07PM +0100, Conor Dooley wrote:
> Hey,
>
> On Fri, May 19, 2023 at 11:14:27AM +0000, JeeHeng Sia wrote:
>
> > > The suspend_restore_csrs is called in both __hibernate_cpu_resume
> > > and the `else` of subsequent swsusp_arch_suspend.
> > >
> > > Removing the first call makes both suspend_{save,restore}_csrs
> > > left in swsusp_arch_suspend for clean code.
It took me embarrassingly long to wrap my head around the control flow
here again. I'm not sure that I agree with you that splitting the calls
between asm & c is cleaner, but whatever:
Reviewed-by: Conor Dooley <[email protected]>
> > >
> > > Signed-off-by: Song Shuai <[email protected]>
> > > Signed-off-by: Song Shuai <[email protected]>
>
> BTW, why the two email addresses? The tinylab one here seems entirely
> redundant - unless it is your employer, in which case should it be the
> only SoB address & also in the author field?
Deja vu with my questioning your email address, but does your
tinylab address actually repeat "shuai"?
> Also, Fixes tag?
>
> > > ---
> > > arch/riscv/kernel/hibernate-asm.S | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
> > > index 5c76671c7e15..d698dd7df637 100644
> > > --- a/arch/riscv/kernel/hibernate-asm.S
> > > +++ b/arch/riscv/kernel/hibernate-asm.S
> > > @@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
> > >
> > > REG_L a0, hibernate_cpu_context
> > >
> > > - suspend_restore_csrs
> > > suspend_restore_regs
>
> > Good catch. This function is invoked twice to restore the CSRs.
> > I am good with removing this function from here.
>
> If that's a review, then please either give an R-b or A-b tag :)
>
> Thanks,
> Conor.
> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Saturday, May 20, 2023 4:07 AM
> To: Conor Dooley <[email protected]>
> Cc: JeeHeng Sia <[email protected]>; Song Shuai <[email protected]>; [email protected];
> [email protected]; [email protected]; Mason Huo <[email protected]>; Leyfoon Tan
> <[email protected]>; [email protected]; [email protected]; [email protected]; Song
> Shuai <[email protected]>
> Subject: Re: [PATCH] riscv: hibernation: Remove duplicate call of suspend_restore_csrs
>
> On Fri, May 19, 2023 at 12:28:07PM +0100, Conor Dooley wrote:
> > Hey,
> >
> > On Fri, May 19, 2023 at 11:14:27AM +0000, JeeHeng Sia wrote:
> >
> > > > The suspend_restore_csrs is called in both __hibernate_cpu_resume
> > > > and the `else` of subsequent swsusp_arch_suspend.
> > > >
> > > > Removing the first call makes both suspend_{save,restore}_csrs
> > > > left in swsusp_arch_suspend for clean code.
>
> It took me embarrassingly long to wrap my head around the control flow
> here again. I'm not sure that I agree with you that splitting the calls
> between asm & c is cleaner, but whatever:
> Reviewed-by: Conor Dooley <[email protected]>
>
> > > >
> > > > Signed-off-by: Song Shuai <[email protected]>
> > > > Signed-off-by: Song Shuai <[email protected]>
> >
> > BTW, why the two email addresses? The tinylab one here seems entirely
> > redundant - unless it is your employer, in which case should it be the
> > only SoB address & also in the author field?
>
> Deja vu with my questioning your email address, but does your
> tinylab address actually repeat "shuai"?
>
> > Also, Fixes tag?
> >
> > > > ---
> > > > arch/riscv/kernel/hibernate-asm.S | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
> > > > index 5c76671c7e15..d698dd7df637 100644
> > > > --- a/arch/riscv/kernel/hibernate-asm.S
> > > > +++ b/arch/riscv/kernel/hibernate-asm.S
> > > > @@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
> > > >
> > > > REG_L a0, hibernate_cpu_context
> > > >
> > > > - suspend_restore_csrs
> > > > suspend_restore_regs
> >
> > > Good catch. This function is invoked twice to restore the CSRs.
> > > I am good with removing this function from here.
> >
> > If that's a review, then please either give an R-b or A-b tag :)
Reviewed-by: JeeHeng Sia <[email protected] >
> >
> > Thanks,
> > Conor.
>
Conor Dooley <[email protected]> 于2023年5月19日周五 20:07写道：
>
> On Fri, May 19, 2023 at 12:28:07PM +0100, Conor Dooley wrote:
> > Hey,
> >
> > On Fri, May 19, 2023 at 11:14:27AM +0000, JeeHeng Sia wrote:
> >
> > > > The suspend_restore_csrs is called in both __hibernate_cpu_resume
> > > > and the `else` of subsequent swsusp_arch_suspend.
> > > >
> > > > Removing the first call makes both suspend_{save,restore}_csrs
> > > > left in swsusp_arch_suspend for clean code.
>
> It took me embarrassingly long to wrap my head around the control flow
> here again. I'm not sure that I agree with you that splitting the calls
> between asm & c is cleaner, but whatever:
> Reviewed-by: Conor Dooley <[email protected]>
>
> > > >
> > > > Signed-off-by: Song Shuai <[email protected]>
> > > > Signed-off-by: Song Shuai <[email protected]>
> >
> > BTW, why the two email addresses? The tinylab one here seems entirely
> > redundant - unless it is your employer, in which case should it be the
> > only SoB address & also in the author field?
>
> Deja vu with my questioning your email address, but does your
> tinylab address actually repeat "shuai"?
>
Yes, that's my full name.
As you noticed, I'm switching my email from Gmail to Tinylab but did
something wrong.
Thanks for your correction, I'll re-send this patch with your
suggestions applied.
> > Also, Fixes tag?
> >
> > > > ---
> > > > arch/riscv/kernel/hibernate-asm.S | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/arch/riscv/kernel/hibernate-asm.S b/arch/riscv/kernel/hibernate-asm.S
> > > > index 5c76671c7e15..d698dd7df637 100644
> > > > --- a/arch/riscv/kernel/hibernate-asm.S
> > > > +++ b/arch/riscv/kernel/hibernate-asm.S
> > > > @@ -28,7 +28,6 @@ ENTRY(__hibernate_cpu_resume)
> > > >
> > > > REG_L a0, hibernate_cpu_context
> > > >
> > > > - suspend_restore_csrs
> > > > suspend_restore_regs
> >
> > > Good catch. This function is invoked twice to restore the CSRs.
> > > I am good with removing this function from here.
> >
> > If that's a review, then please either give an R-b or A-b tag :)
> >
> > Thanks,
> > Conor.
>
>
--
Thanks,
Song
Hello:
This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:
On Fri, 19 May 2023 18:29:08 +0800 you wrote:
> The suspend_restore_csrs is called in both __hibernate_cpu_resume
> and the `else` of subsequent swsusp_arch_suspend.
>
> Removing the first call makes both suspend_{save,restore}_csrs
> left in swsusp_arch_suspend for clean code.
>
> Signed-off-by: Song Shuai <[email protected]>
> Signed-off-by: Song Shuai <[email protected]>
>
> [...]
Here is the summary with links:
- riscv: hibernation: Remove duplicate call of suspend_restore_csrs
https://git.kernel.org/riscv/c/c6399b893043
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html