Messages in this thread | | | Date | Fri, 16 Feb 2024 16:09:38 +0530 | Subject | Re: [PATCH v3 2/3] dt-bindings: iio: adc: Add support for QCOM PMIC5 Gen3 ADC | From | Jishnu Prakash <> |
| |
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
| |