Re: [PATCH 1/2] thermal: mediatek: add support for MT7986 and MT7981

[Daniel Golle]

On Tue, Nov 29, 2022 at 11:53:58AM +0000, Henry Yen (顏修溫) wrote:
> On Mon, 2022-10-31 at 23:07 +0000, Daniel Golle wrote:
> > diff --git a/drivers/thermal/mtk_thermal.c
> > b/drivers/thermal/mtk_thermal.c
> > 
> > +static int raw_to_mcelsius_v3(struct mtk_thermal *mt, int sensno,
> > s32 raw)
> > +{
> > +	s32 tmp;
> > +
> > +	if (raw == 0)
> > +		return 0;
> > +
> > +	raw &= 0xfff;
> > +	tmp = 100000 * 15 / 16 * 10000;
> > +	tmp /= 4096 - 512 + mt->adc_ge;
> > +	tmp /= 1490;
> > +	tmp *= raw - mt->vts[sensno] - 2900 - mt->adc_oe + 512;
> 
> Hi Daniel,
> 
> Regarding the conversion formula, I would suggest following the
> original one, i.e., discarding "adc_oe" parameter as shown in [1].

Ok, I see. According to the commit description it looked to me more
like adc_oe has been dropped by accident, it doesn't sound like it
happened on purpose:
"Refactor MT7986 thermal temperature calculation formula to
prevent loss of floating-point accuracy."

Hence it made sense to be to keep the parameter and really only
use the updated formula to not loose precision. Maybe you can inquire
with the original author if dropping adc_oe was intentional, despite
being unmentioned in the commit message.


> This equation is derived based on hardware-specific theory, so any
> arbitrary change could possibly lead to incorrect temperature output.
> Thanks.

On my BPi-R3 board I found the value 512 burned into the efuse, so in
practise the resulting calculated temperature is exactly the same on
this board.

If other MT7986 or MT7981 boards will have arbitrary values stored in
adc_oe field in the efuse because this value isn't even used during
the manufacturer's calibration process, then of course, there is no
choice other than dropping it here as well.

> 
> [1]
> 
> https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/baf36c7eef477aae1f8f2653b6c29e2caf48475b%5E%21/#F0
> 
> 
> Henry
> 
> > +
> > +	return mt->degc_cali * 500 - tmp;
> > +}
> > +
> >  /**
> >   * mtk_thermal_get_bank - get bank
> >   * @bank:	The bank