Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220906143108.1749183-1-kai.heng.feng@canonical.com> <526160bf-f0f8-a861-55d4-1ed5437c9009@gmail.com>
In-Reply-To: <526160bf-f0f8-a861-55d4-1ed5437c9009@gmail.com>
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
Date:   Tue, 13 Sep 2022 08:11:48 +0800
Message-ID: <CAAd53p72POknU3tWyNBtsbZ_rEPVz+FBqi46QVvG3rhEO7ysWQ@mail.gmail.com>
Subject: Re: [PATCH v2] PM: ACPI: reboot: Reinstate S5 for reboot
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     rafael.j.wysocki@intel.com, Josef Bacik <josef@toxicpanda.com>,
        Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Petr Mladek <pmladek@suse.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        tangmeng <tangmeng@uniontech.com>,
        YueHaibing <yuehaibing@huawei.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Sep 7, 2022 at 4:07 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 06.09.2022 17:31, Kai-Heng Feng =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
> > Commit d60cd06331a3 ("PM: ACPI: reboot: Use S5 for reboot") caused Dell
> > PowerEdge r440 hangs at boot.
> >
> > The issue is fixed by commit 2ca1c94ce0b6 ("tg3: Disable tg3 device on
> > system reboot to avoid triggering AER"), so reinstate the patch again.
> >
> > Cc: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > v2:
> >  - Use do_kernel_power_off_prepare() instead.
> >
> >  kernel/reboot.c | 55 +++++++++++++++++++++++++------------------------
> >  1 file changed, 28 insertions(+), 27 deletions(-)
> >
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index 3c35445bf5ad3..39cbb45afc54a 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -243,28 +243,6 @@ void migrate_to_reboot_cpu(void)
> >       set_cpus_allowed_ptr(current, cpumask_of(cpu));
> >  }
> >
> > -/**
> > - *   kernel_restart - reboot the system
> > - *   @cmd: pointer to buffer containing command to execute for restart
> > - *           or %NULL
> > - *
> > - *   Shutdown everything and perform a clean reboot.
> > - *   This is not safe to call in interrupt context.
> > - */
> > -void kernel_restart(char *cmd)
> > -{
> > -     kernel_restart_prepare(cmd);
> > -     migrate_to_reboot_cpu();
> > -     syscore_shutdown();
> > -     if (!cmd)
> > -             pr_emerg("Restarting system\n");
> > -     else
> > -             pr_emerg("Restarting system with command '%s'\n", cmd);
> > -     kmsg_dump(KMSG_DUMP_SHUTDOWN);
> > -     machine_restart(cmd);
> > -}
> > -EXPORT_SYMBOL_GPL(kernel_restart);
> > -
> >  static void kernel_shutdown_prepare(enum system_states state)
> >  {
> >       blocking_notifier_call_chain(&reboot_notifier_list,
> > @@ -301,6 +279,34 @@ static BLOCKING_NOTIFIER_HEAD(power_off_prep_handl=
er_list);
> >   */
> >  static ATOMIC_NOTIFIER_HEAD(power_off_handler_list);
> >
> > +static void do_kernel_power_off_prepare(void)
> > +{
> > +     blocking_notifier_call_chain(&power_off_prep_handler_list, 0, NUL=
L);
> > +}
> > +
> > +/**
> > + *   kernel_restart - reboot the system
> > + *   @cmd: pointer to buffer containing command to execute for restart
> > + *           or %NULL
> > + *
> > + *   Shutdown everything and perform a clean reboot.
> > + *   This is not safe to call in interrupt context.
> > + */
> > +void kernel_restart(char *cmd)
> > +{
> > +     kernel_restart_prepare(cmd);
> > +     do_kernel_power_off_prepare();
>
> Looks like an abuse to me. Adding new SYS_OFF_MODE_RESTART_PREPARE and
> updating acpi_sleep_init() to use it should be a better solution.

Thanks, will send a new one based on your comment.

Kai-Heng