This driver implements poweroff/reboot support for the SpacemiT P1 PMIC
chip, which is commonly paired with the SpacemiT K1 SoC.
The SpacemiT P1 support is implemented as a MFD driver, so the access is
done directly through the regmap interface. Reboot or poweroff is
triggered by setting a specific bit in a control register, which is
automatically cleared by the hardware afterwards.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
v2:
- Replace the "select" by a "depends on"
- Remove outdated Reviewed-by
drivers/power/reset/Kconfig | 9 +++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++
3 files changed, 98 insertions(+)
create mode 100644 drivers/power/reset/spacemit-p1-reboot.c
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
+ help
+ This driver supports power-off and reset operations for the SpacemiT
+ P1 PMIC.
+
config POWER_RESET_SYSCON
bool "Generic SYSCON regmap reset driver"
depends on OF
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 51da87e05ce76..0e4ae6f6b5c55 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
obj-$(CONFIG_POWER_RESET_REGULATOR) += regulator-poweroff.o
obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
+obj-$(CONFIG_POWER_RESET_SPACEMIT_P1) += spacemit-p1-reboot.o
obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
obj-$(CONFIG_POWER_RESET_TH1520_AON) += th1520-aon-reboot.o
obj-$(CONFIG_POWER_RESET_TORADEX_EC) += tdx-ec-poweroff.o
diff --git a/drivers/power/reset/spacemit-p1-reboot.c b/drivers/power/reset/spacemit-p1-reboot.c
new file mode 100644
index 0000000000000..9ec3d1fff8f3d
--- /dev/null
+++ b/drivers/power/reset/spacemit-p1-reboot.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 by Aurelien Jarno
+ */
+
+#include <linux/bits.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reboot.h>
+
+/* Power Control Register 2 */
+#define PWR_CTRL2 0x7e
+#define PWR_CTRL2_SHUTDOWN BIT(2) /* Shutdown request */
+#define PWR_CTRL2_RST BIT(1) /* Reset request */
+
+static int spacemit_p1_pwroff_handler(struct sys_off_data *data)
+{
+ struct regmap *regmap = data->cb_data;
+ int ret;
+
+ /* Put the PMIC into shutdown state */
+ ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_SHUTDOWN);
+ if (ret) {
+ dev_err(data->dev, "shutdown failed: %d\n", ret);
+ return notifier_from_errno(ret);
+ }
+
+ return NOTIFY_DONE;
+}
+
+static int spacemit_p1_restart_handler(struct sys_off_data *data)
+{
+ struct regmap *regmap = data->cb_data;
+ int ret;
+
+ /* Put the PMIC into reset state */
+ ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_RST);
+ if (ret) {
+ dev_err(data->dev, "restart failed: %d\n", ret);
+ return notifier_from_errno(ret);
+ }
+
+ return NOTIFY_DONE;
+}
+
+static int spacemit_p1_reboot_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap;
+ int ret;
+
+ regmap = dev_get_regmap(dev->parent, NULL);
+ if (!regmap)
+ return -ENODEV;
+
+ ret = devm_register_power_off_handler(dev, &spacemit_p1_pwroff_handler,
+ regmap);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to register power off handler\n");
+
+ ret = devm_register_restart_handler(dev, spacemit_p1_restart_handler,
+ regmap);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to register restart handler\n");
+
+ return 0;
+}
+
+static const struct platform_device_id spacemit_p1_reboot_id_table[] = {
+ { "spacemit-p1-reboot", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, spacemit_p1_reboot_id_table);
+
+static struct platform_driver spacemit_p1_reboot_driver = {
+ .driver = {
+ .name = "spacemit-p1-reboot",
+ },
+ .probe = spacemit_p1_reboot_probe,
+ .id_table = spacemit_p1_reboot_id_table,
+};
+module_platform_driver(spacemit_p1_reboot_driver);
+
+MODULE_DESCRIPTION("SpacemiT P1 reboot/poweroff driver");
+MODULE_LICENSE("GPL");
--
2.47.2On Sun, Oct 26, 2025 at 11:41:14PM +0100, Aurelien Jarno wrote:
> This driver implements poweroff/reboot support for the SpacemiT P1 PMIC
> chip, which is commonly paired with the SpacemiT K1 SoC.
>
> The SpacemiT P1 support is implemented as a MFD driver, so the access is
> done directly through the regmap interface. Reboot or poweroff is
> triggered by setting a specific bit in a control register, which is
> automatically cleared by the hardware afterwards.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> v2:
> - Replace the "select" by a "depends on"
> - Remove outdated Reviewed-by
>
> drivers/power/reset/Kconfig | 9 +++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++
> 3 files changed, 98 insertions(+)
> create mode 100644 drivers/power/reset/spacemit-p1-reboot.c
>
> 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?
- Troy
> + help
> + This driver supports power-off and reset operations for the SpacemiT
> + P1 PMIC.
> +
> config POWER_RESET_SYSCON
> bool "Generic SYSCON regmap reset driver"
> depends on OF
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 51da87e05ce76..0e4ae6f6b5c55 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
> obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
> obj-$(CONFIG_POWER_RESET_REGULATOR) += regulator-poweroff.o
> obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
> +obj-$(CONFIG_POWER_RESET_SPACEMIT_P1) += spacemit-p1-reboot.o
> obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
> obj-$(CONFIG_POWER_RESET_TH1520_AON) += th1520-aon-reboot.o
> obj-$(CONFIG_POWER_RESET_TORADEX_EC) += tdx-ec-poweroff.o
> diff --git a/drivers/power/reset/spacemit-p1-reboot.c b/drivers/power/reset/spacemit-p1-reboot.c
> new file mode 100644
> index 0000000000000..9ec3d1fff8f3d
> --- /dev/null
> +++ b/drivers/power/reset/spacemit-p1-reboot.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 by Aurelien Jarno
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reboot.h>
> +
> +/* Power Control Register 2 */
> +#define PWR_CTRL2 0x7e
> +#define PWR_CTRL2_SHUTDOWN BIT(2) /* Shutdown request */
> +#define PWR_CTRL2_RST BIT(1) /* Reset request */
> +
> +static int spacemit_p1_pwroff_handler(struct sys_off_data *data)
> +{
> + struct regmap *regmap = data->cb_data;
> + int ret;
> +
> + /* Put the PMIC into shutdown state */
> + ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_SHUTDOWN);
> + if (ret) {
> + dev_err(data->dev, "shutdown failed: %d\n", ret);
> + return notifier_from_errno(ret);
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int spacemit_p1_restart_handler(struct sys_off_data *data)
> +{
> + struct regmap *regmap = data->cb_data;
> + int ret;
> +
> + /* Put the PMIC into reset state */
> + ret = regmap_set_bits(regmap, PWR_CTRL2, PWR_CTRL2_RST);
> + if (ret) {
> + dev_err(data->dev, "restart failed: %d\n", ret);
> + return notifier_from_errno(ret);
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int spacemit_p1_reboot_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + int ret;
> +
> + regmap = dev_get_regmap(dev->parent, NULL);
> + if (!regmap)
> + return -ENODEV;
> +
> + ret = devm_register_power_off_handler(dev, &spacemit_p1_pwroff_handler,
> + regmap);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register power off handler\n");
> +
> + ret = devm_register_restart_handler(dev, spacemit_p1_restart_handler,
> + regmap);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register restart handler\n");
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id spacemit_p1_reboot_id_table[] = {
> + { "spacemit-p1-reboot", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(platform, spacemit_p1_reboot_id_table);
> +
> +static struct platform_driver spacemit_p1_reboot_driver = {
> + .driver = {
> + .name = "spacemit-p1-reboot",
> + },
> + .probe = spacemit_p1_reboot_probe,
> + .id_table = spacemit_p1_reboot_id_table,
> +};
> +module_platform_driver(spacemit_p1_reboot_driver);
> +
> +MODULE_DESCRIPTION("SpacemiT P1 reboot/poweroff driver");
> +MODULE_LICENSE("GPL");
> --
> 2.47.2
>
>
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: > > This driver implements poweroff/reboot support for the SpacemiT P1 PMIC > > chip, which is commonly paired with the SpacemiT K1 SoC. > > > > The SpacemiT P1 support is implemented as a MFD driver, so the access is > > done directly through the regmap interface. Reboot or poweroff is > > triggered by setting a specific bit in a control register, which is > > automatically cleared by the hardware afterwards. > > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > v2: > > - Replace the "select" by a "depends on" > > - Remove outdated Reviewed-by > > > > drivers/power/reset/Kconfig | 9 +++ > > drivers/power/reset/Makefile | 1 + > > drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++ > > 3 files changed, 98 insertions(+) > > create mode 100644 drivers/power/reset/spacemit-p1-reboot.c > > > > 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. Building it as module by default shrinks the kernel image, which most distributions are willing to see, thus helps distro maintainers to maintain the configuration. The default value has been discussed in v2 of the series[1], where Emil, Yixun have expressed preference on "default m". So do I here. > - Troy Regards, Yao Zi [1]: https://lore.kernel.org/spacemit/aPXAyeDC7YXAketm@aurel32.net/ > > + help > > + This driver supports power-off and reset operations for the SpacemiT > > + P1 PMIC. > > + > > config POWER_RESET_SYSCON > > bool "Generic SYSCON regmap reset driver" > > depends on OF
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: >> > This driver implements poweroff/reboot support for the SpacemiT P1 PMIC >> > chip, which is commonly paired with the SpacemiT K1 SoC. >> > >> > The SpacemiT P1 support is implemented as a MFD driver, so the access is >> > done directly through the regmap interface. Reboot or poweroff is >> > triggered by setting a specific bit in a control register, which is >> > automatically cleared by the hardware afterwards. >> > >> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >> > --- >> > v2: >> > - Replace the "select" by a "depends on" >> > - Remove outdated Reviewed-by >> > >> > drivers/power/reset/Kconfig | 9 +++ >> > drivers/power/reset/Makefile | 1 + >> > drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++ >> > 3 files changed, 98 insertions(+) >> > create mode 100644 drivers/power/reset/spacemit-p1-reboot.c >> > >> > 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? -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
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: > >> > This driver implements poweroff/reboot support for the SpacemiT P1 PMIC > >> > chip, which is commonly paired with the SpacemiT K1 SoC. > >> > > >> > The SpacemiT P1 support is implemented as a MFD driver, so the access is > >> > done directly through the regmap interface. Reboot or poweroff is > >> > triggered by setting a specific bit in a control register, which is > >> > automatically cleared by the hardware afterwards. > >> > > >> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > >> > --- > >> > v2: > >> > - Replace the "select" by a "depends on" > >> > - Remove outdated Reviewed-by > >> > > >> > drivers/power/reset/Kconfig | 9 +++ > >> > drivers/power/reset/Makefile | 1 + > >> > drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++ > >> > 3 files changed, 98 insertions(+) > >> > create mode 100644 drivers/power/reset/spacemit-p1-reboot.c > >> > > >> > 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. So my statement, maybe as well as the module's default value, should be re-evaluated. Yixun, Emil, what do you think about it? Best regards, Yao Zi > -- > Andreas Schwab, SUSE Labs, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different." > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
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. /Emil
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)
On 2025-10-27 18:31, Yixun Lan wrote: > 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 A possible compromise here might be to use "default MFD_SPACEMIT_P1". This would defer the decision to another level, but I think it makes sense to have all parts of the MFD either built-in or as modules. Regards Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://aurel32.net
On Mon, Oct 27, 2025 at 02:34:33PM +0100, Aurelien Jarno wrote:
> On 2025-10-27 18:31, Yixun Lan wrote:
> > 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
>
> A possible compromise here might be to use "default MFD_SPACEMIT_P1".
> This would defer the decision to another level, but I think it makes
> sense to have all parts of the MFD either built-in or as modules.
I think both the regulator and shutdown/reboot drivers should be
`default MFD_SPACEMIT_P1`.
Otherwise, enabling MFD_SPACEMIT_P1 alone doesn't make much sense,
right?
- Troy
>
> Regards
> Aurelien
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net http://aurel32.net
>
On 2025-10-27 11:20, Troy Mitchell wrote: > On Sun, Oct 26, 2025 at 11:41:14PM +0100, Aurelien Jarno wrote: > > This driver implements poweroff/reboot support for the SpacemiT P1 PMIC > > chip, which is commonly paired with the SpacemiT K1 SoC. > > > > The SpacemiT P1 support is implemented as a MFD driver, so the access is > > done directly through the regmap interface. Reboot or poweroff is > > triggered by setting a specific bit in a control register, which is > > automatically cleared by the hardware afterwards. > > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > v2: > > - Replace the "select" by a "depends on" > > - Remove outdated Reviewed-by > > > > drivers/power/reset/Kconfig | 9 +++ > > drivers/power/reset/Makefile | 1 + > > drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++ > > 3 files changed, 98 insertions(+) > > create mode 100644 drivers/power/reset/spacemit-p1-reboot.c > > > > 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? As explained here, this is equivalent: https://lore.kernel.org/spacemit/CAJM55Z_BzfRo5aKf2VrneTymSizwDQq6OfMK_LNgyoGjp43K8Q@mail.gmail.com/ But I can make a v5 to change that, if it's the preferred form on the SpacemiT side. > Or default ARCH_SPACEMIT? > I believe that reboot and shutdown are actually essential functionalities, > so it might make more sense: default ARCH_SPACEMIT? That was already changed in v3, following a request on v2: https://lore.kernel.org/spacemit/CANBLGczi3GeaC4aWECV8NS-zqSHgRa-5onynz9fGsZeN8qgysg@mail.gmail.com/ Regards Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://aurel32.net
On Mon, Oct 27, 2025 at 07:01:13AM +0100, Aurelien Jarno wrote:
> On 2025-10-27 11:20, Troy Mitchell wrote:
> > On Sun, Oct 26, 2025 at 11:41:14PM +0100, Aurelien Jarno wrote:
> > > This driver implements poweroff/reboot support for the SpacemiT P1 PMIC
> > > chip, which is commonly paired with the SpacemiT K1 SoC.
> > >
> > > The SpacemiT P1 support is implemented as a MFD driver, so the access is
> > > done directly through the regmap interface. Reboot or poweroff is
> > > triggered by setting a specific bit in a control register, which is
> > > automatically cleared by the hardware afterwards.
> > >
> > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > > ---
> > > v2:
> > > - Replace the "select" by a "depends on"
> > > - Remove outdated Reviewed-by
> > >
> > > drivers/power/reset/Kconfig | 9 +++
> > > drivers/power/reset/Makefile | 1 +
> > > drivers/power/reset/spacemit-p1-reboot.c | 88 ++++++++++++++++++++++++
> > > 3 files changed, 98 insertions(+)
> > > create mode 100644 drivers/power/reset/spacemit-p1-reboot.c
> > >
> > > 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?
>
> As explained here, this is equivalent:
> https://lore.kernel.org/spacemit/CAJM55Z_BzfRo5aKf2VrneTymSizwDQq6OfMK_LNgyoGjp43K8Q@mail.gmail.com/
>
> But I can make a v5 to change that, if it's the preferred form on the
> SpacemiT side.
I missed that conversation. Just keep it!
>
> > Or default ARCH_SPACEMIT?
> > I believe that reboot and shutdown are actually essential functionalities,
> > so it might make more sense: default ARCH_SPACEMIT?
>
> That was already changed in v3, following a request on v2:
> https://lore.kernel.org/spacemit/CANBLGczi3GeaC4aWECV8NS-zqSHgRa-5onynz9fGsZeN8qgysg@mail.gmail.com/
Thanks for your link.
- Troy
>
> Regards
> Aurelien
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net http://aurel32.net
>
© 2016 - 2026 Red Hat, Inc.