Re: [PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

On 11/27/2021 12:16 AM, Jonathan Cameron wrote:
On Tue, 23 Nov 2021 11:27:04 +0530
Jishnu Prakash <quic_jprakash@xxxxxxxxxxx> wrote:

Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
close counterpart of PMIC7 ADC and has the same functionality as
PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
It is present on PMK8350 alone, like PMIC7 ADC and can be used
to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
having ADC on a target, through PBS(Programmable Boot Sequence).

Signed-off-by: Jishnu Prakash <quic_jprakash@xxxxxxxxxxx>
Just one note on using put_unaligned_le16() below.  Otherwise, from
a drive by review point of view it looks fine to someone not that
familiar with the driver or thermal :)

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

---
  drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 ++++++++++++++++++++++++++++++-
  1 file changed, 372 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index fc8cd45..a7b33a8 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -4,7 +4,10 @@
   *
   * Based on original driver:
   * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
+ *
+ * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved.
   */

+
+	/* Low temperature corresponds to high voltage threshold */
+	if (low != -INT_MAX) {
+		channel->high_thr_en = true;
+		adc_code = qcom_adc_tm5_gen2_temp_res_scale(low);
+
+		buf[11] = adc_code & 0xff;
+		buf[12] = adc_code >> 8;
looks like a little endian put though not necessarily aligned so
put_unaligned_le16() preferred to open coding it. Same in similar places.
Not my area though so maintainer may not care as much.


I'll use put_unaligned_le16 as suggested, in similar places in the next post.


+	} else {
+		channel->high_thr_en = false;
+	}
+
+	buf[13] = ADC_TM_GEN2_MEAS_EN;
+	if (channel->high_thr_en)
+		buf[13] |= ADC_TM5_GEN2_HIGH_THR_INT_EN;

Thanks,

Jishnu




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux