linux-kernel - Re: [PATCH V10 4/4] thermal: qcom: add support for PMIC5 Gen3 ADC thermal monitoring

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <def530e3-3a5d-41cd-9f1a-1dc3f98747f9@oss.qualcomm.com>
Date: Fri, 6 Feb 2026 18:45:22 +0530
From: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
        agross@...nel.org, andersson@...nel.org, lumag@...nel.org,
        dmitry.baryshkov@....qualcomm.com, 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 V10 4/4] thermal: qcom: add support for PMIC5 Gen3 ADC
 thermal monitoring

Hi Jonathan,

On 1/31/2026 11:24 PM, Jonathan Cameron wrote:
> On Fri, 30 Jan 2026 17:24:21 +0530
> Jishnu Prakash <jishnu.prakash@....qualcomm.com> 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>
> 
> Hi Jishnu.
> 
> Some minor editorial style stuff below if you are spinning again.
> Otherwise this looks good to me
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
> Given I expect this patch will go through the thermal tree and not IIO.

Just to confirm, would it be fine to keep your Reviewed-by tag in place
if I only make changes in this patch to address your comments, in the
next patch series?

> As mentioned in previous patch review, we've missed this cycle for IIO where
> I'd expect to spin an immutable branch for 1-3 so we can do this early
> next cycle.
> 
> Thanks,
> 
> Jonathan
> 
> 
>> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c
>> new file mode 100644
>> index 000000000000..882355d6606d
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5-gen3.c
>> @@ -0,0 +1,512 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/container_of.h>
>> +#include <linux/device.h>
> 
> Similar comment to previous.  It's rare we need device.h
> and if a forwards definition of struct device is enough it
> is better to just do that.

I can do that in this file.

> 
>> +#include <linux/device/devres.h>
>> +#include <linux/dev_printk.h>
>> +#include <linux/err.h>
>> +#include <linux/iio/adc/qcom-adc5-gen3-common.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/thermal.h>
>> +#include <linux/types.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/unaligned.h>
>> +
>> +#include "../thermal_hwmon.h"
> 
>> +
>> +static void tm_handler_work(struct work_struct *work)
>> +{
>> +	struct adc_tm5_gen3_chip *adc_tm5 = container_of(work, struct adc_tm5_gen3_chip,
>> +							 tm_handler_work);
>> +	int sdam_index = -1;
>> +	u8 tm_status[2] = { };
>> +	u8 buf[16] = { };
>> +
>> +	for (int i = 0; i < adc_tm5->nchannels; i++) {
> 
> Not that important but you've been a bit inconsistent on this style
> of putting the loop iterator declaration in the for loop or not.
> You could definitely do it in a few more places.  I didn't comment on those
> because it's a style choice, but consistency is a good idea - hence I'm
> commenting here.

I'll update this everywhere in both driver files and also address your
other comments in the next patch series.

Thanks,
Jishnu

> 
>> +		struct adc_tm5_gen3_channel_props *chan_prop = &adc_tm5->chan_props[i];