From: Mike Larkin Subject: Re: rktemp(4): Add RK3588 support To: Mark Kettenis Cc: patrick@openbsd.org, dlg@openbsd.org, jmatthew@openbsd.org, kurt@openbsd.org, tech@openbsd.org Date: Tue, 11 Jun 2024 11:59:38 -0700 On Tue, Jun 11, 2024 at 11:38:28AM +0200, Mark Kettenis wrote: > The TSADC block of the RK3588 SoC has some significant differences > from earlier SoCs. While implementing support for this new SoC, I > adjusted the existing code for the older SoCs a bit, so this needs > testing on older Rockchip SoCs as well. > > This makes the sensors available, but isn't enough to make the thermal > zones function as those depend on functioning interrupts for trip > points. That is probably true for some of the older SoCs as well. > I'd like to address that in a follow-up diff. > > ok? tested on rock5b, don't see any new sensors but in any case the diff seems ok to me. > > Index: dev/fdt/rktemp.c > =================================================================== > RCS file: /cvs/src/sys/dev/fdt/rktemp.c,v > retrieving revision 1.12 > diff -u -p -r1.12 rktemp.c > --- dev/fdt/rktemp.c 5 Mar 2023 09:57:32 -0000 1.12 > +++ dev/fdt/rktemp.c 11 Jun 2024 09:30:43 -0000 > @@ -36,43 +36,38 @@ > #define TSADC_USER_CON_INTER_PD_SOC_SHIFT 6 > #define TSADC_AUTO_CON 0x0004 > #define TSADC_AUTO_CON_TSHUT_POLARITY (1 << 8) > -#define TSADC_AUTO_CON_SRC3_EN (1 << 7) > -#define TSADC_AUTO_CON_SRC2_EN (1 << 6) > -#define TSADC_AUTO_CON_SRC1_EN (1 << 5) > -#define TSADC_AUTO_CON_SRC0_EN (1 << 4) > +#define TSADC_AUTO_CON_SRC_EN(ch) (1 << ((ch) + 4)) > #define TSADC_AUTO_CON_TSADC_Q_SEL (1 << 1) > #define TSADC_AUTO_CON_AUTO_EN (1 << 0) > #define TSADC_INT_EN 0x0008 > -#define TSADC_INT_EN_TSHUT_2CRU_EN_SRC3 (1 << 11) > -#define TSADC_INT_EN_TSHUT_2CRU_EN_SRC2 (1 << 10) > -#define TSADC_INT_EN_TSHUT_2CRU_EN_SRC1 (1 << 9) > -#define TSADC_INT_EN_TSHUT_2CRU_EN_SRC0 (1 << 8) > -#define TSADC_INT_EN_TSHUT_2GPIO_EN_SRC3 (1 << 7) > -#define TSADC_INT_EN_TSHUT_2GPIO_EN_SRC2 (1 << 6) > -#define TSADC_INT_EN_TSHUT_2GPIO_EN_SRC1 (1 << 5) > -#define TSADC_INT_EN_TSHUT_2GPIO_EN_SRC0 (1 << 4) > +#define TSADC_INT_EN_TSHUT_2CRU_EN_SRC(ch) (1 << ((ch) + 8)) > +#define TSADC_INT_EN_TSHUT_2GPIO_EN_SRC(ch) (1 << ((ch) + 4)) > #define TSADC_INT_PD 0x000c > -#define TSADC_INT_PD_TSHUT_O_SRC0 (1 << 4) > -#define TSADC_INT_PD_TSHUT_O_SRC1 (1 << 5) > -#define TSADC_INT_PD_TSHUT_O_SRC2 (1 << 6) > -#define TSADC_INT_PD_TSHUT_O_SRC3 (1 << 7) > -#define TSADC_DATA0 0x0020 > -#define TSADC_DATA1 0x0024 > -#define TSADC_DATA2 0x0028 > -#define TSADC_DATA3 0x002c > -#define TSADC_COMP0_INT 0x0030 > -#define TSADC_COMP1_INT 0x0034 > -#define TSADC_COMP2_INT 0x0038 > -#define TSADC_COMP3_INT 0x003c > -#define TSADC_COMP0_SHUT 0x0040 > -#define TSADC_COMP1_SHUT 0x0044 > -#define TSADC_COMP2_SHUT 0x0048 > -#define TSADC_COMP3_SHUT 0x004c > +#define TSADC_INT_PD_TSHUT_O_SRC(ch) (1 << ((ch) + 4)) > +#define TSADC_DATA(ch) (0x0020 + (ch) * 4) > +#define TSADC_COMP_INT(ch) (0x0030 + (ch) * 4) > +#define TSADC_COMP_SHUT(ch) (0x0040 + (ch) * 4) > #define TSADC_HIGHT_INT_DEBOUNCE 0x0060 > #define TSADC_HIGHT_TSHUT_DEBOUNCE 0x0064 > #define TSADC_AUTO_PERIOD 0x0068 > #define TSADC_AUTO_PERIOD_HT 0x006c > > +/* RK3588 */ > +#define TSADC_V3_AUTO_SRC 0x000c > +#define TSADC_V3_AUTO_SRC_CH(ch) (1 << (ch)) > +#define TSADC_V3_GPIO_EN 0x0018 > +#define TSADC_V3_GPIO_EN_CH(ch) (1 << (ch)) > +#define TSADC_V3_CRU_EN 0x001c > +#define TSADC_V3_CRU_EN_CH(ch) (1 << (ch)) > +#define TSADC_V3_HLT_INT_PD 0x0024 > +#define TSADC_V3_HT_INT_STATUS(ch) (1 << (ch)) > +#define TSADC_V3_DATA(ch) (0x002c + (ch) * 4) > +#define TSADC_V3_COMP_SHUT(ch) (0x010c + (ch) * 4) > +#define TSADC_V3_HIGHT_INT_DEBOUNCE 0x014c > +#define TSADC_V3_HIGHT_TSHUT_DEBOUNCE 0x0150 > +#define TSADC_V3_AUTO_PERIOD 0x0154 > +#define TSADC_V3_AUTO_PERIOD_HT 0x0158 > + > /* RK3568 */ > #define RK3568_GRF_TSADC_CON 0x0600 > #define RK3568_GRF_TSADC_EN (1 << 8) > @@ -248,16 +243,36 @@ const struct rktemp_entry rk3568_temps[] > > const char *const rk3568_names[] = { "CPU", "GPU" }; > > +/* RK3588 conversion table. */ > +const struct rktemp_entry rk3588_temps[] = { > + { -40000, 215 }, > + { 25000, 285 }, > + { 85000, 350 }, > + { 125000, 395 }, > +}; > + > +const char *const rk3588_names[] = { > + "Top", > + "CPU (big0)", > + "CPU (big1)", > + "CPU (little)", > + "Center", > + "GPU", > + "NPU" > +}; > + > struct rktemp_softc { > struct device sc_dev; > bus_space_tag_t sc_iot; > bus_space_handle_t sc_ioh; > int sc_node; > > + bus_size_t sc_data0; > + > const struct rktemp_entry *sc_temps; > int sc_ntemps; > > - struct ksensor sc_sensors[3]; > + struct ksensor sc_sensors[7]; > int sc_nsensors; > struct ksensordev sc_sensordev; > > @@ -291,7 +306,8 @@ rktemp_match(struct device *parent, void > OF_is_compatible(faa->fa_node, "rockchip,rk3308-tsadc") || > OF_is_compatible(faa->fa_node, "rockchip,rk3328-tsadc") || > OF_is_compatible(faa->fa_node, "rockchip,rk3399-tsadc") || > - OF_is_compatible(faa->fa_node, "rockchip,rk3568-tsadc")); > + OF_is_compatible(faa->fa_node, "rockchip,rk3568-tsadc") || > + OF_is_compatible(faa->fa_node, "rockchip,rk3588-tsadc")); > } > > void > @@ -301,8 +317,7 @@ rktemp_attach(struct device *parent, str > struct fdt_attach_args *faa = aux; > const char *const *names; > uint32_t mode, polarity, temp; > - uint32_t auto_con, int_en; > - uint32_t inter_pd_soc; > + uint32_t auto_con, inter_pd_soc; > int auto_period, auto_period_ht; > int i; > > @@ -354,7 +369,7 @@ rktemp_attach(struct device *parent, str > inter_pd_soc = 13; > auto_period = 1875; /* 2.5 ms */ > auto_period_ht = 1875; /* 2.5 ms */ > - } else { > + } else if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-tsadc")) { > sc->sc_temps = rk3568_temps; > sc->sc_ntemps = nitems(rk3568_temps); > sc->sc_nsensors = 2; > @@ -362,6 +377,14 @@ rktemp_attach(struct device *parent, str > inter_pd_soc = 63; /* 97 us */ > auto_period = 1622; /* 2.5 ms */ > auto_period_ht = 1622; /* 2.5 ms */ > + } else { > + sc->sc_temps = rk3588_temps; > + sc->sc_ntemps = nitems(rk3588_temps); > + sc->sc_nsensors = 7; > + names = rk3588_names; > + inter_pd_soc = 0; > + auto_period = 5000; /* 2.5 ms */ > + auto_period_ht = 5000; /* 2.5 ms */ > } > > pinctrl_byname(sc->sc_node, "init"); > @@ -371,57 +394,110 @@ rktemp_attach(struct device *parent, str > clock_enable(sc->sc_node, "apb_pclk"); > > /* Reset the TS-ADC controller block. */ > - reset_assert(sc->sc_node, "tsadc-apb"); > + reset_assert_all(sc->sc_node); > delay(10); > - reset_deassert(sc->sc_node, "tsadc-apb"); > + reset_deassert_all(sc->sc_node); > > mode = OF_getpropint(sc->sc_node, "rockchip,hw-tshut-mode", 1); > polarity = OF_getpropint(sc->sc_node, "rockchip,hw-tshut-polarity", 0); > temp = OF_getpropint(sc->sc_node, "rockchip,hw-tshut-temp", 95000); > > - HWRITE4(sc, TSADC_USER_CON, > - inter_pd_soc << TSADC_USER_CON_INTER_PD_SOC_SHIFT); > - HWRITE4(sc, TSADC_AUTO_PERIOD, auto_period); > - HWRITE4(sc, TSADC_AUTO_PERIOD_HT, auto_period_ht); > - HWRITE4(sc, TSADC_HIGHT_INT_DEBOUNCE, 4); > - HWRITE4(sc, TSADC_HIGHT_TSHUT_DEBOUNCE, 4); > - > - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-tsadc")) > - rktemp_rk3568_init(sc); > - > - auto_con = HREAD4(sc, TSADC_AUTO_CON); > - auto_con |= TSADC_AUTO_CON_TSADC_Q_SEL; > - if (polarity) > - auto_con |= TSADC_AUTO_CON_TSHUT_POLARITY; > - HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + if (OF_is_compatible(sc->sc_node, "rockchip,rk3588-tsadc")) { > + uint32_t gpio_en, cru_en; > > - /* Set shutdown limit. */ > - for (i = 0; i < sc->sc_nsensors; i++) { > - HWRITE4(sc, TSADC_COMP0_SHUT + i * 4, > - rktemp_calc_code(sc, temp)); > - auto_con |= (TSADC_AUTO_CON_SRC0_EN << i); > - } > - HWRITE4(sc, TSADC_AUTO_CON, auto_con); > - > - /* Clear shutdown output status. */ > - for (i = 0; i < sc->sc_nsensors; i++) > - HWRITE4(sc, TSADC_INT_PD, (TSADC_INT_PD_TSHUT_O_SRC0 << i)); > + sc->sc_data0 = TSADC_V3_DATA(0); > + > + HWRITE4(sc, TSADC_V3_AUTO_PERIOD, auto_period); > + HWRITE4(sc, TSADC_V3_AUTO_PERIOD_HT, auto_period_ht); > + HWRITE4(sc, TSADC_V3_HIGHT_INT_DEBOUNCE, 4); > + HWRITE4(sc, TSADC_V3_HIGHT_TSHUT_DEBOUNCE, 4); > + > + auto_con = TSADC_AUTO_CON_TSHUT_POLARITY << 16; > + if (polarity) > + auto_con = TSADC_AUTO_CON_TSHUT_POLARITY; > + HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + > + /* Set shutdown limit. */ > + for (i = 0; i < sc->sc_nsensors; i++) { > + HWRITE4(sc, TSADC_V3_COMP_SHUT(i), > + rktemp_calc_code(sc, temp)); > + HWRITE4(sc, TSADC_V3_AUTO_SRC, > + TSADC_V3_AUTO_SRC_CH(i) << 16 | > + TSADC_V3_AUTO_SRC_CH(i)); > + } > > - /* Configure mode. */ > - int_en = HREAD4(sc, TSADC_INT_EN); > - for (i = 0; i < sc->sc_nsensors; i++) { > - if (mode) > - int_en |= (TSADC_INT_EN_TSHUT_2GPIO_EN_SRC0 << i); > - else > - int_en |= (TSADC_INT_EN_TSHUT_2CRU_EN_SRC0 << i); > + /* Clear shutdown output status. */ > + for (i = 0; i < sc->sc_nsensors; i++) { > + HWRITE4(sc, TSADC_V3_HLT_INT_PD, > + TSADC_V3_HT_INT_STATUS(i)); > + } > + > + /* Configure mode. */ > + gpio_en = cru_en = 0; > + for (i = 0; i < sc->sc_nsensors; i++) { > + gpio_en |= TSADC_V3_GPIO_EN_CH(i) << 16; > + cru_en |= TSADC_V3_CRU_EN_CH(i) << 16; > + if (mode) > + gpio_en |= TSADC_V3_GPIO_EN_CH(i); > + else > + cru_en |= TSADC_V3_CRU_EN_CH(i); > + } > + HWRITE4(sc, TSADC_V3_GPIO_EN, gpio_en); > + HWRITE4(sc, TSADC_V3_CRU_EN, cru_en); > + } else { > + uint32_t int_en; > + > + sc->sc_data0 = TSADC_DATA(0); > + > + HWRITE4(sc, TSADC_USER_CON, > + inter_pd_soc << TSADC_USER_CON_INTER_PD_SOC_SHIFT); > + HWRITE4(sc, TSADC_AUTO_PERIOD, auto_period); > + HWRITE4(sc, TSADC_AUTO_PERIOD_HT, auto_period_ht); > + HWRITE4(sc, TSADC_HIGHT_INT_DEBOUNCE, 4); > + HWRITE4(sc, TSADC_HIGHT_TSHUT_DEBOUNCE, 4); > + > + if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-tsadc")) > + rktemp_rk3568_init(sc); > + > + auto_con = HREAD4(sc, TSADC_AUTO_CON); > + auto_con |= TSADC_AUTO_CON_TSADC_Q_SEL; > + if (polarity) > + auto_con |= TSADC_AUTO_CON_TSHUT_POLARITY; > + HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + > + /* Set shutdown limit. */ > + for (i = 0; i < sc->sc_nsensors; i++) { > + HWRITE4(sc, TSADC_COMP_SHUT(i), > + rktemp_calc_code(sc, temp)); > + auto_con |= (TSADC_AUTO_CON_SRC_EN(i)); > + } > + HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + > + /* Clear shutdown output status. */ > + for (i = 0; i < sc->sc_nsensors; i++) > + HWRITE4(sc, TSADC_INT_PD, TSADC_INT_PD_TSHUT_O_SRC(i)); > + > + /* Configure mode. */ > + int_en = HREAD4(sc, TSADC_INT_EN); > + for (i = 0; i < sc->sc_nsensors; i++) { > + if (mode) > + int_en |= TSADC_INT_EN_TSHUT_2GPIO_EN_SRC(i); > + else > + int_en |= TSADC_INT_EN_TSHUT_2CRU_EN_SRC(i); > + } > + HWRITE4(sc, TSADC_INT_EN, int_en); > } > - HWRITE4(sc, TSADC_INT_EN, int_en); > > pinctrl_byname(sc->sc_node, "default"); > > /* Finally turn on the ADC. */ > - auto_con |= TSADC_AUTO_CON_AUTO_EN; > - HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + if (OF_is_compatible(sc->sc_node, "rockchip,rk3588-tsadc")) { > + HWRITE4(sc, TSADC_AUTO_CON, > + TSADC_AUTO_CON_AUTO_EN << 16 | TSADC_AUTO_CON_AUTO_EN); > + } else { > + auto_con |= TSADC_AUTO_CON_AUTO_EN; > + HWRITE4(sc, TSADC_AUTO_CON, auto_con); > + } > > /* Register sensors. */ > strlcpy(sc->sc_sensordev.xname, sc->sc_dev.dv_xname, > @@ -557,7 +633,7 @@ rktemp_refresh_sensors(void *arg) > int i; > > for (i = 0; i < sc->sc_nsensors; i++) { > - code = HREAD4(sc, TSADC_DATA0 + i * 4); > + code = HREAD4(sc, sc->sc_data0 + (i * 4)); > temp = rktemp_calc_temp(sc, code); > sc->sc_sensors[i].value = 273150000 + 1000 * temp; > if (rktemp_valid(sc, code)) > @@ -577,7 +653,7 @@ rktemp_get_temperature(void *cookie, uin > if (idx >= sc->sc_nsensors) > return THERMAL_SENSOR_MAX; > > - code = HREAD4(sc, TSADC_DATA0 + idx * 4); > + code = HREAD4(sc, sc->sc_data0 + (idx * 4)); > if (rktemp_valid(sc, code)) > return rktemp_calc_temp(sc, code); > else