On Tue, 7 Nov 2023 08:55:48 +0000 <Marius.Cristea@xxxxxxxxxxxxx> wrote: > Hi > > On Fri, 2023-10-27 at 15:26 +0100, Jonathan Cameron wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > know the content is safe > > > > On Thu, 26 Oct 2023 17:08:07 +0100 > > Conor Dooley <conor@xxxxxxxxxx> wrote: > > > > > On Thu, Oct 26, 2023 at 03:23:46PM +0000, > > > Marius.Cristea@xxxxxxxxxxxxx wrote: > > > > Hi Conor, > > > > > > > > On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote: > > > > > Hey Marius, > > > > > > > > > > On Wed, Oct 25, 2023 at 04:44:03PM +0300, > > > > > marius.cristea@xxxxxxxxxxxxx wrote: > > > > > > From: Marius Cristea <marius.cristea@xxxxxxxxxxxxx> > > > > > > > > > > > > This is the device tree schema for iio driver for > > > > > > Microchip PAC193X series of Power Monitors with Accumulator. > > > > > > > > > > > > > ...... > > > > > > + > > > > > > + reg: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + "#address-cells": > > > > > > + const: 1 > > > > > > + > > > > > > + "#size-cells": > > > > > > + const: 0 > > > > > > + > > > > > > + interrupts: > > > > > > + description: IRQ line of the ADC > > > > > > + maxItems: 1 > > > > > > + > > > > > > + drive-open-drain: > > > > > > + description: The IRQ signal is configured as open-drain. > > > > > > + type: boolean > > > > > > + maxItems: 1 > > > > > > + > > > > > > + microchip,slow-io: > > > > > > + type: boolean > > > > > > + description: | > > > > > > + A GPIO used to trigger a change is sampling rate > > > > > > (lowering > > > > > > the chip power consumption). > > > > > > + In default mode, if this pin is forced high, sampling > > > > > > rate > > > > > > is forced to eight > > > > > > + samples/second. When it is forced low, the sampling > > > > > > rate is > > > > > > 1024 samples/second unless > > > > > > + a different sample rate has been programmed. > > > > > > > > > > This description doesn't really make sense to me - if a GPIO is > > > > > used > > > > > to > > > > > drive the pin low or high, why do we need a property? A DT > > > > > property > > > > > implies that this is a static configuration depending on the > > > > > board, > > > > > but > > > > > reading the description this seems to be something that can be > > > > > toggled > > > > > at runtime. > > > > > I do note though, that this GPIO is not documented in the > > > > > binding, so > > > > > I > > > > > suppose what really needs to happen here is document the gpio > > > > > so that > > > > > the driver can determine at runtime what state this pin is in? > > > > > > > > > > Also, you say "In default mode", but don't mention what the > > > > > non- > > > > > default > > > > > mode is. What happens in the other mode? > > > > > > > This is a "double function" pin. On the PAC193x there is the > > > > SLOW/ALERT > > > > pin. At runtime this pin could be configured as an input to the > > > > PAC and > > > > the functionality will be "SLOW" that means if it is forced high, > > > > the > > > > PAC will work in low power mode by changing the sample rate to 8 > > > > SPS. > > > > If it's forced low the PAC will work at it's full sample rate. > > > > > > Since this is a runtime thing, it doesn't make sense to have a > > > property > > > that is set at dts creation time that decides what mode the pin is > > > in. > > > > > > > "SLOW" is the default function of the pin but it may be > > > > programmed to > > > > function as ALERT pin (Open Collector when functioning as ALERT, > > > > requires pull-up resistor to VDD I/O). This time the pin will be > > > > set as > > > > output from PAC (ALERT functionality) to trigger an interrupt to > > > > the > > > > system (this is covered by the interrupts and drive-open-drain). > > > > > > Hmm, at the risk of getting out of my depth with what the GPIO > > > subsystem > > > is capable of doing, I would expect to see something like > > > > > > sampling-rate-gpios: > > > description: > > > <what you have above> > > > maxItems: 1 > > > > > > Which would allow the driver to either drive this pin via the gpio > > > subsystem, or to use the interrupt property to use it as an > > > interrupt > > > instead. > > > > > > Perhaps Jonathan etc knows better for these sort of dual mode pins. > > > > Beyond them being a pain? The fun is they may get wired to interrupt > > controllers that are also GPIOs or they may not (and the other way > > around > > with them wired to GPIO pins that aren't interrupt pins). > > > > I don't understand the usecase for the SLOW control. > > Given it seems software can override the use for SLOW I'd be tempted > > to > > always do that. > > Thus making this pin useable only as an optional interrupt. > > > > If someone hard wires it to high or low that is harmless if we aren't > > letting it control anything. > > > > Here I was trying to define/describe 3 possible situations: > - 1) the pin is not used at all, so it doesn't matter if it's connected > somewhere > > - 2) the pin is user configured as "interrupt" and it's connected to > the interrupt controller (this case is not supported in the driver > right now) > > - 3) the pin is user configured as "SLOW" (this case is not supported > in the driver right now). That means it should be connected to a GPIO > pin. This function (SLOW control) will automatically change the PAC > internal sampling frequency to lower the PAC internal power > consumption. For example, the PAC could be configured to a sample rate > of 1024 samples/s (it will consume maximum current). Using the SLOW > control, the chip will internally change to 8 samples/s but the math > internally will "behave" as the 1024 samples/s but at a much lower > power consumption. It's very useful in case the system wants to lower > power consumption (we still need to measure battery power consumption > even if the system is put into a low power state). PAC internal power > consumption is proportional to the number of channels used and also the > sampling frequency. So far so good, but how does 3 differ from just setting the chip to sample at 8 samples/s, which I believe we can do from software? Anyhow, for a DT binding, provide both gpio and interrupt as optional and the driver can make up it's mind on what to do if both are provided. Jonathan > > > > > > > > > > The system could work fine without this pin. The driver doesn't > > > > use > > > > interrupt at this time, but it could be extended. > > > > > > Cheers, > > > Conor. > > > > Thanks, > Marius
