rk_tsadc breaks (my) Rock64

[Peter Jeremy]

On 2019-Dec-02 15:45:48 +0100, Emmanuel Vadot <manu at bidouilliste.com> wrote:
>> >> Firstly, I've found that the syscon at ff100000 FDT entry attaches as two
>> >> distinct devices:
>> >> rk_grf0: <RockChip General Register Files> mem 0xff100000-0xff100fff on ofwbus0
>> >>   (via compatible = "rockchip,rk3328-grf")
>> >> simple_mfd0: <Simple MFD (Multi-Functions Device)> mem 0xff450000-0xff45ffff on ofwbus0
>> >>   (via compatible = "simple-mfd")
>> > 
>> >  ??? those aren't the same devices.

Yes.  Looking more closely, I was wrong.  There are 2 GRF devices in
the RK3328 that probe/attach separately.

> This isn't enough, RK3328 initialization is V2 but needs the
>AUTO_Q_SEL bit too (like v3). There might be other stuff that I
>haven't found yet.
> That still doesn't explain why it's working for me (both DEBUG and
>NODEBUG kernels).

I don't understand how the SYSCON_WRITE_4() operations can work when
the offsets are outside the memory range allocated to the device.

The other oddity I have is that Rockchip_RK3328TRM_V1.1-Part1-20170321.pdf
Table 9-1 (the Temperature-to-code mapping table) is identical to
rk3288_calib_data and looks completely different to rk3328_calib_data.
(Looking more closely, the values in the two tables sum to 4096).

I've tried the following, slightly more extensive patch and now get
temperature readings that aren't insane (~40°C idle and ~52°C under
heavy FPU load, at 1200MHz with a small heatsink and active airflow).
I haven't worked through the TRM in detail to see if there's anything
else missing.

Index: rk_tsadc.c
===================================================================
--- rk_tsadc.c	(revision 355286)
+++ rk_tsadc.c	(working copy)
@@ -98,11 +98,6 @@
 	int			channel;
 };
 
-enum tsadc_type {
-	RK_TSADC_V2,
-	RK_TSADC_V3
-};
-
 struct rk_calib_entry {
 	uint32_t	raw;
 	int		temp;
@@ -109,13 +104,14 @@
 };
 
 struct tsadc_calib_info {
-	bool decrement_mode;
+    //bool decrement_mode;
 	struct rk_calib_entry	*table;
 	int			nentries;
 };
 
 struct tsadc_conf {
-	enum tsadc_type		type;
+	int			use_syscon;
+	int			q_sel_ntc;
 	int			shutdown_temp;
 	int			shutdown_mode;
 	int			shutdown_pol;
@@ -188,7 +184,8 @@
 };
 
 struct tsadc_conf rk3288_tsadc_conf = {
-	.type = 		RK_TSADC_V2,
+	.use_syscon=		0,
+	.q_sel_ntc=		0,
 	.shutdown_temp =	95000,
 	.shutdown_mode =	1, /* GPIO */
 	.shutdown_pol =		0, /* Low  */
@@ -241,7 +238,8 @@
 };
 
 static struct tsadc_conf rk3328_tsadc_conf = {
-	.type = 		RK_TSADC_V3,
+	.use_syscon=		0,
+	.q_sel_ntc=		1,
 	.shutdown_temp =	95000,
 	.shutdown_mode =	0, /* CRU */
 	.shutdown_pol =		0, /* Low  */
@@ -296,7 +294,8 @@
 };
 
 static struct tsadc_conf rk3399_tsadc_conf = {
-	.type = 		RK_TSADC_V3,
+	.use_syscon=		1,
+	.q_sel_ntc=		1,
 	.shutdown_temp =	95000,
 	.shutdown_mode =	1, /* GPIO */
 	.shutdown_pol =		0, /* Low  */
@@ -444,11 +443,11 @@
 		val |= TSADC_AUTO_CON_POL_HI;
 	else
 		val &= ~TSADC_AUTO_CON_POL_HI;
-	if (sc->conf->type == RK_TSADC_V3)
+	if (sc->conf->q_sel_ntc)
 		val |= TSADC_AUTO_Q_SEL;
 	WR4(sc, TSADC_AUTO_CON, val);
 
-	if (sc->conf->type == RK_TSADC_V2) {
+	if (!sc->conf->use_syscon) {
 		/* V2 init */
 		WR4(sc, TSADC_AUTO_PERIOD, 250); 	/* 250 ms */
 		WR4(sc, TSADC_AUTO_PERIOD_HT, 50);	/*  50 ms */

-- 
Peter Jeremy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-arm/attachments/20191203/09f7a7e2/attachment.sig>