On Fri, Nov 22, 2024 at 12:17:06AM +0100, Mark Kettenis wrote:
> > +#define RK805_DEV_CTRL		0x4b
> > +#define  RK805_DEV_CTRL_DEV_OFF	0x01
> > +
> > +#define RK806_SYS_CFG3		0x72
> > +#define  RK806_SYS_CFG3_DEV_OFF	0x01
> > +
> > +#define RK808_DEVCTRL		0x4b
> > +#define  RK808_DEVCTRL_DEV_OFF_RST	0x08
> 
> I wonder why for the RK808 the DEV_OFF_RST gets used whereas for the
> others just DEV_OFF.  There is a DEV_OFF bit for the RK808 as well.  I
> noticed that Linux does the same thing, but I do wonder what happens
> if you use the DEV_OFF bit instead of the DEV_OFF_RST bit on the
> RK808.

I wondered the same and my conclusion was that since DEV_OFF_RST not
only powers the system off but also fully resets the PMIC that might
provide better consistency (nice to have every startup from identical
state) so should be preferred when available. Surprisingly only RK808
seems to offer this (or probably the documentation for the others is
inaccurate but I do not have hardware to test).

Per your request I tried DEV_OFF (0x01) bit on RK808 here and it
"worked the same" (but I haven't really checked if any of the state
was preserved). Since we know DEV_OFF_RST works for Linux users I do
not see a good reason to deviate.

Here follows the updated diff which handles the non-deprecated node
name.

---

diff --git sys/dev/fdt/rkpmic.c sys/dev/fdt/rkpmic.c
index cec42e25a..67a613024 100644
--- sys/dev/fdt/rkpmic.c
+++ sys/dev/fdt/rkpmic.c
@@ -33,6 +33,8 @@
 
 #include <dev/clock_subr.h>
 
+extern void (*powerdownfn)(void);
+
 #define RK80X_SECONDS		0x00
 #define RK80X_MINUTES		0x01
 #define RK80X_HOURS		0x02
@@ -52,10 +54,20 @@
 #define RK809_RTC_STATUS	0x0e
 #define  RK80X_RTC_STATUS_POWER_UP	0x80
 
+#define RK805_DEV_CTRL		0x4b
+#define  RK805_DEV_CTRL_DEV_OFF	0x01
+
+#define RK806_SYS_CFG3		0x72
+#define  RK806_SYS_CFG3_DEV_OFF	0x01
+
+#define RK808_DEVCTRL		0x4b
+#define  RK808_DEVCTRL_DEV_OFF_RST	0x08
+
 #define RK809_PMIC_SYS_CFG3	0xf4
 #define  RK809_PMIC_SYS_CFG3_SLP_FUN_MASK	0x18
 #define  RK809_PMIC_SYS_CFG3_SLP_FUN_NONE	0x00
 #define  RK809_PMIC_SYS_CFG3_SLP_FUN_SLEEP	0x08
+#define  RK809_PMIC_SYS_CFG3_DEV_OFF		0x01
 #define RK809_PMIC_INT_STS0	0xf8
 #define RK809_PMIC_INT_MSK0	0xf9
 #define  RK809_PMIC_INT_MSK0_PWRON_FALL_INT_IM	0x01
@@ -338,6 +350,8 @@ struct rkpmic_softc {
 	struct spi_config sc_spi_conf;
 
 	int sc_rtc_ctrl_reg, sc_rtc_status_reg;
+	uint8_t sc_dev_ctrl_reg, sc_dev_off_val;
+
 	struct todr_chip_handle sc_todr;
 	const struct rkpmic_regdata *sc_regdata;
 
@@ -382,6 +396,9 @@ int	rkpmic_clock_write(struct rkpmic_softc *, struct clock_ymdhms *);
 int	rkpmic_gettime(struct todr_chip_handle *, struct timeval *);
 int	rkpmic_settime(struct todr_chip_handle *, struct timeval *);
 
+struct rkpmic_softc *rkpmic_sc;
+void    rkpmic_powerdown(void);
+
 int
 rkpmic_i2c_match(struct device *parent, void *match, void *aux)
 {
@@ -447,24 +464,34 @@ rkpmic_attach(struct device *parent, struct device *self, void *aux)
 		chip = "RK805";
 		sc->sc_rtc_ctrl_reg = RK805_RTC_CTRL;
 		sc->sc_rtc_status_reg = RK805_RTC_STATUS;
+		sc->sc_dev_ctrl_reg = RK805_DEV_CTRL;
+		sc->sc_dev_off_val = RK805_DEV_CTRL_DEV_OFF;
 		sc->sc_regdata = rk805_regdata;
 	} else if (OF_is_compatible(sc->sc_node, "rockchip,rk806")) {
 		chip = "RK806";
+		sc->sc_dev_ctrl_reg = RK806_SYS_CFG3;
+		sc->sc_dev_off_val = RK806_SYS_CFG3_DEV_OFF;
 		sc->sc_regdata = rk806_regdata;
 	} else if (OF_is_compatible(sc->sc_node, "rockchip,rk808")) {
 		chip = "RK808";
 		sc->sc_rtc_ctrl_reg = RK808_RTC_CTRL;
 		sc->sc_rtc_status_reg = RK808_RTC_STATUS;
+		sc->sc_dev_ctrl_reg = RK808_DEVCTRL;
+		sc->sc_dev_off_val = RK808_DEVCTRL_DEV_OFF_RST;
 		sc->sc_regdata = rk808_regdata;
 	} else if (OF_is_compatible(sc->sc_node, "rockchip,rk809")) {
 		chip = "RK809";
 		sc->sc_rtc_ctrl_reg = RK809_RTC_CTRL;
 		sc->sc_rtc_status_reg = RK809_RTC_STATUS;
+		sc->sc_dev_ctrl_reg = RK809_PMIC_SYS_CFG3;
+		sc->sc_dev_off_val = RK809_PMIC_SYS_CFG3_DEV_OFF;
 		sc->sc_regdata = rk809_regdata;
 	} else {
 		chip = "RK817";
 		sc->sc_rtc_ctrl_reg = RK809_RTC_CTRL;
 		sc->sc_rtc_status_reg = RK809_RTC_STATUS;
+		sc->sc_dev_ctrl_reg = RK809_PMIC_SYS_CFG3;
+		sc->sc_dev_off_val = RK809_PMIC_SYS_CFG3_DEV_OFF;
 		sc->sc_regdata = rk817_regdata;
 	}
 	printf(": %s\n", chip);
@@ -511,6 +538,12 @@ rkpmic_attach(struct device *parent, struct device *self, void *aux)
 			device_register_wakeup(&sc->sc_dev);
 #endif
 	}
+
+	if (OF_getpropbool(sc->sc_node, "system-power-controller") ||
+	    OF_getpropbool(sc->sc_node, "rockchip,system-power-controller")) {
+		rkpmic_sc = sc;
+		powerdownfn = rkpmic_powerdown;
+	}
 }
 
 int
@@ -557,6 +590,14 @@ rkpmic_intr(void *arg)
 	return 1;
 }
 
+void
+rkpmic_powerdown(void)
+{
+	struct rkpmic_softc *sc = rkpmic_sc;
+	rkpmic_reg_write(sc, sc->sc_dev_ctrl_reg,
+	    rkpmic_reg_read(sc, sc->sc_dev_ctrl_reg) | sc->sc_dev_off_val);
+}
+
 struct rkpmic_regulator {
 	struct rkpmic_softc *rr_sc;