Re: [PATCH v4 1/2] driver: reset: spacemit-p1: add driver for poweroff/reboot

From: Yixun Lan <dlan@gentoo.org>
To: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: Andreas Schwab <schwab@suse.de>, Yao Zi <ziyao@disroot.org>,
	Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	Aurelien Jarno <aurelien@aurel32.net>,
	linux-kernel@vger.kernel.org, Lee Jones <lee@kernel.org>,
	Sebastian Reichel <sre@kernel.org>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	linux-riscv@lists.infradead.org, spacemit@lists.linux.dev,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH v4 1/2] driver: reset: spacemit-p1: add driver for poweroff/reboot
Date: Mon, 27 Oct 2025 18:31:47 +0800	[thread overview]
Message-ID: <20251027103147-GYA1541054@gentoo.org> (raw)
In-Reply-To: <CAJM55Z9w0pnFmVjJKJNMs1iqOxoL=YpkBu0L7NuUZ_0ipMtQAQ@mail.gmail.com>

Hi 

On 03:17 Mon 27 Oct     , Emil Renner Berthing wrote:
> Quoting Yao Zi (2025-10-27 10:24:30)
> > On Mon, Oct 27, 2025 at 10:03:44AM +0100, Andreas Schwab wrote:
> > > On Okt 27 2025, Yao Zi wrote:
> > > > On Mon, Oct 27, 2025 at 11:20:33AM +0800, Troy Mitchell wrote:
> > > >> On Sun, Oct 26, 2025 at 11:41:14PM +0100, Aurelien Jarno wrote:
> > > >> > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> > > >> > index 8248895ca9038..61c16f3d5abc7 100644
> > > >> > --- a/drivers/power/reset/Kconfig
> > > >> > +++ b/drivers/power/reset/Kconfig
> > > >> > @@ -283,6 +283,15 @@ config POWER_RESET_KEYSTONE
> > > >> >          help
> > > >> >            Reboot support for the KEYSTONE SoCs.
> > > >> >
> > > >> > +config POWER_RESET_SPACEMIT_P1
> > > >> > +        tristate "SpacemiT P1 poweroff and reset driver"
> > > >> > +        depends on ARCH_SPACEMIT || COMPILE_TEST
> > > >> > +        depends on MFD_SPACEMIT_P1
> > > >> > +        default m
> > > >> default m if ARCH_SPACEMIT? Or default ARCH_SPACEMIT?
> > > >> I believe that reboot and shutdown are actually essential functionalities,
> > > >> so it might make more sense: default ARCH_SPACEMIT?
> > > >
> > > > I don't think there's anything preventing it to be built as module by
> > > > default: even though it's "essential", it's unnecessary during kernel
> > > > and userspace startup, thus I see no reason to build it in the image.
> > >
> > > Wouldn't it be needed in a reboot-on-panic situation?
> >
> > Oops, yeah, I missed this stuff. Seems systemd automatic boot assessment
> > could switch to another boot option if one fails to boot. And if it's
> > caused by a (very early) kernel panic, then reboot support does play a
> > part here.
> 
> But if systemd is running then you've at least got as far as the initramfs,
> and have the module available. So I don't see the problem.
> 
In rare case, if got kernel panic before load this module, then we
should really fix it instead.. Besides, there is no restriction to prevent
user to make this driver as built-in, right?

So I think this isn't really a big problem either
-- 
Yixun Lan (dlan)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv