On Tue, 6 Nov 2018 17:41:15 +0100 Philippe Schenker <dev@xxxxxxxxxxxx> wrote: > From: Stefan Agner <stefan@xxxxxxxx> > > This adds the devicetree bindings for the STMPE ADC. > > Signed-off-by: Stefan Agner <stefan@xxxxxxxx> > Signed-off-by: Max Krummenacher <max.krummenacher@xxxxxxxxxxx> > Signed-off-by: Philippe Schenker <philippe.schenker@xxxxxxxxxxx> > --- > .../devicetree/bindings/iio/adc/stmpe-adc.txt | 34 +++++++++++++++++++ > 1 file changed, 34 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt > > diff --git a/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt > new file mode 100644 > index 000000000000..752ef35a794d > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt > @@ -0,0 +1,34 @@ > +STMPE ADC driver > +---------------- > + > +Required properties: > + - compatible: "st,stmpe-adc" > + > +Optional properties: > +Note that the ADC is shared with the STMPE touchscreen, so if using both the > +settings should be the same. Ah, guessing there is already a binding in place for that which we need to closely match. Should there be one overarching device with both as children though if it is the same hardware? > +If the settings are not the same, the settings of the driver initialized last > +will be active. > +- st,sample-time: ADC conversion time in number of clock. (0 -> 36 clocks, > + 1 -> 44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, > + 5 -> 96 clocks, 6 -> 144 clocks), recommended is 4. Could of questions here. Is this a hardware dictated limit or is it dependent on application? Basically I'm asking if it should be in userspace rather than here. Ideally I'd love to see it in a unit of time, but failing that can we have it in clock cycles please rather than an enum? Always nice if a binding is human readable. st,sample-clock-cycles would be a better name. > +- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC) > +- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external > + reference) If there is an external reference, I'd expect to see a regulator providing it. Usually the mere presence of such an optional regulator does the job of saying if it should be used. > +- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz) So this is some internal clock? What dictates the right setting? I.e. how should anyone know which one to pick? > +- st,norequest-mask: bitmask specifying which ADC channels should _not_ be > + requestable due to different usage (e.g. touch) That's a little bit ugly. Would prefer to see an explicit set of channels brought out as children in DT. There are some patches around standardising some properties for doing that under review at the moment. [PATCH v4 2/4] dt-bindings: iio: adc: Add common ADCs properties to a separate file > + > +Node name must be stmpe_adc and should be child node of stmpe node to > +which it belongs. > + > +Example: > + > + stmpe_adc { adc { is the moderately standard naming for ADCS. > + compatible = "st,stmpe-adc"; > + st,sample-time = <4>; > + st,mod-12b = <1>; > + st,ref-sel = <0>; > + st,adc-freq = <1>; > + st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */ > + };