Received-SPF: pass (google.com: domain of linux-kernel+bounces-68460-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <12723477-aee2-40bc-80f0-a86c16c98988@quicinc.com>
Date: Fri, 16 Feb 2024 16:09:38 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 2/3] dt-bindings: iio: adc: Add support for QCOM PMIC5
 Gen3 ADC
Content-Language: en-US
To: Jonathan Cameron <jic23@kernel.org>
CC: <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <conor+dt@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <lee@kernel.org>,
        <andriy.shevchenko@linux.intel.com>, <daniel.lezcano@linaro.org>,
        <dmitry.baryshkov@linaro.org>, <lars@metafoo.de>, <luca@z3ntu.xyz>,
        <marijn.suijten@somainline.org>, <agross@kernel.org>,
        <sboyd@kernel.org>, <rafael@kernel.org>, <rui.zhang@intel.com>,
        <lukasz.luba@arm.com>, <linus.walleij@linaro.org>,
        <quic_subbaram@quicinc.com>, <quic_collinsd@quicinc.com>,
        <quic_amelende@quicinc.com>, <quic_kamalw@quicinc.com>,
        <kernel@quicinc.com>, <linux-kernel@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-arm-msm-owner@vger.kernel.org>,
        <linux-iio@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <cros-qcom-dts-watchers@chromium.org>
References: <20231231171237.3322376-1-quic_jprakash@quicinc.com>
 <20231231171237.3322376-3-quic_jprakash@quicinc.com>
 <20240101180209.56e04267@jic23-huawei>
From: Jishnu Prakash <quic_jprakash@quicinc.com>
In-Reply-To: <20240101180209.56e04267@jic23-huawei>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Jonathan,

On 1/1/2024 11:32 PM, Jonathan Cameron wrote:
> On Sun, 31 Dec 2023 22:42:36 +0530
> Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>
>> For the PMIC5-Gen3 type PMICs, ADC peripheral is present in HW for the

>> +
>> +      label:
>> +        $ref: /schemas/types.yaml#/definitions/string
>> +        description: |
>> +            ADC input of the platform as seen in the schematics.
>> +            For thermistor inputs connected to generic AMUX or GPIO inputs
>> +            these can vary across platform for the same pins. Hence select
>> +            the platform schematics name for this channel.
> defined in adc.yaml, so should just have a reference to that here.
>
>> +
>> +      qcom,decimation:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: |
>> +            This parameter is used to decrease ADC sampling rate.
>> +            Quicker measurements can be made by reducing decimation ratio.
> Why is this in DT rather than as a userspace control?


We don't intend this property to be something that can be controlled 
from userspace - if a client wants to read an ADC channel from 
userspace, we only intend to provide them the processed value, 
calculated with a fixed set of ADC properties mentioned in the 
corresponding channel node in DT.


>> +        enum: [ 85, 340, 1360 ]
>> +        default: 1360
>> +

>> +
>> +      qcom,hw-settle-time:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: |
>> +            Time between AMUX getting configured and the ADC starting
>> +            conversion. The 'hw_settle_time' is an index used from valid values
>> +            and programmed in hardware to achieve the hardware settling delay.
>> +        enum: [ 15, 100, 200, 300, 400, 500, 600, 700, 1000, 2000, 4000,
>> +                8000, 16000, 32000, 64000, 128000 ]
>> +        default: 15
> only currently defined for muxes but we have settle-time-us which has benefit of
> providing the units (which are missing here from the description as well)
>
>> +
>> +      qcom,avg-samples:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: |
>> +            Number of samples to be used for measurement.
>> +            Averaging provides the option to obtain a single measurement
>> +            from the ADC that is an average of multiple samples. The value
>> +            selected is 2^(value).
> Why is this in dt?  Why not just userspace control (in_voltageX_oversampling_ratio
>
> If it needs to be, we do have standard DT bindings for it in adc.yaml


avg-samples is also something we don't want the client to modify from 
userspace. As for using adc.yaml, I think I could use settling-time-us 
and oversampling-ratio from it for the above two properties.

However, Krzysztof has mentioned in another comment that I should put 
properties common to ADC5 Gen3 and older QCOM VADC devices in a common 
schema. If I now try replacing the existing qcom,hw-settle-time and 
qcom,avg-samples properties with settling-time-us and oversampling-ratio 
for older devices too, I would have to make several DT changes for 
existing devices...are you fine with this? Or should I just replace 
these two properties for ADC5 Gen3?


I'll address your other comments in the next patchset.


Thanks,

Jishnu