| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0b72a12c-286f-79d0-09e9-b1761530850a@collabora.com>
Date: Thu, 1 Dec 2022 14:24:17 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Daniel Golle <daniel@...rotopia.org>, linux-pm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
"Rafael J. Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Rob Herring <robh+dt@...nel.org>
Cc: Steven Liu <steven.liu@...iatek.com>,
Henry Yen <Henry.Yen@...iatek.com>
Subject: Re: [PATCH v2 1/2] thermal: mediatek: add support for MT7986 and
MT7981
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 <daniel@...rotopia.org>
> Reviewed-by: Henry Yen <henry.yen@...iatek.com>
> ---
> 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
Powered by blists - more mailing lists