Index | Thread | Search

From:
Mike Larkin <mlarkin@nested.page>
Subject:
Re: rktemp(4): Add RK3588 support
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
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

Download raw body.

Thread
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