From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2402DC47088 for ; Thu, 1 Dec 2022 13:24:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231402AbiLANY2 (ORCPT ); Thu, 1 Dec 2022 08:24:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36118 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230183AbiLANYZ (ORCPT ); Thu, 1 Dec 2022 08:24:25 -0500 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 19ACE4E69E; Thu, 1 Dec 2022 05:24:23 -0800 (PST) Received: from [192.168.1.100] (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 002906602AA1; Thu, 1 Dec 2022 13:24:20 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1669901061; bh=W4oiVr5wu3WAPK6sr8VuvGqiEL/dy9kQpNxqOD3eRpA=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ofQVYHGP2yx6yoxUD13+v3tIREioSOMoxBXNNyhSYE5Q9Xx2arU1P7HTdqDYRF5xa FNLIKtdVQuRMjwSx0iEQp1vzvZAK0C+KnwF6juunWlsVfDnpw6Zv2roAmexkuybXos LjKoOYg1SGihvhQakzEV8KDhf3Scn/5NyuB31u1i0LzcJoufmFFx422iW2vYAdPFeL F3iAD23XyYQYaVPm20CUNaDuq2ALwhza6eJWId2YpoAVjBpFAPUDJfEktqidh01SzL SMeVrvtYyp7fvhZdNEST7UVRl6pHXZhHBUlp4/yPdJAsZj3KG3sIcY2eqFJ6XhSbOj NWPiOfL7uPHKA== Message-ID: <0b72a12c-286f-79d0-09e9-b1761530850a@collabora.com> Date: Thu, 1 Dec 2022 14:24:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH v2 1/2] thermal: mediatek: add support for MT7986 and MT7981 Content-Language: en-US To: Daniel Golle , linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, "Rafael J. Wysocki" , Daniel Lezcano , Amit Kucheria , Zhang Rui , Matthias Brugger , Rob Herring Cc: Steven Liu , Henry Yen References: From: AngeloGioacchino Del Regno In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Il 30/11/22 14:19, Daniel Golle ha scritto: > Add support for V3 generation thermal found in MT7986 and MT7981 SoCs. > Brings code to assign values from efuse as well as new function to > convert raw temperature to millidegree celsius, as found in MediaTek's > SDK sources (but cleaned up and de-duplicated) > > [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/baf36c7eef477aae1f8f2653b6c29e2caf48475b > Signed-off-by: Daniel Golle > Reviewed-by: Henry Yen > --- > Changes since v1: Drop use of adc_oe field in efuse, Henry Yen confirmed > its use has been dropped intentionally in MTK SDK as well. > > drivers/thermal/mtk_thermal.c | 122 +++++++++++++++++++++++++++++++++- > 1 file changed, 119 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > index 8440692e3890..6a69419f8960 100644 > --- a/drivers/thermal/mtk_thermal.c > +++ b/drivers/thermal/mtk_thermal.c > @@ -150,6 +150,21 @@ > #define CALIB_BUF1_VALID_V2(x) (((x) >> 4) & 0x1) > #define CALIB_BUF1_O_SLOPE_SIGN_V2(x) (((x) >> 3) & 0x1) > > +/* > + * Layout of the fuses providing the calibration data > + * These macros can be used for MT7981 and MT7986. > + */ > +#define CALIB_BUF0_ADC_GE_V3(x) (((x) >> 0) & 0x3ff) > +#define CALIB_BUF0_ADC_OE_V3(x) (((x) >> 10) & 0x3ff) > +#define CALIB_BUF0_DEGC_CALI_V3(x) (((x) >> 20) & 0x3f) > +#define CALIB_BUF0_O_SLOPE_V3(x) (((x) >> 26) & 0x3f) > +#define CALIB_BUF1_VTS_TS1_V3(x) (((x) >> 0) & 0x1ff) > +#define CALIB_BUF1_VTS_TS2_V3(x) (((x) >> 21) & 0x1ff) > +#define CALIB_BUF1_VTS_TSABB_V3(x) (((x) >> 9) & 0x1ff) > +#define CALIB_BUF1_VALID_V3(x) (((x) >> 18) & 0x1) > +#define CALIB_BUF1_O_SLOPE_SIGN_V3(x) (((x) >> 19) & 0x1) > +#define CALIB_BUF1_ID_V3(x) (((x) >> 20) & 0x1) > + > enum { > VTS1, > VTS2, > @@ -163,6 +178,7 @@ enum { > enum mtk_thermal_version { > MTK_THERMAL_V1 = 1, > MTK_THERMAL_V2, > + MTK_THERMAL_V3, > }; > > /* MT2701 thermal sensors */ > @@ -245,6 +261,27 @@ enum mtk_thermal_version { > /* The calibration coefficient of sensor */ > #define MT8183_CALIBRATION 153 > > +/* AUXADC channel 11 is used for the temperature sensors */ > +#define MT7986_TEMP_AUXADC_CHANNEL 11 > + > +/* The total number of temperature sensors in the MT7986 */ > +#define MT7986_NUM_SENSORS 1 > + > +/* The number of banks in the MT7986 */ > +#define MT7986_NUM_ZONES 1 > + > +/* The number of sensing points per bank */ > +#define MT7986_NUM_SENSORS_PER_ZONE 1 > + > +/* MT7986 thermal sensors */ > +#define MT7986_TS1 0 > + > +/* The number of controller in the MT7986 */ > +#define MT7986_NUM_CONTROLLER 1 > + > +/* The calibration coefficient of sensor */ > +#define MT7986_CALIBRATION 165 > + > struct mtk_thermal; > > struct thermal_bank_cfg { > @@ -386,6 +423,14 @@ static const int mt7622_mux_values[MT7622_NUM_SENSORS] = { 0, }; > static const int mt7622_vts_index[MT7622_NUM_SENSORS] = { VTS1 }; > static const int mt7622_tc_offset[MT7622_NUM_CONTROLLER] = { 0x0, }; > > +/* MT7986 thermal sensor data */ > +static const int mt7986_bank_data[MT7986_NUM_SENSORS] = { MT7986_TS1, }; > +static const int mt7986_msr[MT7986_NUM_SENSORS_PER_ZONE] = { TEMP_MSR0, }; > +static const int mt7986_adcpnp[MT7986_NUM_SENSORS_PER_ZONE] = { TEMP_ADCPNP0, }; > +static const int mt7986_mux_values[MT7986_NUM_SENSORS] = { 0, }; > +static const int mt7986_vts_index[MT7986_NUM_SENSORS] = { VTS1 }; > +static const int mt7986_tc_offset[MT7986_NUM_CONTROLLER] = { 0x0, }; > + > /* > * The MT8173 thermal controller has four banks. Each bank can read up to > * four temperature sensors simultaneously. The MT8173 has a total of 5 > @@ -549,6 +594,30 @@ static const struct mtk_thermal_data mt8183_thermal_data = { > .version = MTK_THERMAL_V1, > }; > > +/* > + * MT7986 uses AUXADC Channel 11 for raw data access. > + */ > +static const struct mtk_thermal_data mt7986_thermal_data = { > + .auxadc_channel = MT7986_TEMP_AUXADC_CHANNEL, > + .num_banks = MT7986_NUM_ZONES, > + .num_sensors = MT7986_NUM_SENSORS, > + .vts_index = mt7986_vts_index, > + .cali_val = MT7986_CALIBRATION, > + .num_controller = MT7986_NUM_CONTROLLER, > + .controller_offset = mt7986_tc_offset, > + .need_switch_bank = true, > + .bank_data = { > + { > + .num_sensors = 1, > + .sensors = mt7986_bank_data, > + }, > + }, > + .msr = mt7986_msr, > + .adcpnp = mt7986_adcpnp, > + .sensor_mux_values = mt7986_mux_values, > + .version = MTK_THERMAL_V3, > +}; > + > /** > * raw_to_mcelsius - convert a raw ADC value to mcelsius > * @mt: The thermal controller > @@ -603,6 +672,22 @@ static int raw_to_mcelsius_v2(struct mtk_thermal *mt, int sensno, s32 raw) > return (format_2 - tmp) * 100; > } > > +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; > + > + return mt->degc_cali * 500 - tmp; > +} > + > /** > * mtk_thermal_get_bank - get bank > * @bank: The bank > @@ -659,9 +744,12 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank) > if (mt->conf->version == MTK_THERMAL_V1) { > temp = raw_to_mcelsius_v1( > mt, conf->bank_data[bank->id].sensors[i], raw); > - } else { > + } else if (mt->conf->version == MTK_THERMAL_V2) { > temp = raw_to_mcelsius_v2( > mt, conf->bank_data[bank->id].sensors[i], raw); > + } else { > + temp = raw_to_mcelsius_v3( > + mt, conf->bank_data[bank->id].sensors[i], raw); > } What about optimizing this with assigning a function pointer? Like that, we wouldn't check any version in there... as in that case we'd simply do something like temp = conf->raw_to_mcelsius(mt, conf->bank...blahblah...); ...and this would also mean that the snippet saying "the first read of a sensor often contains very high bogus temperature value [...]" would get merged in the v2 of raw_to_mcelsius (as that function is used only in mtk_thermal_bank_temperature). > > /* > @@ -887,6 +975,26 @@ static int mtk_thermal_extract_efuse_v2(struct mtk_thermal *mt, u32 *buf) > return 0; > } > > +static int mtk_thermal_extract_efuse_v3(struct mtk_thermal *mt, u32 *buf) > +{ > + if (!CALIB_BUF1_VALID_V3(buf[1])) > + return -EINVAL; > + > + mt->adc_oe = CALIB_BUF0_ADC_OE_V3(buf[0]); > + mt->adc_ge = CALIB_BUF0_ADC_GE_V3(buf[0]); > + mt->degc_cali = CALIB_BUF0_DEGC_CALI_V3(buf[0]); > + mt->o_slope = CALIB_BUF0_O_SLOPE_V3(buf[0]); > + mt->vts[VTS1] = CALIB_BUF1_VTS_TS1_V3(buf[1]); > + mt->vts[VTS2] = CALIB_BUF1_VTS_TS2_V3(buf[1]); > + mt->vts[VTSABB] = CALIB_BUF1_VTS_TSABB_V3(buf[1]); > + mt->o_slope_sign = CALIB_BUF1_O_SLOPE_SIGN_V3(buf[1]); > + > + if (CALIB_BUF1_ID_V3(buf[1]) == 0) > + mt->o_slope = 0; > + > + return 0; > +} > + > static int mtk_thermal_get_calibration_data(struct device *dev, > struct mtk_thermal *mt) > { > @@ -897,6 +1005,7 @@ static int mtk_thermal_get_calibration_data(struct device *dev, > > /* Start with default values */ > mt->adc_ge = 512; > + mt->adc_oe = 512; > for (i = 0; i < mt->conf->num_sensors; i++) > mt->vts[i] = 260; > mt->degc_cali = 40; > @@ -924,8 +1033,10 @@ static int mtk_thermal_get_calibration_data(struct device *dev, > > if (mt->conf->version == MTK_THERMAL_V1) > ret = mtk_thermal_extract_efuse_v1(mt, buf); > - else > + else if (mt->conf->version == MTK_THERMAL_V2) > ret = mtk_thermal_extract_efuse_v2(mt, buf); > + else > + ret = mtk_thermal_extract_efuse_v3(mt, buf); I propose to use a switch here instead. switch(mt->conf->version) { case MTK_THERMAL_V1: .... case MTK_THERMAL_V2: .... case MTK_THERMAL_V3: .... default: ret = -EINVAL; break; }; This would also prevent a potential issue with getting an invalid calibration due to us calling the wrong version of the get_calibration() function, in which case, using the default calibration values would be at that point preferred. Regards, Angelo