* Re: [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2022-07-21 21:39 kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2022-07-21 21:39 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 13853 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220720181854.547881-5-aouledameur@baylibre.com>
References: <20220720181854.547881-5-aouledameur@baylibre.com>
TO: "Amjad Ouled-Ameur" <aouledameur@baylibre.com>
TO: matthias.bgg(a)gmail.com
CC: rafael(a)kernel.org
CC: fparent(a)baylibre.com
CC: amitk(a)kernel.org
CC: daniel.lezcano(a)linaro.org
CC: devicetree(a)vger.kernel.org
CC: krzysztof.kozlowski+dt(a)linaro.org
CC: linux-arm-kernel(a)lists.infradead.org
CC: linux-kernel(a)vger.kernel.org
CC: linux-mediatek(a)lists.infradead.org
CC: linux-pm(a)vger.kernel.org
CC: robh+dt(a)kernel.org
CC: rui.zhang(a)intel.com
CC: "Amjad Ouled-Ameur" <aouledameur@baylibre.com>
CC: default avatarMichael Kao <michael.kao@mediatek.com>
CC: "default avatarHsin-Yi Wang" <hsinyi@chromium.org>
Hi Amjad,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rafael-pm/thermal]
[also build test WARNING on clk/clk-next soc/for-next linus/master v5.19-rc7 next-20220721]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Amjad-Ouled-Ameur/thermal-mediatek-Add-support-for-MT8365-SoC/20220721-022321
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
:::::: branch date: 27 hours ago
:::::: commit date: 27 hours ago
config: ia64-randconfig-m041-20220718 (https://download.01.org/0day-ci/archive/20220722/202207220518.s4Tr2WWY-lkp(a)intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/thermal/mtk_thermal.c:1223 mtk_thermal_probe() error: uninitialized symbol 'tzdev'.
vim +/tzdev +1223 drivers/thermal/mtk_thermal.c
89945047b1666c Henry Yen 2020-04-30 1098
a92db1c8089e82 Sascha Hauer 2015-11-30 1099 static int mtk_thermal_probe(struct platform_device *pdev)
a92db1c8089e82 Sascha Hauer 2015-11-30 1100 {
bd9403943d7d19 Michael Kao 2019-02-01 1101 int ret, i, ctrl_id;
a92db1c8089e82 Sascha Hauer 2015-11-30 1102 struct device_node *auxadc, *apmixedsys, *np = pdev->dev.of_node;
a92db1c8089e82 Sascha Hauer 2015-11-30 1103 struct mtk_thermal *mt;
a92db1c8089e82 Sascha Hauer 2015-11-30 1104 struct resource *res;
a92db1c8089e82 Sascha Hauer 2015-11-30 1105 u64 auxadc_phys_base, apmixed_phys_base;
1f6b0889d0ea03 Axel Lin 2016-09-07 1106 struct thermal_zone_device *tzdev;
89945047b1666c Henry Yen 2020-04-30 1107 void __iomem *apmixed_base, *auxadc_base;
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1108 struct mtk_thermal_zone *tz;
a92db1c8089e82 Sascha Hauer 2015-11-30 1109
a92db1c8089e82 Sascha Hauer 2015-11-30 1110 mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
a92db1c8089e82 Sascha Hauer 2015-11-30 1111 if (!mt)
a92db1c8089e82 Sascha Hauer 2015-11-30 1112 return -ENOMEM;
a92db1c8089e82 Sascha Hauer 2015-11-30 1113
9efc58dfa14a20 Ryder Lee 2018-04-16 1114 mt->conf = of_device_get_match_data(&pdev->dev);
b7cf0053738c54 dawei.chien(a)mediatek.com 2016-08-18 1115
a92db1c8089e82 Sascha Hauer 2015-11-30 1116 mt->clk_peri_therm = devm_clk_get(&pdev->dev, "therm");
a92db1c8089e82 Sascha Hauer 2015-11-30 1117 if (IS_ERR(mt->clk_peri_therm))
a92db1c8089e82 Sascha Hauer 2015-11-30 1118 return PTR_ERR(mt->clk_peri_therm);
a92db1c8089e82 Sascha Hauer 2015-11-30 1119
a92db1c8089e82 Sascha Hauer 2015-11-30 1120 mt->clk_auxadc = devm_clk_get(&pdev->dev, "auxadc");
a92db1c8089e82 Sascha Hauer 2015-11-30 1121 if (IS_ERR(mt->clk_auxadc))
a92db1c8089e82 Sascha Hauer 2015-11-30 1122 return PTR_ERR(mt->clk_auxadc);
a92db1c8089e82 Sascha Hauer 2015-11-30 1123
a92db1c8089e82 Sascha Hauer 2015-11-30 1124 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
a92db1c8089e82 Sascha Hauer 2015-11-30 1125 mt->thermal_base = devm_ioremap_resource(&pdev->dev, res);
a92db1c8089e82 Sascha Hauer 2015-11-30 1126 if (IS_ERR(mt->thermal_base))
a92db1c8089e82 Sascha Hauer 2015-11-30 1127 return PTR_ERR(mt->thermal_base);
a92db1c8089e82 Sascha Hauer 2015-11-30 1128
a92db1c8089e82 Sascha Hauer 2015-11-30 1129 ret = mtk_thermal_get_calibration_data(&pdev->dev, mt);
a92db1c8089e82 Sascha Hauer 2015-11-30 1130 if (ret)
a92db1c8089e82 Sascha Hauer 2015-11-30 1131 return ret;
a92db1c8089e82 Sascha Hauer 2015-11-30 1132
a92db1c8089e82 Sascha Hauer 2015-11-30 1133 mutex_init(&mt->lock);
a92db1c8089e82 Sascha Hauer 2015-11-30 1134
a92db1c8089e82 Sascha Hauer 2015-11-30 1135 mt->dev = &pdev->dev;
a92db1c8089e82 Sascha Hauer 2015-11-30 1136
a92db1c8089e82 Sascha Hauer 2015-11-30 1137 auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
a92db1c8089e82 Sascha Hauer 2015-11-30 1138 if (!auxadc) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1139 dev_err(&pdev->dev, "missing auxadc node\n");
a92db1c8089e82 Sascha Hauer 2015-11-30 1140 return -ENODEV;
a92db1c8089e82 Sascha Hauer 2015-11-30 1141 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1142
89945047b1666c Henry Yen 2020-04-30 1143 auxadc_base = of_iomap(auxadc, 0);
a92db1c8089e82 Sascha Hauer 2015-11-30 1144 auxadc_phys_base = of_get_phys_base(auxadc);
a92db1c8089e82 Sascha Hauer 2015-11-30 1145
a92db1c8089e82 Sascha Hauer 2015-11-30 1146 of_node_put(auxadc);
a92db1c8089e82 Sascha Hauer 2015-11-30 1147
a92db1c8089e82 Sascha Hauer 2015-11-30 1148 if (auxadc_phys_base == OF_BAD_ADDR) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1149 dev_err(&pdev->dev, "Can't get auxadc phys address\n");
a92db1c8089e82 Sascha Hauer 2015-11-30 1150 return -EINVAL;
a92db1c8089e82 Sascha Hauer 2015-11-30 1151 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1152
a92db1c8089e82 Sascha Hauer 2015-11-30 1153 apmixedsys = of_parse_phandle(np, "mediatek,apmixedsys", 0);
a92db1c8089e82 Sascha Hauer 2015-11-30 1154 if (!apmixedsys) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1155 dev_err(&pdev->dev, "missing apmixedsys node\n");
a92db1c8089e82 Sascha Hauer 2015-11-30 1156 return -ENODEV;
a92db1c8089e82 Sascha Hauer 2015-11-30 1157 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1158
89945047b1666c Henry Yen 2020-04-30 1159 apmixed_base = of_iomap(apmixedsys, 0);
a92db1c8089e82 Sascha Hauer 2015-11-30 1160 apmixed_phys_base = of_get_phys_base(apmixedsys);
a92db1c8089e82 Sascha Hauer 2015-11-30 1161
a92db1c8089e82 Sascha Hauer 2015-11-30 1162 of_node_put(apmixedsys);
a92db1c8089e82 Sascha Hauer 2015-11-30 1163
a92db1c8089e82 Sascha Hauer 2015-11-30 1164 if (apmixed_phys_base == OF_BAD_ADDR) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1165 dev_err(&pdev->dev, "Can't get auxadc phys address\n");
a92db1c8089e82 Sascha Hauer 2015-11-30 1166 return -EINVAL;
a92db1c8089e82 Sascha Hauer 2015-11-30 1167 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1168
703456ba76e944 Fabien Parent 2020-10-21 1169 ret = device_reset_optional(&pdev->dev);
6760f3f74e72d6 Louis Yu 2017-08-01 1170 if (ret)
6760f3f74e72d6 Louis Yu 2017-08-01 1171 return ret;
6760f3f74e72d6 Louis Yu 2017-08-01 1172
a92db1c8089e82 Sascha Hauer 2015-11-30 1173 ret = clk_prepare_enable(mt->clk_auxadc);
a92db1c8089e82 Sascha Hauer 2015-11-30 1174 if (ret) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1175 dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
a92db1c8089e82 Sascha Hauer 2015-11-30 1176 return ret;
a92db1c8089e82 Sascha Hauer 2015-11-30 1177 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1178
a92db1c8089e82 Sascha Hauer 2015-11-30 1179 ret = clk_prepare_enable(mt->clk_peri_therm);
a92db1c8089e82 Sascha Hauer 2015-11-30 1180 if (ret) {
a92db1c8089e82 Sascha Hauer 2015-11-30 1181 dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
a92db1c8089e82 Sascha Hauer 2015-11-30 1182 goto err_disable_clk_auxadc;
a92db1c8089e82 Sascha Hauer 2015-11-30 1183 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1184
b91b391dd49f5d Markus Schneider-Pargmann 2022-07-20 1185 mtk_thermal_turn_on_buffer(mt, apmixed_base);
b91b391dd49f5d Markus Schneider-Pargmann 2022-07-20 1186
89945047b1666c Henry Yen 2020-04-30 1187 if (mt->conf->version == MTK_THERMAL_V2) {
89945047b1666c Henry Yen 2020-04-30 1188 mtk_thermal_release_periodic_ts(mt, auxadc_base);
89945047b1666c Henry Yen 2020-04-30 1189 }
89945047b1666c Henry Yen 2020-04-30 1190
bd9403943d7d19 Michael Kao 2019-02-01 1191 for (ctrl_id = 0; ctrl_id < mt->conf->num_controller ; ctrl_id++)
b7cf0053738c54 dawei.chien(a)mediatek.com 2016-08-18 1192 for (i = 0; i < mt->conf->num_banks; i++)
eb4fc33eb26807 Eduardo Valentin 2016-02-18 1193 mtk_thermal_init_bank(mt, i, apmixed_phys_base,
bd9403943d7d19 Michael Kao 2019-02-01 1194 auxadc_phys_base, ctrl_id);
a92db1c8089e82 Sascha Hauer 2015-11-30 1195
a92db1c8089e82 Sascha Hauer 2015-11-30 1196 platform_set_drvdata(pdev, mt);
a92db1c8089e82 Sascha Hauer 2015-11-30 1197
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1198 for (i = 0; i < mt->conf->num_sensors + 1; i++) {
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1199 tz = kmalloc(sizeof(*tz), GFP_KERNEL);
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1200 if (!tz)
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1201 return -ENOMEM;
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1202
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1203 tz->mt = mt;
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1204 tz->id = i;
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1205
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1206 tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i, tz, (i == 0) ?
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1207 &mtk_thermal_ops :
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1208 &mtk_thermal_sensor_ops);
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1209
1f6b0889d0ea03 Axel Lin 2016-09-07 1210 if (IS_ERR(tzdev)) {
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1211 if (PTR_ERR(tzdev) == -ENODEV) {
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1212 dev_warn(&pdev->dev,
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1213 "sensor %d not registered in thermal zone in dt\n", i);
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1214 continue;
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1215 }
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1216 if (PTR_ERR(tzdev) == -EACCES) {
1f6b0889d0ea03 Axel Lin 2016-09-07 1217 ret = PTR_ERR(tzdev);
1f6b0889d0ea03 Axel Lin 2016-09-07 1218 goto err_disable_clk_peri_therm;
1f6b0889d0ea03 Axel Lin 2016-09-07 1219 }
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1220 }
f478b3ee651af2 Amjad Ouled-Ameur 2022-07-20 1221 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1222
24e21d9f40ec3f Frank Wunderlich 2021-06-08 @1223 ret = devm_thermal_add_hwmon_sysfs(tzdev);
24e21d9f40ec3f Frank Wunderlich 2021-06-08 1224 if (ret)
24e21d9f40ec3f Frank Wunderlich 2021-06-08 1225 dev_warn(&pdev->dev, "error in thermal_add_hwmon_sysfs");
24e21d9f40ec3f Frank Wunderlich 2021-06-08 1226
a92db1c8089e82 Sascha Hauer 2015-11-30 1227 return 0;
a92db1c8089e82 Sascha Hauer 2015-11-30 1228
1f6b0889d0ea03 Axel Lin 2016-09-07 1229 err_disable_clk_peri_therm:
1f6b0889d0ea03 Axel Lin 2016-09-07 1230 clk_disable_unprepare(mt->clk_peri_therm);
a92db1c8089e82 Sascha Hauer 2015-11-30 1231 err_disable_clk_auxadc:
a92db1c8089e82 Sascha Hauer 2015-11-30 1232 clk_disable_unprepare(mt->clk_auxadc);
a92db1c8089e82 Sascha Hauer 2015-11-30 1233
a92db1c8089e82 Sascha Hauer 2015-11-30 1234 return ret;
a92db1c8089e82 Sascha Hauer 2015-11-30 1235 }
a92db1c8089e82 Sascha Hauer 2015-11-30 1236
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
2022-07-20 18:54 ` Christophe JAILLET
@ 2022-08-26 14:12 ` Amjad Ouled-Ameur
-1 siblings, 0 replies; 7+ messages in thread
From: Amjad Ouled-Ameur @ 2022-08-26 14:12 UTC (permalink / raw)
To: Christophe JAILLET
Cc: amitk, daniel.lezcano, devicetree, fparent, hsinyi,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, matthias.bgg, michael.kao, rafael,
robh+dt, rui.zhang
Hi Christophe,
On 7/20/22 20:54, Christophe JAILLET wrote:
> Le 20/07/2022 à 20:18, Amjad Ouled-Ameur a écrit :
>> Provide thermal zone to read thermal sensor in the SoC. We can read
>> all the
>> thermal sensors value in the SoC by the node /sys/class/thermal/
>>
>> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
>> on the first read of sensor that often are bogus values.
>> This can avoid following warning on boot:
>>
>> thermal thermal_zone6: failed to read out thermal zone (-13)
>>
>> Signed-off-by: default avatarMichael Kao
>> <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: default avatarHsin-Yi Wang
>> <hsinyi-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Amjad Ouled-Ameur
>> <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> Tested-by: Amjad Ouled-Ameur
>> <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>> drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
>> 1 file changed, 76 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/thermal/mtk_thermal.c
>> b/drivers/thermal/mtk_thermal.c
>> index 1dc276f8c4f1..79b14ce1a08d 100644
>> --- a/drivers/thermal/mtk_thermal.c
>> +++ b/drivers/thermal/mtk_thermal.c
>> @@ -259,6 +259,11 @@ enum mtk_thermal_version {
>> struct mtk_thermal;
>> +struct mtk_thermal_zone {
>> + struct mtk_thermal *mt;
>> + int id;
>> +};
>> +
>> struct thermal_bank_cfg {
>> unsigned int num_sensors;
>> const int *sensors;
>> @@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct
>> mtk_thermal_bank *bank)
>> mutex_unlock(&mt->lock);
>> }
>> +static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
>> +{
>> + u32 raw;
>> + int temp;
>> +
>> + const struct mtk_thermal_data *conf = mt->conf;
>> +
>> + raw = readl(mt->thermal_base + conf->msr[id]);
>> +
>> + if (mt->conf->version == MTK_THERMAL_V1)
>> + temp = raw_to_mcelsius_v1(mt, id, raw);
>> + else
>> + temp = raw_to_mcelsius_v2(mt, id, raw);
>> +
>> + /*
>> + * The first read of a sensor often contains very high bogus
>> + * temperature value. Filter these out so that the system does
>> + * not immediately shut down.
>> + */
>> +
>> + if (temp > 200000)
>> + return -EAGAIN;
>
> This function returns a u32. Is it ok to return -EAGAIN?
>
_get_sensor_temp() should normally return int instead u32, will fix it
in V3.
> There is also 2 spaces here...
>
>> + else
>> + return temp;
>
> ... and a tab here.
>
will fix them in V3.
>> +}
>> +
>> /**
>> * mtk_thermal_bank_temperature - get the temperature of a bank
>> * @bank: The bank
>> @@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct
>> mtk_thermal_bank *bank)
>> struct mtk_thermal *mt = bank->mt;
>> const struct mtk_thermal_data *conf = mt->conf;
>> int i, temp = INT_MIN, max = INT_MIN;
>> - u32 raw;
>> for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
>> - raw = readl(mt->thermal_base + conf->msr[i]);
>> -
>> - if (mt->conf->version == MTK_THERMAL_V1) {
>> - temp = raw_to_mcelsius_v1(
>> - mt, conf->bank_data[bank->id].sensors[i], raw);
>> - } else {
>> - temp = raw_to_mcelsius_v2(
>> - mt, conf->bank_data[bank->id].sensors[i], raw);
>> - }
>> -
>> - /*
>> - * The first read of a sensor often contains very high bogus
>> - * temperature value. Filter these out so that the system does
>> - * not immediately shut down.
>> - */
>> - if (temp > 200000)
>> - temp = 0;
>> + temp = _get_sensor_temp(mt, i);
>
> Is it ok if _get_sensor_temp() returns -EAGAIN?
>
drivers/thermal/thermal_core.c:update_temperature() checks for -EAGAIN
after
thermal_zone_get_temp() is called, thus, I think it's good to return
-EAGAIN in
case of failure.
>> if (temp > max)
>> max = temp;
>> @@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct
>> mtk_thermal_bank *bank)
>> static int mtk_read_temp(void *data, int *temperature)
>> {
>> - struct mtk_thermal *mt = data;
>> + struct mtk_thermal_zone *tz = data;
>> + struct mtk_thermal *mt = tz->mt;
>> int i;
>> int tempmax = INT_MIN;
>> @@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int
>> *temperature)
>> return 0;
>> }
>> +static int mtk_read_sensor_temp(void *data, int *temperature)
>> +{
>> + struct mtk_thermal_zone *tz = data;
>> + struct mtk_thermal *mt = tz->mt;
>> + int id = tz->id - 1;
>> +
>> + if (id < 0)
>> + return -EACCES;
>
> 2 spaces.
>
will fix it in V3.
>> +
>> + *temperature = _get_sensor_temp(mt, id);
>
> If _get_sensor_temp() returns -EAGAIN, should this be propagated to
> the caller?
>
>> +
>> + return 0;
>> +}
>> +
>> static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>> .get_temp = mtk_read_temp,
>> };
>> +static const struct thermal_zone_of_device_ops
>> mtk_thermal_sensor_ops = {
>> + .get_temp = mtk_read_sensor_temp,
>> +};
>> +
>> static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>> u32 apmixed_phys_base, u32 auxadc_phys_base,
>> int ctrl_id)
>> @@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct
>> platform_device *pdev)
>> u64 auxadc_phys_base, apmixed_phys_base;
>> struct thermal_zone_device *tzdev;
>> void __iomem *apmixed_base, *auxadc_base;
>> + struct mtk_thermal_zone *tz;
>> mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>> if (!mt)
>> @@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct
>> platform_device *pdev)
>> platform_set_drvdata(pdev, mt);
>> - tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
>> - &mtk_thermal_ops);
>> - if (IS_ERR(tzdev)) {
>> - ret = PTR_ERR(tzdev);
>> - goto err_disable_clk_peri_therm;
>> + for (i = 0; i < mt->conf->num_sensors + 1; i++) {
>> + tz = kmalloc(sizeof(*tz), GFP_KERNEL);
>
> Should this memory allocation be a devm_kmalloc(), or is this memory
> freed at some point by the framework?
>
> (I don't know the thermal_zone API and the patch has no kfree())
AFAIK, thermal API does not free private data, therefore devm_kmalloc()
should be used.
>
> CJ
>
>> + if (!tz)
>> + return -ENOMEM;
>> +
>> + tz->mt = mt;
>> + tz->id = i;
>> +
>> + tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
>> tz, (i == 0) ?
>> + &mtk_thermal_ops :
>> + &mtk_thermal_sensor_ops);
>> +
>> + if (IS_ERR(tzdev)) {
>> + if (PTR_ERR(tzdev) == -ENODEV) {
>> + dev_warn(&pdev->dev,
>> + "sensor %d not registered in thermal zone in
>> dt\n", i);
>> + continue;
>> + }
>> + if (PTR_ERR(tzdev) == -EACCES) {
>> + ret = PTR_ERR(tzdev);
>> + goto err_disable_clk_peri_therm;
>> + }
>> + }
>> }
>> ret = devm_thermal_add_hwmon_sysfs(tzdev);
>
Thank you Christophe for the review.
Regards,
Amjad
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2022-08-26 14:12 ` Amjad Ouled-Ameur
0 siblings, 0 replies; 7+ messages in thread
From: Amjad Ouled-Ameur @ 2022-08-26 14:12 UTC (permalink / raw)
To: Christophe JAILLET
Cc: amitk, daniel.lezcano, devicetree, fparent, hsinyi,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, matthias.bgg, michael.kao, rafael,
robh+dt, rui.zhang
Hi Christophe,
On 7/20/22 20:54, Christophe JAILLET wrote:
> Le 20/07/2022 à 20:18, Amjad Ouled-Ameur a écrit :
>> Provide thermal zone to read thermal sensor in the SoC. We can read
>> all the
>> thermal sensors value in the SoC by the node /sys/class/thermal/
>>
>> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
>> on the first read of sensor that often are bogus values.
>> This can avoid following warning on boot:
>>
>> thermal thermal_zone6: failed to read out thermal zone (-13)
>>
>> Signed-off-by: default avatarMichael Kao
>> <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: default avatarHsin-Yi Wang
>> <hsinyi-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Amjad Ouled-Ameur
>> <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> Tested-by: Amjad Ouled-Ameur
>> <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>> drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
>> 1 file changed, 76 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/thermal/mtk_thermal.c
>> b/drivers/thermal/mtk_thermal.c
>> index 1dc276f8c4f1..79b14ce1a08d 100644
>> --- a/drivers/thermal/mtk_thermal.c
>> +++ b/drivers/thermal/mtk_thermal.c
>> @@ -259,6 +259,11 @@ enum mtk_thermal_version {
>> struct mtk_thermal;
>> +struct mtk_thermal_zone {
>> + struct mtk_thermal *mt;
>> + int id;
>> +};
>> +
>> struct thermal_bank_cfg {
>> unsigned int num_sensors;
>> const int *sensors;
>> @@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct
>> mtk_thermal_bank *bank)
>> mutex_unlock(&mt->lock);
>> }
>> +static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
>> +{
>> + u32 raw;
>> + int temp;
>> +
>> + const struct mtk_thermal_data *conf = mt->conf;
>> +
>> + raw = readl(mt->thermal_base + conf->msr[id]);
>> +
>> + if (mt->conf->version == MTK_THERMAL_V1)
>> + temp = raw_to_mcelsius_v1(mt, id, raw);
>> + else
>> + temp = raw_to_mcelsius_v2(mt, id, raw);
>> +
>> + /*
>> + * The first read of a sensor often contains very high bogus
>> + * temperature value. Filter these out so that the system does
>> + * not immediately shut down.
>> + */
>> +
>> + if (temp > 200000)
>> + return -EAGAIN;
>
> This function returns a u32. Is it ok to return -EAGAIN?
>
_get_sensor_temp() should normally return int instead u32, will fix it
in V3.
> There is also 2 spaces here...
>
>> + else
>> + return temp;
>
> ... and a tab here.
>
will fix them in V3.
>> +}
>> +
>> /**
>> * mtk_thermal_bank_temperature - get the temperature of a bank
>> * @bank: The bank
>> @@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct
>> mtk_thermal_bank *bank)
>> struct mtk_thermal *mt = bank->mt;
>> const struct mtk_thermal_data *conf = mt->conf;
>> int i, temp = INT_MIN, max = INT_MIN;
>> - u32 raw;
>> for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
>> - raw = readl(mt->thermal_base + conf->msr[i]);
>> -
>> - if (mt->conf->version == MTK_THERMAL_V1) {
>> - temp = raw_to_mcelsius_v1(
>> - mt, conf->bank_data[bank->id].sensors[i], raw);
>> - } else {
>> - temp = raw_to_mcelsius_v2(
>> - mt, conf->bank_data[bank->id].sensors[i], raw);
>> - }
>> -
>> - /*
>> - * The first read of a sensor often contains very high bogus
>> - * temperature value. Filter these out so that the system does
>> - * not immediately shut down.
>> - */
>> - if (temp > 200000)
>> - temp = 0;
>> + temp = _get_sensor_temp(mt, i);
>
> Is it ok if _get_sensor_temp() returns -EAGAIN?
>
drivers/thermal/thermal_core.c:update_temperature() checks for -EAGAIN
after
thermal_zone_get_temp() is called, thus, I think it's good to return
-EAGAIN in
case of failure.
>> if (temp > max)
>> max = temp;
>> @@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct
>> mtk_thermal_bank *bank)
>> static int mtk_read_temp(void *data, int *temperature)
>> {
>> - struct mtk_thermal *mt = data;
>> + struct mtk_thermal_zone *tz = data;
>> + struct mtk_thermal *mt = tz->mt;
>> int i;
>> int tempmax = INT_MIN;
>> @@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int
>> *temperature)
>> return 0;
>> }
>> +static int mtk_read_sensor_temp(void *data, int *temperature)
>> +{
>> + struct mtk_thermal_zone *tz = data;
>> + struct mtk_thermal *mt = tz->mt;
>> + int id = tz->id - 1;
>> +
>> + if (id < 0)
>> + return -EACCES;
>
> 2 spaces.
>
will fix it in V3.
>> +
>> + *temperature = _get_sensor_temp(mt, id);
>
> If _get_sensor_temp() returns -EAGAIN, should this be propagated to
> the caller?
>
>> +
>> + return 0;
>> +}
>> +
>> static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>> .get_temp = mtk_read_temp,
>> };
>> +static const struct thermal_zone_of_device_ops
>> mtk_thermal_sensor_ops = {
>> + .get_temp = mtk_read_sensor_temp,
>> +};
>> +
>> static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>> u32 apmixed_phys_base, u32 auxadc_phys_base,
>> int ctrl_id)
>> @@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct
>> platform_device *pdev)
>> u64 auxadc_phys_base, apmixed_phys_base;
>> struct thermal_zone_device *tzdev;
>> void __iomem *apmixed_base, *auxadc_base;
>> + struct mtk_thermal_zone *tz;
>> mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>> if (!mt)
>> @@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct
>> platform_device *pdev)
>> platform_set_drvdata(pdev, mt);
>> - tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
>> - &mtk_thermal_ops);
>> - if (IS_ERR(tzdev)) {
>> - ret = PTR_ERR(tzdev);
>> - goto err_disable_clk_peri_therm;
>> + for (i = 0; i < mt->conf->num_sensors + 1; i++) {
>> + tz = kmalloc(sizeof(*tz), GFP_KERNEL);
>
> Should this memory allocation be a devm_kmalloc(), or is this memory
> freed at some point by the framework?
>
> (I don't know the thermal_zone API and the patch has no kfree())
AFAIK, thermal API does not free private data, therefore devm_kmalloc()
should be used.
>
> CJ
>
>> + if (!tz)
>> + return -ENOMEM;
>> +
>> + tz->mt = mt;
>> + tz->id = i;
>> +
>> + tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
>> tz, (i == 0) ?
>> + &mtk_thermal_ops :
>> + &mtk_thermal_sensor_ops);
>> +
>> + if (IS_ERR(tzdev)) {
>> + if (PTR_ERR(tzdev) == -ENODEV) {
>> + dev_warn(&pdev->dev,
>> + "sensor %d not registered in thermal zone in
>> dt\n", i);
>> + continue;
>> + }
>> + if (PTR_ERR(tzdev) == -EACCES) {
>> + ret = PTR_ERR(tzdev);
>> + goto err_disable_clk_peri_therm;
>> + }
>> + }
>> }
>> ret = devm_thermal_add_hwmon_sysfs(tzdev);
>
Thank you Christophe for the review.
Regards,
Amjad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
2022-07-20 18:18 ` Amjad Ouled-Ameur
@ 2022-07-20 18:54 ` Christophe JAILLET
-1 siblings, 0 replies; 7+ messages in thread
From: Christophe JAILLET @ 2022-07-20 18:54 UTC (permalink / raw)
To: aouledameur
Cc: amitk, daniel.lezcano, devicetree, fparent, hsinyi,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, matthias.bgg, michael.kao, rafael,
robh+dt, rui.zhang
Le 20/07/2022 à 20:18, Amjad Ouled-Ameur a écrit :
> Provide thermal zone to read thermal sensor in the SoC. We can read all the
> thermal sensors value in the SoC by the node /sys/class/thermal/
>
> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
> on the first read of sensor that often are bogus values.
> This can avoid following warning on boot:
>
> thermal thermal_zone6: failed to read out thermal zone (-13)
>
> Signed-off-by: default avatarMichael Kao <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: default avatarHsin-Yi Wang <hsinyi-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Amjad Ouled-Ameur <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Tested-by: Amjad Ouled-Ameur <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
> 1 file changed, 76 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 1dc276f8c4f1..79b14ce1a08d 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -259,6 +259,11 @@ enum mtk_thermal_version {
>
> struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> + struct mtk_thermal *mt;
> + int id;
> +};
> +
> struct thermal_bank_cfg {
> unsigned int num_sensors;
> const int *sensors;
> @@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> mutex_unlock(&mt->lock);
> }
>
> +static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
> +{
> + u32 raw;
> + int temp;
> +
> + const struct mtk_thermal_data *conf = mt->conf;
> +
> + raw = readl(mt->thermal_base + conf->msr[id]);
> +
> + if (mt->conf->version == MTK_THERMAL_V1)
> + temp = raw_to_mcelsius_v1(mt, id, raw);
> + else
> + temp = raw_to_mcelsius_v2(mt, id, raw);
> +
> + /*
> + * The first read of a sensor often contains very high bogus
> + * temperature value. Filter these out so that the system does
> + * not immediately shut down.
> + */
> +
> + if (temp > 200000)
> + return -EAGAIN;
This function returns a u32. Is it ok to return -EAGAIN?
There is also 2 spaces here...
> + else
> + return temp;
... and a tab here.
> +}
> +
> /**
> * mtk_thermal_bank_temperature - get the temperature of a bank
> * @bank: The bank
> @@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
> struct mtk_thermal *mt = bank->mt;
> const struct mtk_thermal_data *conf = mt->conf;
> int i, temp = INT_MIN, max = INT_MIN;
> - u32 raw;
>
> for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
> - raw = readl(mt->thermal_base + conf->msr[i]);
> -
> - if (mt->conf->version == MTK_THERMAL_V1) {
> - temp = raw_to_mcelsius_v1(
> - mt, conf->bank_data[bank->id].sensors[i], raw);
> - } else {
> - temp = raw_to_mcelsius_v2(
> - mt, conf->bank_data[bank->id].sensors[i], raw);
> - }
> -
> - /*
> - * The first read of a sensor often contains very high bogus
> - * temperature value. Filter these out so that the system does
> - * not immediately shut down.
> - */
> - if (temp > 200000)
> - temp = 0;
> + temp = _get_sensor_temp(mt, i);
Is it ok if _get_sensor_temp() returns -EAGAIN?
>
> if (temp > max)
> max = temp;
> @@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
> static int mtk_read_temp(void *data, int *temperature)
> {
> - struct mtk_thermal *mt = data;
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> int i;
> int tempmax = INT_MIN;
>
> @@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int *temperature)
> return 0;
> }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> + int id = tz->id - 1;
> +
> + if (id < 0)
> + return -EACCES;
2 spaces.
> +
> + *temperature = _get_sensor_temp(mt, id);
If _get_sensor_temp() returns -EAGAIN, should this be propagated to the
caller?
> +
> + return 0;
> +}
> +
> static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
> .get_temp = mtk_read_temp,
> };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> + .get_temp = mtk_read_sensor_temp,
> +};
> +
> static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
> u32 apmixed_phys_base, u32 auxadc_phys_base,
> int ctrl_id)
> @@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> u64 auxadc_phys_base, apmixed_phys_base;
> struct thermal_zone_device *tzdev;
> void __iomem *apmixed_base, *auxadc_base;
> + struct mtk_thermal_zone *tz;
>
> mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
> if (!mt)
> @@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, mt);
>
> - tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> - &mtk_thermal_ops);
> - if (IS_ERR(tzdev)) {
> - ret = PTR_ERR(tzdev);
> - goto err_disable_clk_peri_therm;
> + for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> + tz = kmalloc(sizeof(*tz), GFP_KERNEL);
Should this memory allocation be a devm_kmalloc(), or is this memory
freed at some point by the framework?
(I don't know the thermal_zone API and the patch has no kfree())
CJ
> + if (!tz)
> + return -ENOMEM;
> +
> + tz->mt = mt;
> + tz->id = i;
> +
> + tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i, tz, (i == 0) ?
> + &mtk_thermal_ops :
> + &mtk_thermal_sensor_ops);
> +
> + if (IS_ERR(tzdev)) {
> + if (PTR_ERR(tzdev) == -ENODEV) {
> + dev_warn(&pdev->dev,
> + "sensor %d not registered in thermal zone in dt\n", i);
> + continue;
> + }
> + if (PTR_ERR(tzdev) == -EACCES) {
> + ret = PTR_ERR(tzdev);
> + goto err_disable_clk_peri_therm;
> + }
> + }
> }
>
> ret = devm_thermal_add_hwmon_sysfs(tzdev);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2022-07-20 18:54 ` Christophe JAILLET
0 siblings, 0 replies; 7+ messages in thread
From: Christophe JAILLET @ 2022-07-20 18:54 UTC (permalink / raw)
To: aouledameur
Cc: amitk, daniel.lezcano, devicetree, fparent, hsinyi,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, matthias.bgg, michael.kao, rafael,
robh+dt, rui.zhang
Le 20/07/2022 à 20:18, Amjad Ouled-Ameur a écrit :
> Provide thermal zone to read thermal sensor in the SoC. We can read all the
> thermal sensors value in the SoC by the node /sys/class/thermal/
>
> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
> on the first read of sensor that often are bogus values.
> This can avoid following warning on boot:
>
> thermal thermal_zone6: failed to read out thermal zone (-13)
>
> Signed-off-by: default avatarMichael Kao <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: default avatarHsin-Yi Wang <hsinyi-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Amjad Ouled-Ameur <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Tested-by: Amjad Ouled-Ameur <aouledameur-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
> 1 file changed, 76 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 1dc276f8c4f1..79b14ce1a08d 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -259,6 +259,11 @@ enum mtk_thermal_version {
>
> struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> + struct mtk_thermal *mt;
> + int id;
> +};
> +
> struct thermal_bank_cfg {
> unsigned int num_sensors;
> const int *sensors;
> @@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> mutex_unlock(&mt->lock);
> }
>
> +static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
> +{
> + u32 raw;
> + int temp;
> +
> + const struct mtk_thermal_data *conf = mt->conf;
> +
> + raw = readl(mt->thermal_base + conf->msr[id]);
> +
> + if (mt->conf->version == MTK_THERMAL_V1)
> + temp = raw_to_mcelsius_v1(mt, id, raw);
> + else
> + temp = raw_to_mcelsius_v2(mt, id, raw);
> +
> + /*
> + * The first read of a sensor often contains very high bogus
> + * temperature value. Filter these out so that the system does
> + * not immediately shut down.
> + */
> +
> + if (temp > 200000)
> + return -EAGAIN;
This function returns a u32. Is it ok to return -EAGAIN?
There is also 2 spaces here...
> + else
> + return temp;
... and a tab here.
> +}
> +
> /**
> * mtk_thermal_bank_temperature - get the temperature of a bank
> * @bank: The bank
> @@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
> struct mtk_thermal *mt = bank->mt;
> const struct mtk_thermal_data *conf = mt->conf;
> int i, temp = INT_MIN, max = INT_MIN;
> - u32 raw;
>
> for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
> - raw = readl(mt->thermal_base + conf->msr[i]);
> -
> - if (mt->conf->version == MTK_THERMAL_V1) {
> - temp = raw_to_mcelsius_v1(
> - mt, conf->bank_data[bank->id].sensors[i], raw);
> - } else {
> - temp = raw_to_mcelsius_v2(
> - mt, conf->bank_data[bank->id].sensors[i], raw);
> - }
> -
> - /*
> - * The first read of a sensor often contains very high bogus
> - * temperature value. Filter these out so that the system does
> - * not immediately shut down.
> - */
> - if (temp > 200000)
> - temp = 0;
> + temp = _get_sensor_temp(mt, i);
Is it ok if _get_sensor_temp() returns -EAGAIN?
>
> if (temp > max)
> max = temp;
> @@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
> static int mtk_read_temp(void *data, int *temperature)
> {
> - struct mtk_thermal *mt = data;
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> int i;
> int tempmax = INT_MIN;
>
> @@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int *temperature)
> return 0;
> }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> + struct mtk_thermal_zone *tz = data;
> + struct mtk_thermal *mt = tz->mt;
> + int id = tz->id - 1;
> +
> + if (id < 0)
> + return -EACCES;
2 spaces.
> +
> + *temperature = _get_sensor_temp(mt, id);
If _get_sensor_temp() returns -EAGAIN, should this be propagated to the
caller?
> +
> + return 0;
> +}
> +
> static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
> .get_temp = mtk_read_temp,
> };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> + .get_temp = mtk_read_sensor_temp,
> +};
> +
> static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
> u32 apmixed_phys_base, u32 auxadc_phys_base,
> int ctrl_id)
> @@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> u64 auxadc_phys_base, apmixed_phys_base;
> struct thermal_zone_device *tzdev;
> void __iomem *apmixed_base, *auxadc_base;
> + struct mtk_thermal_zone *tz;
>
> mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
> if (!mt)
> @@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, mt);
>
> - tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> - &mtk_thermal_ops);
> - if (IS_ERR(tzdev)) {
> - ret = PTR_ERR(tzdev);
> - goto err_disable_clk_peri_therm;
> + for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> + tz = kmalloc(sizeof(*tz), GFP_KERNEL);
Should this memory allocation be a devm_kmalloc(), or is this memory
freed at some point by the framework?
(I don't know the thermal_zone API and the patch has no kfree())
CJ
> + if (!tz)
> + return -ENOMEM;
> +
> + tz->mt = mt;
> + tz->id = i;
> +
> + tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i, tz, (i == 0) ?
> + &mtk_thermal_ops :
> + &mtk_thermal_sensor_ops);
> +
> + if (IS_ERR(tzdev)) {
> + if (PTR_ERR(tzdev) == -ENODEV) {
> + dev_warn(&pdev->dev,
> + "sensor %d not registered in thermal zone in dt\n", i);
> + continue;
> + }
> + if (PTR_ERR(tzdev) == -EACCES) {
> + ret = PTR_ERR(tzdev);
> + goto err_disable_clk_peri_therm;
> + }
> + }
> }
>
> ret = devm_thermal_add_hwmon_sysfs(tzdev);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
2022-07-20 18:18 [PATCH v2 0/4] thermal: mediatek: Add support for MT8365 SoC Amjad Ouled-Ameur
@ 2022-07-20 18:18 ` Amjad Ouled-Ameur
0 siblings, 0 replies; 7+ messages in thread
From: Amjad Ouled-Ameur @ 2022-07-20 18:18 UTC (permalink / raw)
To: matthias.bgg
Cc: rafael, fparent, amitk, daniel.lezcano, devicetree,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, robh+dt, rui.zhang, Amjad Ouled-Ameur,
default avatarMichael Kao, default avatarHsin-Yi Wang
Provide thermal zone to read thermal sensor in the SoC. We can read all the
thermal sensors value in the SoC by the node /sys/class/thermal/
In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
on the first read of sensor that often are bogus values.
This can avoid following warning on boot:
thermal thermal_zone6: failed to read out thermal zone (-13)
Signed-off-by: default avatarMichael Kao <michael.kao@mediatek.com>
Signed-off-by: default avatarHsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
Tested-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
---
drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
1 file changed, 76 insertions(+), 24 deletions(-)
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 1dc276f8c4f1..79b14ce1a08d 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -259,6 +259,11 @@ enum mtk_thermal_version {
struct mtk_thermal;
+struct mtk_thermal_zone {
+ struct mtk_thermal *mt;
+ int id;
+};
+
struct thermal_bank_cfg {
unsigned int num_sensors;
const int *sensors;
@@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
mutex_unlock(&mt->lock);
}
+static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
+{
+ u32 raw;
+ int temp;
+
+ const struct mtk_thermal_data *conf = mt->conf;
+
+ raw = readl(mt->thermal_base + conf->msr[id]);
+
+ if (mt->conf->version == MTK_THERMAL_V1)
+ temp = raw_to_mcelsius_v1(mt, id, raw);
+ else
+ temp = raw_to_mcelsius_v2(mt, id, raw);
+
+ /*
+ * The first read of a sensor often contains very high bogus
+ * temperature value. Filter these out so that the system does
+ * not immediately shut down.
+ */
+
+ if (temp > 200000)
+ return -EAGAIN;
+ else
+ return temp;
+}
+
/**
* mtk_thermal_bank_temperature - get the temperature of a bank
* @bank: The bank
@@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
struct mtk_thermal *mt = bank->mt;
const struct mtk_thermal_data *conf = mt->conf;
int i, temp = INT_MIN, max = INT_MIN;
- u32 raw;
for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
- raw = readl(mt->thermal_base + conf->msr[i]);
-
- if (mt->conf->version == MTK_THERMAL_V1) {
- temp = raw_to_mcelsius_v1(
- mt, conf->bank_data[bank->id].sensors[i], raw);
- } else {
- temp = raw_to_mcelsius_v2(
- mt, conf->bank_data[bank->id].sensors[i], raw);
- }
-
- /*
- * The first read of a sensor often contains very high bogus
- * temperature value. Filter these out so that the system does
- * not immediately shut down.
- */
- if (temp > 200000)
- temp = 0;
+ temp = _get_sensor_temp(mt, i);
if (temp > max)
max = temp;
@@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
static int mtk_read_temp(void *data, int *temperature)
{
- struct mtk_thermal *mt = data;
+ struct mtk_thermal_zone *tz = data;
+ struct mtk_thermal *mt = tz->mt;
int i;
int tempmax = INT_MIN;
@@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int *temperature)
return 0;
}
+static int mtk_read_sensor_temp(void *data, int *temperature)
+{
+ struct mtk_thermal_zone *tz = data;
+ struct mtk_thermal *mt = tz->mt;
+ int id = tz->id - 1;
+
+ if (id < 0)
+ return -EACCES;
+
+ *temperature = _get_sensor_temp(mt, id);
+
+ return 0;
+}
+
static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
.get_temp = mtk_read_temp,
};
+static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
+ .get_temp = mtk_read_sensor_temp,
+};
+
static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
u32 apmixed_phys_base, u32 auxadc_phys_base,
int ctrl_id)
@@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
u64 auxadc_phys_base, apmixed_phys_base;
struct thermal_zone_device *tzdev;
void __iomem *apmixed_base, *auxadc_base;
+ struct mtk_thermal_zone *tz;
mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
if (!mt)
@@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mt);
- tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
- &mtk_thermal_ops);
- if (IS_ERR(tzdev)) {
- ret = PTR_ERR(tzdev);
- goto err_disable_clk_peri_therm;
+ for (i = 0; i < mt->conf->num_sensors + 1; i++) {
+ tz = kmalloc(sizeof(*tz), GFP_KERNEL);
+ if (!tz)
+ return -ENOMEM;
+
+ tz->mt = mt;
+ tz->id = i;
+
+ tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i, tz, (i == 0) ?
+ &mtk_thermal_ops :
+ &mtk_thermal_sensor_ops);
+
+ if (IS_ERR(tzdev)) {
+ if (PTR_ERR(tzdev) == -ENODEV) {
+ dev_warn(&pdev->dev,
+ "sensor %d not registered in thermal zone in dt\n", i);
+ continue;
+ }
+ if (PTR_ERR(tzdev) == -EACCES) {
+ ret = PTR_ERR(tzdev);
+ goto err_disable_clk_peri_therm;
+ }
+ }
}
ret = devm_thermal_add_hwmon_sysfs(tzdev);
--
2.37.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2022-07-20 18:18 ` Amjad Ouled-Ameur
0 siblings, 0 replies; 7+ messages in thread
From: Amjad Ouled-Ameur @ 2022-07-20 18:18 UTC (permalink / raw)
To: matthias.bgg
Cc: rafael, fparent, amitk, daniel.lezcano, devicetree,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pm, robh+dt, rui.zhang, Amjad Ouled-Ameur,
default avatarMichael Kao, default avatarHsin-Yi Wang
Provide thermal zone to read thermal sensor in the SoC. We can read all the
thermal sensors value in the SoC by the node /sys/class/thermal/
In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
on the first read of sensor that often are bogus values.
This can avoid following warning on boot:
thermal thermal_zone6: failed to read out thermal zone (-13)
Signed-off-by: default avatarMichael Kao <michael.kao@mediatek.com>
Signed-off-by: default avatarHsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
Tested-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
---
drivers/thermal/mtk_thermal.c | 100 ++++++++++++++++++++++++++--------
1 file changed, 76 insertions(+), 24 deletions(-)
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 1dc276f8c4f1..79b14ce1a08d 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -259,6 +259,11 @@ enum mtk_thermal_version {
struct mtk_thermal;
+struct mtk_thermal_zone {
+ struct mtk_thermal *mt;
+ int id;
+};
+
struct thermal_bank_cfg {
unsigned int num_sensors;
const int *sensors;
@@ -709,6 +714,32 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
mutex_unlock(&mt->lock);
}
+static u32 _get_sensor_temp(struct mtk_thermal *mt, int id)
+{
+ u32 raw;
+ int temp;
+
+ const struct mtk_thermal_data *conf = mt->conf;
+
+ raw = readl(mt->thermal_base + conf->msr[id]);
+
+ if (mt->conf->version == MTK_THERMAL_V1)
+ temp = raw_to_mcelsius_v1(mt, id, raw);
+ else
+ temp = raw_to_mcelsius_v2(mt, id, raw);
+
+ /*
+ * The first read of a sensor often contains very high bogus
+ * temperature value. Filter these out so that the system does
+ * not immediately shut down.
+ */
+
+ if (temp > 200000)
+ return -EAGAIN;
+ else
+ return temp;
+}
+
/**
* mtk_thermal_bank_temperature - get the temperature of a bank
* @bank: The bank
@@ -721,26 +752,9 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
struct mtk_thermal *mt = bank->mt;
const struct mtk_thermal_data *conf = mt->conf;
int i, temp = INT_MIN, max = INT_MIN;
- u32 raw;
for (i = 0; i < conf->bank_data[bank->id].num_sensors; i++) {
- raw = readl(mt->thermal_base + conf->msr[i]);
-
- if (mt->conf->version == MTK_THERMAL_V1) {
- temp = raw_to_mcelsius_v1(
- mt, conf->bank_data[bank->id].sensors[i], raw);
- } else {
- temp = raw_to_mcelsius_v2(
- mt, conf->bank_data[bank->id].sensors[i], raw);
- }
-
- /*
- * The first read of a sensor often contains very high bogus
- * temperature value. Filter these out so that the system does
- * not immediately shut down.
- */
- if (temp > 200000)
- temp = 0;
+ temp = _get_sensor_temp(mt, i);
if (temp > max)
max = temp;
@@ -751,7 +765,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
static int mtk_read_temp(void *data, int *temperature)
{
- struct mtk_thermal *mt = data;
+ struct mtk_thermal_zone *tz = data;
+ struct mtk_thermal *mt = tz->mt;
int i;
int tempmax = INT_MIN;
@@ -770,10 +785,28 @@ static int mtk_read_temp(void *data, int *temperature)
return 0;
}
+static int mtk_read_sensor_temp(void *data, int *temperature)
+{
+ struct mtk_thermal_zone *tz = data;
+ struct mtk_thermal *mt = tz->mt;
+ int id = tz->id - 1;
+
+ if (id < 0)
+ return -EACCES;
+
+ *temperature = _get_sensor_temp(mt, id);
+
+ return 0;
+}
+
static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
.get_temp = mtk_read_temp,
};
+static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
+ .get_temp = mtk_read_sensor_temp,
+};
+
static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
u32 apmixed_phys_base, u32 auxadc_phys_base,
int ctrl_id)
@@ -1072,6 +1105,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
u64 auxadc_phys_base, apmixed_phys_base;
struct thermal_zone_device *tzdev;
void __iomem *apmixed_base, *auxadc_base;
+ struct mtk_thermal_zone *tz;
mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
if (!mt)
@@ -1161,11 +1195,29 @@ static int mtk_thermal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mt);
- tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
- &mtk_thermal_ops);
- if (IS_ERR(tzdev)) {
- ret = PTR_ERR(tzdev);
- goto err_disable_clk_peri_therm;
+ for (i = 0; i < mt->conf->num_sensors + 1; i++) {
+ tz = kmalloc(sizeof(*tz), GFP_KERNEL);
+ if (!tz)
+ return -ENOMEM;
+
+ tz->mt = mt;
+ tz->id = i;
+
+ tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i, tz, (i == 0) ?
+ &mtk_thermal_ops :
+ &mtk_thermal_sensor_ops);
+
+ if (IS_ERR(tzdev)) {
+ if (PTR_ERR(tzdev) == -ENODEV) {
+ dev_warn(&pdev->dev,
+ "sensor %d not registered in thermal zone in dt\n", i);
+ continue;
+ }
+ if (PTR_ERR(tzdev) == -EACCES) {
+ ret = PTR_ERR(tzdev);
+ goto err_disable_clk_peri_therm;
+ }
+ }
}
ret = devm_thermal_add_hwmon_sysfs(tzdev);
--
2.37.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-26 14:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 21:39 [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2022-07-20 18:18 [PATCH v2 0/4] thermal: mediatek: Add support for MT8365 SoC Amjad Ouled-Ameur
2022-07-20 18:18 ` [PATCH v2 4/4] thermal: mediatek: add another get_temp ops for thermal sensors Amjad Ouled-Ameur
2022-07-20 18:18 ` Amjad Ouled-Ameur
2022-07-20 18:54 ` Christophe JAILLET
2022-07-20 18:54 ` Christophe JAILLET
2022-08-26 14:12 ` Amjad Ouled-Ameur
2022-08-26 14:12 ` Amjad Ouled-Ameur
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.