Re: [PATCH 2/2] thermal: rockchip: Support the rk3562 SoC in thermal driver

[Dragan Simic]

Hello Daniel,

On 2025-02-11 10:36, Daniel Lezcano wrote:
> On 24/12/2024 10:40, Kever Yang wrote:
> > From: Shaohan Yao <shaohan.yao@xxxxxxxxxxxxxx>
> >
> > There are one Temperature Sensor on rk3562, channel 0 is for chip.
>
> A bit stingy in terms of description, no ?

[snip]

> > @@ -497,6 +509,45 @@ static const struct tsadc_table 
> > rk3399_code_table[] = {
> >   	{TSADCV3_DATA_MASK, 125000},
> >   };
> >   +static const struct tsadc_table rk3562_code_table[] = {
> > +	{0, -40000},
> > +	{1419, -40000},
> > +	{1428, -35000},
> > +	{1436, -30000},
> > +	{1445, -25000},
> > +	{1453, -20000},
> > +	{1462, -15000},
> > +	{1470, -10000},
> > +	{1479, -5000},
> > +	{1487, 0},
> > +	{1496, 5000},
> > +	{1504, 10000},
> > +	{1512, 15000},
> > +	{1521, 20000},
> > +	{1529, 25000},
> > +	{1538, 30000},
> > +	{1546, 35000},
> > +	{1555, 40000},
> > +	{1563, 45000},
> > +	{1572, 50000},
> > +	{1580, 55000},
> > +	{1589, 60000},
> > +	{1598, 65000},
> > +	{1606, 70000},
> > +	{1615, 75000},
> > +	{1623, 80000},
> > +	{1632, 85000},
> > +	{1640, 90000},
> > +	{1648, 95000},
> > +	{1657, 100000},
> > +	{1666, 105000},
> > +	{1674, 110000},
> > +	{1682, 115000},
> > +	{1691, 120000},
> > +	{1699, 125000},
> > +	{TSADCV2_DATA_MASK, 125000},
> > +};
>
> May be it is time to optimize all these tables out of the memory 
> driver?
>
> It is the 9th table introduced.

FWIW, I'm also not a big fan of these tables, and I always feel like
"meh" when I see them.  However, I'm also not a big fan of substituting
them with some kind of formulas, because the SoC TRMs specify exactly
these tables, with some additional notes that the values may change in
future SoC revisions.  In other words, these values may even change at
some point, at least in theory.

With all that in mind, FWIW, I'd rather see these conversion tables
included verbatim in the driver code, because such an approach reduces
the possibilities for some hard-to-spot errors, allows easy, direct
correlation with the TRMs, and should make possible changes to them
in the future easier.