| 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: <693d9370-ea2f-4eee-bd24-9803c2b15c39@oss.qualcomm.com>
Date: Fri, 19 Dec 2025 18:44:21 +0530
From: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: jic23@...nel.org, robh@...nel.org, krzysztof.kozlowski@...aro.org,
krzk+dt@...nel.org, conor+dt@...nel.org, agross@...nel.org,
andersson@...nel.org, lumag@...nel.org, konradybcio@...nel.org,
daniel.lezcano@...aro.org, sboyd@...nel.org, amitk@...nel.org,
thara.gopinath@...il.com, lee@...nel.org, rafael@...nel.org,
subbaraman.narayanamurthy@....qualcomm.com,
david.collins@....qualcomm.com, anjelique.melendez@....qualcomm.com,
kamal.wadhwa@....qualcomm.com, rui.zhang@...el.com,
lukasz.luba@....com, devicetree@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
cros-qcom-dts-watchers@...omium.org, quic_kotarake@...cinc.com,
neil.armstrong@...aro.org, stephan.gerhold@...aro.org
Subject: Re: [PATCH V8 4/4] thermal: qcom: add support for PMIC5 Gen3 ADC
thermal monitoring
Hi Dmitry,
On 12/6/2025 7:54 AM, Dmitry Baryshkov wrote:
> On Thu, Nov 27, 2025 at 07:10:36PM +0530, Jishnu Prakash wrote:
>> Add support for ADC_TM part of PMIC5 Gen3.
>>
>> This is an auxiliary driver under the Gen3 ADC driver, which implements the
>> threshold setting and interrupt generating functionalities of QCOM ADC_TM
>> drivers, used to support thermal trip points.
>>
>> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
>> ---
...
>>
>> drivers/thermal/qcom/Kconfig | 9 +
>> drivers/thermal/qcom/Makefile | 1 +
>> drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c | 530 ++++++++++++++++++
>> 3 files changed, 540 insertions(+)
>> create mode 100644 drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c
>>
>> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
>> index a6bb01082ec6..1acb11e4ac80 100644
>> --- a/drivers/thermal/qcom/Kconfig
>> +++ b/drivers/thermal/qcom/Kconfig
>> @@ -21,6 +21,15 @@ config QCOM_SPMI_ADC_TM5
>> Thermal client sets threshold temperature for both warm and cool and
>> gets updated when a threshold is reached.
>>
>> +config QCOM_SPMI_ADC_TM5_GEN3
>> + tristate "Qualcomm SPMI PMIC Thermal Monitor ADC5 Gen3"
>> + depends on QCOM_SPMI_ADC5_GEN3
>
> This module depends directly on the Gen3 ADC driver. I think you can
> drop a separate "common" submodule.
>
Yes, I can do this in the next patch series.
>> + help
>> + This enables the auxiliary thermal driver for the ADC5 Gen3 thermal
>> + monitoring device. It shows up as a thermal zone with multiple trip points.
>> + Thermal client sets threshold temperature for both warm and cool and
>> + gets updated when a threshold is reached.
>> +
>> config QCOM_SPMI_TEMP_ALARM
>> tristate "Qualcomm SPMI PMIC Temperature Alarm"
>> depends on OF && SPMI && IIO
>
>
>> +
>> +static struct adc_tm5_auxiliary_drv adctm5gen3_auxiliary_drv = {
>> + .adrv = {
>> + .id_table = adctm5_auxiliary_id_table,
>> + .probe = adc_tm5_probe,
>> + },
>> + .tm_event_notify = adctm_event_handler,
>> +};
>> +
>> +static int __init adctm5_init_module(void)
>> +{
>> + return auxiliary_driver_register(&adctm5gen3_auxiliary_drv.adrv);
>> +}
>> +
>> +static void __exit adctm5_exit_module(void)
>> +{
>> + auxiliary_driver_unregister(&adctm5gen3_auxiliary_drv.adrv);
>> +}
>> +
>> +module_init(adctm5_init_module);
>> +module_exit(adctm5_exit_module);
>
> We really need to make this work with module_auxiliary_driver-like
> macro.
>
I tried doing this again now, but I'm not sure if the way I found is fine.
Just to recap, the main issue with using module_auxiliary_driver() directly,
which I discussed with Jonathan here earlier: (https://lore.kernel.org/all/20250301032901.7b38fed4@jic23-huawei/)
was that it is a macro definition which uses its input argument to
generate function names. So if I have a line like this:
module_auxiliary_driver(adctm5gen3_auxiliary_drv.adrv);
it will generate function definitions like this, due to text substitutions:
static int __init adctm5gen3_auxiliary_drv.adrv_init(void)
which will fail compilation.
I see that in other drivers where module_auxiliary_driver() is used, there is a
"struct auxiliary_driver" initialization just before that initialized variable
is passed to module_auxiliary_driver(). I tried making a similar change here now:
-static struct adc_tm5_auxiliary_drv adctm5gen3_auxiliary_drv = {
- .adrv = {
- .id_table = adctm5_auxiliary_id_table,
- .probe = adc_tm5_probe,
- },
- .tm_event_notify = adctm_event_handler,
+static struct auxiliary_driver adctm5gen3_auxiliary_driver = {
+ .id_table = adctm5_auxiliary_id_table,
+ .probe = adc_tm5_probe,
};
-static int __init adctm5_init_module(void)
-{
- return auxiliary_driver_register(&adctm5gen3_auxiliary_drv.adrv);
-}
-
-static void __exit adctm5_exit_module(void)
-{
- auxiliary_driver_unregister(&adctm5gen3_auxiliary_drv.adrv);
-}
+struct adc_tm5_auxiliary_drv adctm5gen3_auxiliary_drv = {
+ .adrv = adctm5gen3_auxiliary_driver,
+ .tm_event_notify = adctm_event_handler,
+};
-module_init(adctm5_init_module);
-module_exit(adctm5_exit_module);
+module_auxiliary_driver(adctm5gen3_auxiliary_driver);
With this, I get the following error:
drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c:513:10: error: initializer element is not constant
513 | .adrv = adctm5gen3_auxiliary_driver,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
It looks like the above definiton of adctm5gen3_auxiliary_driver is not considered
a compile-time constant. I made the following modification to fix this:
-static struct auxiliary_driver adctm5gen3_auxiliary_driver = {
+static const struct auxiliary_driver adctm5gen3_auxiliary_driver = {
And with this, the code does get built, but with warnings like this:
drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c: In function ‘adctm5gen3_auxiliary_driver_init’:
./include/linux/device/driver.h:260:20: warning: passing argument 1 of ‘__auxiliary_driver_register’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
260 | return __register(&(__driver) , ##__VA_ARGS__); \
| ^~~~~~~~~~~
./include/linux/auxiliary_bus.h:253:30: note: in definition of macro ‘auxiliary_driver_register’
253 | __auxiliary_driver_register(auxdrv, THIS_MODULE, KBUILD_MODNAME)
| ^~~~~~
./include/linux/auxiliary_bus.h:287:2: note: in expansion of macro ‘module_driver’
287 | module_driver(__auxiliary_driver, auxiliary_driver_register, auxiliary_driver_unregister)
| ^~~~~~~~~~~~~
drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c:517:1: note: in expansion of macro ‘module_auxiliary_driver’
517 | module_auxiliary_driver(adctm5gen3_auxiliary_driver);
| ^~~~~~~~~~~~~~~~~~~~~~~
./include/linux/auxiliary_bus.h:250:58: note: expected ‘struct auxiliary_driver *’ but argument is of type ‘const struct auxiliary_driver *’
250 | int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *owner,
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
Is it acceptable to have the above code changes to use module_auxiliary_driver(),
even with the warnings generated?
Or do you have any other suggestions?
Thanks,
Jishnu
>> +
>> +MODULE_DESCRIPTION("SPMI PMIC Thermal Monitor ADC driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_IMPORT_NS("QCOM_SPMI_ADC5_GEN3");
>> --
>> 2.25.1
>>
>
Powered by blists - more mailing lists