Re: [PATCH V4 3/5] iio: adc: Add support for PMIC7 ADC

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

 



Hi Andy,

On 5/13/2020 3:18 PM, Andy Shevchenko wrote:
On Wed, May 13, 2020 at 12:23 PM Jishnu Prakash <jprakash@xxxxxxxxxxxxxx> wrote:
The ADC architecture on PMIC7 is changed as compared to PMIC5. The
major change from PMIC5 is that all SW communication to ADC goes through
PMK8350, which communicates with other PMICs through PBS when the ADC
on PMK8350 works in master mode. The SID register is used to identify the
PMICs with which the PBS needs to communicate. Add support for the same.
+#define ADC7_CONV_TIMEOUT                      msecs_to_jiffies(10)
...

+       ret = adc5_read(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
+       if (ret < 0)
Is ' < 0' part necessary?
Ditto for same cases in other places in the code.
I'll fix this at all required locations in this patch in the next post.

+               return ret;
...

+       switch (mask) {
+       case IIO_CHAN_INFO_PROCESSED:
+               ret = adc7_do_conversion(adc, prop, chan,
+                                       &adc_code_volt, &adc_code_cur);
+               if (ret)
+                       return ret;
+
+               ret = qcom_adc5_hw_scale(prop->scale_fn_type,
+                       &adc5_prescale_ratios[prop->prescale],
+                       adc->data,
+                       adc_code_volt, val);
+
+               if (ret)
+                       return ret;
+
+               return IIO_VAL_INT;
+       default:
+               return -EINVAL;
+       }
+
+       return 0;
Dead code?
Right, I'll remove it in the next post.

...

+static int qcom_vadc7_scale_hw_calib_die_temp(
+                               const struct vadc_prescale_ratio *prescale,
+                               const struct adc5_data *data,
+                               u16 adc_code, int *result_mdec)
+{
+
+       int voltage, vtemp0, temp, i = ARRAY_SIZE(adcmap7_die_temp) - 1;
How assignment to i is useful?


I'm using it in adcmap7_die_temp[i] below, to keep it within the character limit per line. I think it's more readable that way.


+       voltage = qcom_vadc_scale_code_voltage_factor(adc_code,
+                               prescale, data, 1);
+
+       if (adcmap7_die_temp[0].x > voltage) {
+               *result_mdec = DIE_TEMP_ADC7_SCALE_1;
+               return 0;
+       } else if (adcmap7_die_temp[i].x <= voltage) {
Redundant 'else'.
The expression is different, it's adcmap7_die_temp[i] here, not adcmap7_die_temp[0].


+               *result_mdec = DIE_TEMP_ADC7_MAX;
+               return 0;
+       }
+
+       for (i = 0; i < ARRAY_SIZE(adcmap7_die_temp); i++)
+               if (adcmap7_die_temp[i].x > voltage)
+                       break;
+
+       vtemp0 = adcmap7_die_temp[i - 1].x;
+       voltage = voltage - vtemp0;
+       temp = div64_s64(voltage * DIE_TEMP_ADC7_SCALE_FACTOR,
+               adcmap7_die_temp[i - 1].y);
+       temp += DIE_TEMP_ADC7_SCALE_1 + (DIE_TEMP_ADC7_SCALE_2 * (i - 1));
+       *result_mdec = temp;
+
+       return 0;
+}
...

+#define RATIO_MAX_ADC7         0x4000
Hmm... Why the last is in hex? Is it related to amount of bits in the
hardware? Then probably better to use BIT().
It is the upper limit reading for a ratiometric calibration measurement, which is reported as a 14 bit reading. I'll change this in the next post.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux