Hi Jonathan, On Thu, Jul 15, 2021 at 2:02 PM Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote: > > On Wed, 14 Jul 2021 19:24:27 +0100 > "Lad, Prabhakar" <prabhakar.csengg@xxxxxxxxx> wrote: > > > Hi Jonathan, > > > > On Wed, Jul 14, 2021 at 1:39 PM Jonathan Cameron > > <Jonathan.Cameron@xxxxxxxxxx> wrote: > > > > > > On Wed, 14 Jul 2021 10:11:49 +0100 > > > "Lad, Prabhakar" <prabhakar.csengg@xxxxxxxxx> wrote: > > > > > > > Hi Jonathan, > > > > > > > > Thank you for the review. > > > > > > > > On Sat, Jul 3, 2021 at 6:17 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > > > > > > > On Tue, 29 Jun 2021 23:03:27 +0100 > > > > > Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote: > > > > > > > > > > > Add binding documentation for Renesas RZ/G2L A/D converter block. > > > > > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > > > > Hi, > > > > > > > > > > See inline > > > > > > > > > > Jonathan > > > > > > > > > > > --- > > > > > > .../bindings/iio/adc/renesas,rzg2l-adc.yaml | 121 ++++++++++++++++++ > > > > > > 1 file changed, 121 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..db935d6d59eb > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > > > > @@ -0,0 +1,121 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/iio/adc/renesas,rzg2l-adc.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Renesas RZ/G2L ADC > > > > > > + > > > > > > +maintainers: > > > > > > + - Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > > + > > > > > > +description: | > > > > > > + A/D Converter block is a successive approximation analog-to-digital converter > > > > > > + with a 12-bit accuracy. Up to eight analog input channels can be selected. > > > > > > + Conversions can be performed in single or repeat mode. Result of the ADC is > > > > > > + stored in a 32-bit data register corresponding to each channel. > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + oneOf: > > > > > > + - items: > > > > > > + - enum: > > > > > > + - renesas,r9a07g044-adc # RZ/G2{L,LC} > > > > > > + - const: renesas,rzg2l-adc > > > > > > + > > > > > > + reg: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + interrupts: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + clocks: > > > > > > + items: > > > > > > + - description: converter clock > > > > > > + - description: peripheral clock > > > > > > + > > > > > > + clock-names: > > > > > > + items: > > > > > > + - const: adclk > > > > > > + - const: pclk > > > > > > + > > > > > > + power-domains: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + resets: > > > > > > + maxItems: 2 > > > > > > + > > > > > > + reset-names: > > > > > > + items: > > > > > > + - const: presetn > > > > > > + - const: adrst-n > > > > > > + > > > > > > + renesas-rzg2l,adc-trigger-mode: > > > > > > + $ref: /schemas/types.yaml#/definitions/uint8 > > > > > > + description: Trigger mode for A/D converter > > > > > > + enum: > > > > > > + - 0 # Software trigger mode (Defaults) > > > > > > + - 1 # Asynchronous trigger using ADC_TRG trigger input pin > > > > > > + - 2 # Synchronous trigger (Trigger from MTU3a/GPT) > > > > > > > > > > Is this a function of the board in some fashion? If not it sounds like > > > > > something that should be in control of userspace. Normally we'd > > > > > do that by having the driver register some iio_triggers and depending > > > > > on which one is selected do the equivalent of what you have here. > > > > > > > > > Agreed for Asynchronous and Synchronous triggers. WRT Software trigger > > > > should this be registered as a iio_triggers too or read_raw() > > > > callback (with IIO_CHAN_INFO_RAW case) should be treated as Software > > > > trigger? > > > > > > > > > > Normally we'd use an external trigger to provide the software trigger > > > (plus as you say sysfs reads will map to this functionality). > > > > > > Something like the sysfs trigger or the hrtimer one would get used, though > > > also fine to use the dataready trigger from a different device (if you want > > > approximately synced dta. > > > > > We can live with syfs reads for now for SW triggers. Coming back to HW > > triggers I responded too quickly!. I am now trying to implement a gpio > > based HW trigger i.e. to kick adc conversion start but I couldn't find > > any drivers doing that. I looked at iio-trig-interrupt.c which > > registers irq based triggers, so something similar needs to be > > implemented in the adc driver? If that is the case the gpio has to be > > passed via to DT and use gpio_to_irq to register the handler. Or is it > > that I am missing something here ? > > Ok, I'm not really following the usecase for this. Is the thought that you'll > get lower latency / jitter triggering via a gpio rather than using a > bus write to the device (though on an integrated ADC I can't see why that would > be the case)? > Sorry for the confusion. ADC_TRIG I was referring to automatically triggers ADC conversion depending on the edges (whatever its is configured to). The external triggers can be handled by iio_trigger as you pointed out earlier! > If so, then what is actually setting the gpio? Something is ultimately > acting as the real trigger. A common model would be an hrtimer trigger > for example. If you then want to wire the driver up to capture on demand > using the gpio (to reduce latency) that's fine, but the gpio itself is > never a trigger in the sense of an IIO trigger (rather than a trigger > to the ADC itself). In that case, have the trigger handler set the > the gpio and wait for data capture to finish. Quite a few drivers > do this as some devices can only start sampling on an external pin being > set. E.g. adc/ad7606.c > > The iio-trig-interrupt is about using an external interrupt to trigger > a capture initialized by a register write or similar, it's not a direct > hardware capture signal. > thanks for the explanation, I realized it now. Cheers, Prabhakar
