Messages in this thread | | | From | Henry Yen (顏修溫) <> | Subject | Re: [PATCH 1/2] thermal: mediatek: add support for MT7986 and MT7981 | Date | Wed, 30 Nov 2022 07:43:53 +0000 |
| |
On Tue, 2022-11-29 at 12:11 +0000, Daniel Golle wrote: > 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.
Actually I'm just the original author who developes MT7986/MT7981 thermal driver and refactors this conversion formula. Getting rid of "adc_oe" is indeed an intentional change, because back then our hardware members indicated that this parameter shouldn't have been used.
Thank you for your carefulness :)
> > > 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. >
The value of "adc_oe" is not necessarily 512. It could possibly vary from board to board.
- Henry
> > > > [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
| |