On Fri, 22 Mar 2024 10:58:54 +0100 Andrej Picej <andrej.picej@xxxxxxxxx> wrote: > On 22. 03. 24 09:14, Krzysztof Kozlowski wrote: > > On 22/03/2024 08:39, Andrej Picej wrote: > >> On 20. 03. 24 13:15, Krzysztof Kozlowski wrote: > >>> On 20/03/2024 13:05, Andrej Picej wrote: > >>>> Hi Krzysztof, > >>>> > >>>> On 20. 03. 24 11:26, Krzysztof Kozlowski wrote: > >>>>> On 20/03/2024 11:04, Andrej Picej wrote: > >>>>>> Document calibration properties and how to set them. > >>>>> > >>>>> Bindings are before users. > >>>> > >>>> will change patch order when I send a v2. > >>>> > >>>>> > >>>>> Please use subject prefixes matching the subsystem. You can get them for > >>>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > >>>>> your patch is touching. > >>>>> There is no file extension in prefixes. > >>>> > >>>> So: dt-bindings: iio/adc: nxp,imx93-adc: Add calibration properties? > >>> > >>> Did you run the command I proposed? I don't see much of "/", but except > >>> that looks good. > >> > >> Ok noted. > >> > >>> > >>>> > >>>>> > >>>>>> > >>>>>> Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx> > >>>>>> --- > >>>>>> .../bindings/iio/adc/nxp,imx93-adc.yaml | 15 +++++++++++++++ > >>>>>> 1 file changed, 15 insertions(+) > >>>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml > >>>>>> index dacc526dc695..64958be62a6a 100644 > >>>>>> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml > >>>>>> @@ -46,6 +46,21 @@ properties: > >>>>>> "#io-channel-cells": > >>>>>> const: 1 > >>>>>> > >>>>>> + nxp,calib-avg-en: > >>>>>> + description: > >>>>>> + Enable or disable averaging of calibration time. > >>>>>> + enum: [ 0, 1 ] > >>>>>> + > >>>>>> + nxp,calib-nr-samples: > >>>>>> + description: > >>>>>> + Selects the number of averaging samples to be used during calibration. > >>>>>> + enum: [ 16, 32, 128, 512 ] > >>>>>> + > >>>>>> + nxp,calib-t-samples: > >>>>>> + description: > >>>>>> + Specifies the sample time of calibration conversions. > >>>>>> + enum: [ 8, 16, 22, 32 ] > >>>>> > >>>>> No, use existing, generic properties. Open other bindings for this. > >>>> > >>>> You mean I should use generic properties for the ADC calibration > >>>> settings? Is there already something in place? Because as I understand > >>>> it, these calib-* values only effect the calibration process of the ADC. > >>> > >>> Please take a look at other devices and dtschema. We already have some > >>> properties for this... but maybe they cannot be used? > >>> > >> > >> I did look into other ADC devices, grep across iio/adc, adc bindings > >> folders and couldn't find anything closely related to what we are > >> looking for. Could you please point me to the properties that you think > >> should be used for this? > > > > Indeed, there are few device specific like qcom,avg-samples. We have > > though oversampling-ratio, settling-time-us and min-sample-time (which > > is not that good because does not use unit suffix). > > Ok, these are examples but I think I should not use them, since these > are i.MX93 ADC specific settings, which are used for configuration of > calibration process, and are not related to the standard conversion > process during runtime. Calibration process is the first step that > should be done after every power-on reset. > > > > > Then follow up questions: > > - nxp,calib-avg-en: Why is it a board-level decision? I would assume > > this depends on user choice and what kind of input you have (which could > > be board dependent or could be runtime decision). > > Not really sure I get your question, so please elaborate if I missed the > point. > This is a user choice, to enable or disable the averaging function in > calibration, but this is a board-level decision, probably relates on > external ADC regulators and input connections. The same options are used > for every ADC channel and this can not be a runtime decision, since > calibration is done before the ADC is even registered. I'll raise this question in reply to the cover letter or patch 1 where it is perhaps more appropriate, but I'd really like to know more about why these are useful at all. > > > - nxp,calib-t-samples: what does it mean? Time is expressed in time > > units, but there is nothing about units in the property name. > > > > You are right, basically this is "time" in cycles of AD_CLK. I should at > least add that to the property description. > > Best regards, > Andrej Picej
