On Tue, 24 Mar 2020 21:14:10 +0530
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.
>
> In addition, add definitions for ADC channels and virtual channel
> definitions per PMIC, to be used by ADC clients for PMIC7.
>
> Signed-off-by: Jishnu Prakash <jprakash@xxxxxxxxxxxxxx>
A few additions from me.
> ---
...
>
> +static int adc5_exit(struct platform_device *pdev)
> +{
> + struct adc5_chip *adc = platform_get_drvdata(pdev);
> +
> + mutex_destroy(&adc->lock);
Andy raised potential races. You definitely have some as this
will destroy the mutex before you've removed the userspace interfaces.
> + return 0;
> +}
> +
> static struct platform_driver adc5_driver = {
> .driver = {
> .name = "qcom-spmi-adc5.c",
> .of_match_table = adc5_match_table,
> },
> .probe = adc5_probe,
> + .remove = adc5_exit,
> };
> module_platform_driver(adc5_driver);
>
...
>
> +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 = 0;
> +
> + voltage = qcom_vadc_scale_code_voltage_factor(adc_code,
> + prescale, data, 1);
> +
> + while (i < ARRAY_SIZE(adcmap7_die_temp)) {
> + if (adcmap7_die_temp[i].x > voltage)
> + break;
> + i++;
> + }
For loop (I think Andy also raised this one).
> +
> + if (i == 0) {
> + *result_mdec = DIE_TEMP_ADC7_SCALE_1;
> + } else if (i == ARRAY_SIZE(adcmap7_die_temp)) {
> + *result_mdec = DIE_TEMP_ADC7_MAX;
> + } else {
> + vtemp0 = adcmap7_die_temp[i-1].x;
Spaces around the -
Same elsewhere.
> + 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;
> +}
> +
> static int qcom_vadc_scale_hw_smb_temp(
> const struct vadc_prescale_ratio *prescale,
> const struct adc5_data *data,
...
