From: Mark Kettenis Subject: Re: rkpmic(4) support system power down To: Paul Fertser Cc: tech@openbsd.org Date: Sat, 23 Nov 2024 22:24:48 +0100 > Date: Fri, 22 Nov 2024 13:53:58 +0300 > From: Paul Fertser > > 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. Yes, I agree. > Here follows the updated diff which handles the non-deprecated node > name. Thanks. I committed this version of the diff. > 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 > > +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; > >