From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 819623D5648 for ; Thu, 11 Jun 2026 10:43:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.180.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781174587; cv=none; b=qEzVNMRraCEN8kzIaA2IXkMJDi3NEy60B/+4pwdQ9NCsL8WuYNEIEDkEWLtBEso+cXiEA+YmBqCkBna/YQXeknNr2gmuVxdaiJU4ifDMnz+vQ0/HAZkgeNB+4+Tt8mb+ISyP+7cKf9cxYW2Ty3arVSwLGkFwT1ENmTZx8LSLe9I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781174587; c=relaxed/simple; bh=TUrkQVS+nK+woCaaeRYuiCWpYY0/NY/9Hz7hw3d9ZUE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=iqQ5tEFRGyv+OMhxlP3EIgeLi7/24stFElFvxyywWo72+xlIc5VD6FkjmclwFTcgZ1cPH8OFSC358kqfuZwi21KumNspBmp7rGHso95OkLuOfRdFLiJvfB93jwTCo8mDJKCKzxGpug7O7NUtlxKHpmS3q3c/CQqKpTH/2nAMdwc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=F0HAjLk8; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=Vd4Q7ky2; arc=none smtp.client-ip=205.220.180.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="F0HAjLk8"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="Vd4Q7ky2" Received: from pps.filterd (m0279868.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 65BA1S2w168107 for ; Thu, 11 Jun 2026 10:43:04 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= r8yW4Y/G286voQEgRLE73+RcFwqX62AXTHtgT7u6WBA=; b=F0HAjLk8oKvTGXUG C3cwWNqwhprLLzEb6+F/vOb3T5UdV5O8v0bNTjQhZrg1319JsFS7voEH/Y5ZAE0i hv+oT9Hej2fIioLcKtA6khYh4dme241OpROqQbzwNlPndJpk08X26tbqFxFdUvbV ZgxmEXeBqmKjS8/WfFyGRsBXFN4aCR64vYrOZmTLwKEu8K9F5EJ355zJmnA2Gouy lHBYmFFCXoAE1LVfLEwYQDB3sl82R58cVAKydlshx6oJEBcKX2uiQUOxN6AC4hmk iaj1gd6TD83y5eYzYOiNkoBBKD8xVhOsCbEMdXUYqXIJYs3N4x/4v/JluYvY1DTi y6SJ0Q== Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4eqe6ub041-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Thu, 11 Jun 2026 10:43:04 +0000 (GMT) Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-36d97a4e08fso7311548a91.0 for ; Thu, 11 Jun 2026 03:43:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1781174583; x=1781779383; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=r8yW4Y/G286voQEgRLE73+RcFwqX62AXTHtgT7u6WBA=; b=Vd4Q7ky2hUzVf821qjT6nWHaQTEmBR76f1yhqeNDKuK/NdtDZiJGmUukhoR+emOhpX w+BciiD5JhT0XMjkBB6n//YZWy2pdSrVfOykQulg4TgKU3VnvCNehtrcp8Q8X5x+/3UD R+FHp1zug4tFbztnvsu3mr3TN+/t5/F1Ye9pDT+nisb9xSeZRQolET9wvCK5MnU3h2Lg 1jlk3jT+3lkO2EDEATPAWMjCpJoDTo2Yo6nrC0jKF5Uu5a87jqXmiuJU7jLlH4CiC9Lz KdSQcr/3AqOibyAry5esOROmZQMiufPzN6QYswQnjwi18FBz9216PmMpMjXcrgXGvHoJ qgig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781174583; x=1781779383; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=r8yW4Y/G286voQEgRLE73+RcFwqX62AXTHtgT7u6WBA=; b=WMVDGaDClxXPfiycZwSeR8J+COV9ZxWTEju0D7S/OmZpOj9tTQQM2s2Co420F0nXsX zoTj11ZNaUPw6moCo1hDF7bpHJcb27Y1JGhgeWCZvUAJr3uTA6yqVTDuqksE70W9o1Ai qq1iiJKHYXRx1YxLWERvv55D5ahM4TvfB3VmHFgqbQTmaGsjmHzDjw7/nUxmXo3uXBLt 2J8TdAT+tdpgm0hJWLvJ2yqRuehm59u98tyz4bn6zuJGkzn23fbwoqbv5CDYUZIK50qU CBUoEeSoy8B3wRe4TaZjJ++sMKDPG0o7x8D5YePezwkzT2OcYA3QM11h86tmendYJLsz /34A== X-Forwarded-Encrypted: i=1; AFNElJ9bQrlvPMBdQcXLvghFTLJVEVv1fs8jvMPtpco9LNVdeSs5tbKVoRJFcxAXzjN/kyXBy0f8n6vHp2XwqpI=@vger.kernel.org X-Gm-Message-State: AOJu0YyvSGdQSicUGOV23BXj4129iXnGKINQK5lsitXzCSIqDk+qgEz6 +T+Z053r1G3eYR1LqJksvYlhuDplgzID3IxChAJ3yIFr63WjosBniuz2I+5DDv4gPwQFXh0KFPT QnFmI4o/qSR65LO7spBXkxP6IziFDZ89mH2TK39vr9v4nAx4c/+i3UdLgEt3v60yeSxE= X-Gm-Gg: Acq92OGnO3Fj0dchw3bpp2bJEHYJ5JcWrVyuqx+eKEisCSxgestmfKBXO+Dxm1w6F8W qlHDG/Iu9ubWeO4KwYiXVN53CP7r/zm8KmtORHQdRo6ZLaM/wJ1Sh2uSkAcjOcx85rDyyAj3mqA E5Fp4/8iVDGVrDsc5i0+E9imfo4RrKCvJZphCs5+2IPU3h0KAyYNv2XlOo4QBFh+XoqowzoCQPw kY2KkFQhVlLgbJS+FihSqIc3JdthSSlmgPzRJEJYswytzxVNe2bXjhJqai12GK+in8255Yx6TT1 TxVf2xo9EayWOOJf7I8D8pQam1cZdv5u6AkJGKkiYNCVFDACf1sfiLeE0qclNLIQBNAlpjo35+l GVUqkJ6A/9SdfzsopcrL5GeDt2fM2F+QQYXehgPKQwEIfroO7NVC01nR8kZntUvy3 X-Received: by 2002:a17:90b:3ec6:b0:36b:9798:4f6a with SMTP id 98e67ed59e1d1-3779f091c30mr2715981a91.10.1781174583018; Thu, 11 Jun 2026 03:43:03 -0700 (PDT) X-Received: by 2002:a17:90b:3ec6:b0:36b:9798:4f6a with SMTP id 98e67ed59e1d1-3779f091c30mr2715949a91.10.1781174582523; Thu, 11 Jun 2026 03:43:02 -0700 (PDT) Received: from [10.217.217.28] ([202.46.22.19]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2c164f84335sm273881735ad.19.2026.06.11.03.42.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Jun 2026 03:43:02 -0700 (PDT) Message-ID: <2c7b6222-d394-4edf-ad6d-d413ddd97f5f@oss.qualcomm.com> Date: Thu, 11 Jun 2026 16:12:55 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/2] thermal: qcom: add support for PMIC5 Gen3 ADC thermal monitoring To: Andy Shevchenko Cc: Jonathan Cameron , David Lechner , =?UTF-8?Q?Nuno_S=C3=A1?= , Andy Shevchenko , Amit Kucheria , Thara Gopinath , "Rafael J. Wysocki" , Daniel Lezcano , Zhang Rui , Lukasz Luba , linux-arm-msm@vger.kernel.org, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Kamal Wadhwa , David Collins , Anjelique Melendez , Neil Armstrong , Stephan Gerhold References: <20260526-gen3_adc_tm-v2-0-702fbac919ac@oss.qualcomm.com> <20260526-gen3_adc_tm-v2-2-702fbac919ac@oss.qualcomm.com> Content-Language: en-US From: Jishnu Prakash In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Proofpoint-Spam-Info: AW1haW4tMjYwNjExMDEwNyBTYWx0ZWRfX5kN3fDw8scqu VHHLD2foORSetMGX8ZOWpL3VvANkINJnXawDavuRayGVTMnbdfttNMf8O4tdNoIXT1UOA2kjxaG WVPkyzMjx7/Rj1zeWq8cS31iCD/tdBo= X-Authority-Analysis: v=2.4 cv=atOCzyZV c=1 sm=1 tr=0 ts=6a2a9138 cx=c_pps a=RP+M6JBNLl+fLTcSJhASfg==:117 a=fChuTYTh2wq5r3m49p7fHw==:17 a=IkcTkHD0fZMA:10 a=FelO9ux0wxsA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22 a=ZpdpYltYx_vBUK5n70dp:22 a=r5C68v33nSZEUmtJHtYA:9 a=QEXdDO2ut3YA:10 a=iS9zxrgQBfv6-_F4QbHw:22 X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwNjExMDEwNyBTYWx0ZWRfX8dWRTrphbamH gYBAlWFneuLn89XbozY2qny6jUD0WJc42APucB7X7U6qNTCsb2RYkxRhwuwCBAQ1kZcYDlGh0RT BNxtjHxbGWTYTl4GZXfeJ8Qm1yxEeMtvyMWwni0M276LYIGN5IXjK1Nt8IvuvLbFXTsgIiVSAQ1 uuFzQiGiJ6rc+Bixjp2o+0ny1h5al9d6X0Zl2mjWhixl8dXD0jdoQ+NW2qF/S+qXpZ3cuz8oKo6 yA6a0xiKxjEFzF3rkmXT4UWBapsLQU0kE8foPYJT+zPaf5+bIUvTgLnHfr8/NU0sCTdbMC022xc UT/PsdS/E9fEaANvrU6vdOfq6tHCqBXqeIqkdfggoEJwWatb0C3grhaCm2iYx6e/7nGjgGQksW4 4iNzEZHGiYDzfr2fNZxlrCZckdPxqPuKpdT9R7l4cDWWxxHNb5qcbEopISxX3oZ/28877PieMIn rEw2Y/JEAMBVYcLAXmA== X-Proofpoint-GUID: B0EqQ4ppummEM8STG43pPD3l78T5dD1Q X-Proofpoint-ORIG-GUID: B0EqQ4ppummEM8STG43pPD3l78T5dD1Q X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.125,FMLib:17.12.100.49 definitions=2026-06-11_02,2026-06-09_02,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 impostorscore=0 bulkscore=0 clxscore=1015 spamscore=0 malwarescore=0 phishscore=0 priorityscore=1501 lowpriorityscore=0 suspectscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2606040000 definitions=main-2606110107 Hi Andy, On 6/3/2026 5:14 AM, Andy Shevchenko wrote: > On Tue, May 26, 2026 at 04:26:10PM +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. > > ... > >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > >> +#include > > Why?! I think this is not needed with the other headers included now, I'll remove it. > >> +#include >> +#include >> +#include >> +#include > > ... > >> +/** >> + * struct adc_tm5_gen3_channel_props - ADC_TM channel structure >> + * @timer: time period of recurring TM measurement. >> + * @tm_chan_index: TM channel number used (ranging from 1-7). >> + * @sdam_index: SDAM on which this TM channel lies. >> + * @common_props: structure with common ADC channel properties. >> + * @high_thr_en: TM high threshold crossing detection enabled. >> + * @low_thr_en: TM low threshold crossing detection enabled. >> + * @chip: ADC TM device. >> + * @tzd: pointer to thermal device corresponding to TM channel. >> + */ >> +struct adc_tm5_gen3_channel_props { >> + unsigned int timer; >> + unsigned int tm_chan_index; >> + unsigned int sdam_index; >> + struct adc5_channel_common_prop common_props; >> + bool high_thr_en; >> + bool low_thr_en; >> + struct adc_tm5_gen3_chip *chip; >> + struct thermal_zone_device *tzd; > > Wouldn't `pahole` suggest better layout? > >> +}; > > ... > >> +struct adc_tm5_gen3_chip { >> + struct adc5_device_data *dev_data; >> + struct adc_tm5_gen3_channel_props *chan_props; >> + unsigned int nchannels; >> + struct device *dev; > > Ditto. I would expect the nchannels to be the last. Also play with the position > of dev to see if bloat-o-meter will show the difference. > Thanks for your suggestions, I used pahole to check and I'll update the above two structs to remove existing holes. And with bloat-o-meter, I tried moving dev around in adc_tm5_gen3_chip and found the below arrangement was the most optimal: struct adc_tm5_gen3_chip { struct adc5_device_data *dev_data; struct adc_tm5_gen3_channel_props *chan_props; struct device *dev; unsigned int nchannels; }; possibly because dev_data and chan_props are accessed the most frequently in this file. I'll make these changes in the next patch series. >> +}; > > ... > >> + return adc5_gen3_get_scaled_reading(adc_tm5->dev, &prop->common_props, >> + temp); > > Make it a single line. > > ... > >> + /* Low temperature corresponds to high voltage threshold */ >> + prop->high_thr_en = (low_temp != -INT_MAX); > > Can low_temp be INT_MIN at some point? >From what I see, that would not happen. It looks like the convention in the thermal framework is to use -INT_MAX as the limit for the lower threshold in the .set_trips callback. > >> + if (prop->high_thr_en) { >> + adc_code = qcom_adc_tm5_gen2_temp_res_scale(low_temp); >> + put_unaligned_le16(adc_code, &buf[10]); >> + } > > ... > >> +static int adc_tm5_probe(struct auxiliary_device *aux_dev, >> + const struct auxiliary_device_id *id) >> +{ >> + struct adc_tm5_gen3_chip *adc_tm5; >> + struct tm5_aux_dev_wrapper *aux_dev_wrapper; >> + struct device *dev = &aux_dev->dev; >> + int ret; >> + >> + adc_tm5 = devm_kzalloc(dev, sizeof(*adc_tm5), GFP_KERNEL); >> + if (!adc_tm5) >> + return -ENOMEM; >> + >> + aux_dev_wrapper = container_of(aux_dev, struct tm5_aux_dev_wrapper, >> + aux_dev); > > One line is easier to read. > >> + adc_tm5->dev = dev; >> + adc_tm5->dev_data = aux_dev_wrapper->dev_data; >> + adc_tm5->nchannels = aux_dev_wrapper->n_tm_channels; >> + adc_tm5->chan_props = devm_kcalloc(dev, aux_dev_wrapper->n_tm_channels, >> + sizeof(*adc_tm5->chan_props), GFP_KERNEL); >> + if (!adc_tm5->chan_props) >> + return -ENOMEM; >> + >> + for (int i = 0; i < adc_tm5->nchannels; i++) { >> + adc_tm5->chan_props[i].common_props = aux_dev_wrapper->tm_props[i]; >> + adc_tm5->chan_props[i].timer = MEAS_INT_1S; >> + adc_tm5->chan_props[i].sdam_index = (i + 1) / 8; >> + adc_tm5->chan_props[i].tm_chan_index = (i + 1) % 8; >> + adc_tm5->chan_props[i].chip = adc_tm5; >> + } >> + >> + /* This is to disable all ADC_TM channels in case of probe failure. */ >> + ret = devm_add_action(dev, adc5_gen3_disable, adc_tm5); >> + if (ret) >> + return ret; >> + >> + /* >> + * First SDAM's interrupt is shared between main ADC driver >> + * and auxiliary TM driver, so its flags must include >> + * IRQF_SHARED. This is not needed for other SDAMs as they >> + * will be used only for TM functionality. >> + */ > >> + ret = devm_request_threaded_irq(dev, >> + adc_tm5->dev_data->base[0].irq, >> + adctm5_gen3_isr, adctm5_gen3_isr_thread, >> + IRQF_ONESHOT | IRQF_SHARED, >> + adc_tm5->dev_data->base[0].irq_name, >> + adc_tm5); >> + if (ret < 0) >> + return ret; >> + >> + for (int i = 1; i < adc_tm5->dev_data->num_sdams; i++) { >> + ret = devm_request_threaded_irq(dev, >> + adc_tm5->dev_data->base[i].irq, >> + adctm5_gen3_isr, adctm5_gen3_isr_thread, >> + IRQF_ONESHOT, adc_tm5->dev_data->base[i].irq_name, >> + adc_tm5); >> + if (ret < 0) >> + return ret; >> + } > > Can't it be combined by using temporary irq_flags variable > > /* ...the fat comment... */ > irq_flags = ... > for (int i = 0; ...) { > ... > irq_flags = ... > } > > ? Thanks for your suggestion, I'll update it this way. I'll also address all your other comments in the next patch series. Thanks, Jishnu > >> + return adc_tm5_register_tzd(adc_tm5); >> +} > > ... > >> +static const struct auxiliary_device_id adctm5_auxiliary_id_table[] = { >> + { .name = "qcom_spmi_adc5_gen3.adc5_tm_gen3", }, > > Inner comma is redundant. > >> + { } >> +}; > >> + > > Unneeded blank line. > >> +MODULE_DEVICE_TABLE(auxiliary, adctm5_auxiliary_id_table); >> + >> +static struct auxiliary_driver adctm5gen3_auxiliary_driver = { >> + .id_table = adctm5_auxiliary_id_table, >> + .probe = adc_tm5_probe, >> +}; > >> + > > Ditto. > >> +module_auxiliary_driver(adctm5gen3_auxiliary_driver); >