| 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: <aed6c4ab-59e2-ebcb-be91-4b7e909419d9@linaro.org>
Date: Sat, 22 Feb 2020 08:39:26 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Anson Huang <anson.huang@....com>
Cc: "robh+dt@...nel.org" <robh+dt@...nel.org>,
"mark.rutland@....com" <mark.rutland@....com>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>,
"rui.zhang@...el.com" <rui.zhang@...el.com>,
"amit.kucheria@...durent.com" <amit.kucheria@...durent.com>,
Aisheng Dong <aisheng.dong@....com>,
"linux@...ck-us.net" <linux@...ck-us.net>,
"srinivas.kandagatla@...aro.org" <srinivas.kandagatla@...aro.org>,
"krzk@...nel.org" <krzk@...nel.org>,
Andy Duan <fugang.duan@....com>, Peng Fan <peng.fan@....com>,
Daniel Baluta <daniel.baluta@....com>,
"bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
"olof@...om.net" <olof@...om.net>,
"dinguyen@...nel.org" <dinguyen@...nel.org>,
Leonard Crestez <leonard.crestez@....com>,
"marcin.juszkiewicz@...aro.org" <marcin.juszkiewicz@...aro.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH V15 RESEND 3/5] thermal: imx_sc: add i.MX system
controller thermal support
On 22/02/2020 00:46, Anson Huang wrote:
> Hi, Daniel
>
>> Subject: Re: [PATCH V15 RESEND 3/5] thermal: imx_sc: add i.MX system
>> controller thermal support
>>
>> Hi Anson,
>>
>> sorry for the delay with this review, hopefully the upstreaming will be now a
>> bit more smooth.
>
> Thanks very much for review😊
>
>>
>> Apart the comments below, the driver looks good to me.
>>
>> On Thu, Feb 20, 2020 at 09:10:26AM +0800, Anson Huang wrote:
>>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
>>> inside, the system controller is in charge of controlling power, clock
>>> and thermal sensors etc..
>>>
>>> This patch adds i.MX system controller thermal driver support, Linux
>>> kernel has to communicate with system controller via MU (message unit)
>>> IPC to get each thermal sensor's temperature, it supports multiple
>>> sensors which are passed from device tree, please see the binding doc
>>> for details.
>>>
>>> Signed-off-by: Anson Huang <Anson.Huang@....com>
>>> ---
>>> No change.
>>> ---
>>> drivers/thermal/Kconfig | 11 +++
>>> drivers/thermal/Makefile | 1 +
>>> drivers/thermal/imx_sc_thermal.c | 142
>>> +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 154 insertions(+)
>>> create mode 100644 drivers/thermal/imx_sc_thermal.c
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
>>> 5a05db5..d1cb8dc 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -251,6 +251,17 @@ config IMX_THERMAL
>>> cpufreq is used as the cooling device to throttle CPUs when the
>>> passive trip is crossed.
>>>
>>> +config IMX_SC_THERMAL
>>> + tristate "Temperature sensor driver for NXP i.MX SoCs with System
>> Controller"
>>> + depends on ARCH_MXC && IMX_SCU
>>
>> IMX_SCU depends on IMX_MBOX which depends on ARCH_MXC. This
>> dependency could be simplified.
>>
>> Also add the COMPILE_TEST option to improve compilation test coverage.
>
> Will make it depends on IMX_SCU and COMPILE_TEST
>
>>
>>> + depends on OF
>>> + help
>>> + Support for Temperature Monitor (TEMPMON) found on NXP i.MX
>> SoCs with
>>> + system controller inside, Linux kernel has to communicate with
>> system
>>> + controller via MU (message unit) IPC to get temperature from
>> thermal
>>> + sensor. It supports one critical trip point and one
>>> + passive trip point for each thermal sensor.
>>> +
>>> config MAX77620_THERMAL
>>> tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
>>> depends on MFD_MAX77620
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index
>>> 9fb88e2..a11a6d8 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -43,6 +43,7 @@ obj-$(CONFIG_DB8500_THERMAL) +=
>> db8500_thermal.o
>>> obj-$(CONFIG_ARMADA_THERMAL) += armada_thermal.o
>>> obj-$(CONFIG_TANGO_THERMAL) += tango_thermal.o
>>> obj-$(CONFIG_IMX_THERMAL) += imx_thermal.o
>>> +obj-$(CONFIG_IMX_SC_THERMAL) += imx_sc_thermal.o
>>> obj-$(CONFIG_MAX77620_THERMAL) += max77620_thermal.o
>>> obj-$(CONFIG_QORIQ_THERMAL) += qoriq_thermal.o
>>> obj-$(CONFIG_DA9062_THERMAL) += da9062-thermal.o
>>> diff --git a/drivers/thermal/imx_sc_thermal.c
>>> b/drivers/thermal/imx_sc_thermal.c
>>> new file mode 100644
>>> index 0000000..d406ecb
>>> --- /dev/null
>>> +++ b/drivers/thermal/imx_sc_thermal.c
>>> @@ -0,0 +1,142 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright 2018-2019 NXP.
>>
>> *sigh* 2020 now ...
>
> Yes, should be 2018-2020
>
>>
>> [ ... ]
>>
>>> +static int imx_sc_thermal_get_temp(void *data, int *temp) {
>>> + struct imx_sc_msg_misc_get_temp msg;
>>> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
>>> + struct imx_sc_sensor *sensor = data;
>>> + int ret;
>>> +
>>> + msg.data.req.resource_id = sensor->resource_id;
>>> + msg.data.req.type = IMX_SC_C_TEMP;
>>> +
>>> + hdr->ver = IMX_SC_RPC_VERSION;
>>> + hdr->svc = IMX_SC_RPC_SVC_MISC;
>>> + hdr->func = IMX_SC_MISC_FUNC_GET_TEMP;
>>> + hdr->size = 2;
>>
>> Can you explain this 'size' value?
>
> The size means the SCU message size, including the header and the data, its unit
> is word(4 bytes), in thermal get temperature message, the header takes 1 word and
> the data takes another 1, so it is 2, we all pass the size in this way to SCU in i.MX8
> SoCs, the SCU know how long message it will need to receive from AP.
Thanks for the clarification.
>> [ ... ]
>>
>>> +MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
>>> +
>>> +static struct platform_driver imx_sc_thermal_driver = {
>>> + .probe = imx_sc_thermal_probe,
>>
>> The driver can be compiled as module but there is no 'remove' callback
>
> As there is nothing needs to be done in .remove callback, so I skip it. But
> I think I can add a blank .remove callback to make it more complete.
Ah, right. Everything is handled by "devm_"
>>> + .driver = {
>>> + .name = "imx-sc-thermal",
>>> + .of_match_table = imx_sc_thermal_table,
>>> + },
>>> +};
>>> +module_platform_driver(imx_sc_thermal_driver);
>>> +
>>> +MODULE_AUTHOR("Anson Huang <Anson.Huang@....com>");
>>> +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system
>>> +controller"); MODULE_LICENSE("GPL v2");
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists