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 Dmitry,

On 11/29/2021 8:02 AM, Dmitry Baryshkov wrote:
On 23/11/2021 08:57, Jishnu Prakash 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>
---
  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

  static int adc_tm5_set_trips(void *data, int low, int high)
  {
      struct adc_tm5_channel *channel = data;
@@ -422,6 +739,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
          }
      }
  +    mutex_init(&chip->adc_mutex_lock);

Minor issue. This way, the mutex is left uninitialized for ADC_TM5_HC devices. I'd move the mutex_init() call to the _probe function itself.


The mutex is needed only for Gen2 ADC_TM devices, I have mentioned this in the adc_tm5_chip struct description. I'll keep it in the Gen2 ADC_TM init function.



+
+    if (chip->data->gen == ADC_TM5_GEN2)
+        return ret;
+

Please do not do this. Create a separate adc_tm5_gen2_init function.
Add init() callback to adc_tm5_data structure.


Will make this change in next post.



      buf[0] = chip->decimation;
      buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
      buf[2] = ADC_TM5_TIMER1;
@@ -442,7 +764,7 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
                         struct device_node *node)
  {
      const char *name = node->name;
-    u32 chan, value, varr[2];
+    u32 chan, value, adc_channel, varr[2];
      int ret;
      struct device *dev = adc_tm->dev;
      struct of_phandle_args args;
@@ -472,7 +794,11 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
      }
      of_node_put(args.np);
  -    if (args.args_count != 1 || args.args[0] >= ADC5_MAX_CHANNEL) {
+    adc_channel = args.args[0];
+    if (adc_tm->data->gen == ADC_TM5_GEN2)
+        adc_channel &= 0xff;
+
+    if (args.args_count != 1 || adc_channel >= ADC5_MAX_CHANNEL) {

Here you read the data (args.args[0]) before checking that it is actually available (args.args_count is not zero). Please correct the sequence.


Will correct this in next post.



          dev_err(dev, "%s: invalid ADC channel number %d\n", name, chan);
          return -EINVAL;
      }
@@ -518,6 +844,32 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
      else
          channel->cal_method = ADC_TM5_ABSOLUTE_CAL;
  +    if (adc_tm->data->gen == ADC_TM5_GEN2) {
+        ret = of_property_read_u32(node, "qcom,decimation", &value);
+        if (!ret) {
+            ret = qcom_adc5_decimation_from_dt(value, adc_tm->data->decimation);
+            if (ret < 0) {
+                dev_err(dev, "invalid decimation %d\n", value);
+                return ret;

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