On Fri, 19 Jul 2024 20:38:54 +0300 <marius.cristea@xxxxxxxxxxxxx> wrote: > From: Marius Cristea <marius.cristea@xxxxxxxxxxxxx> > > This is the device tree schema for iio driver for > Microchip PAC194X and PAC195X series of Power Monitors with Accumulator. Hi Marius Good to add a tiny bit either here or in below description for why there are so many part numbers. Looks like mixture of number of channels and high vs low side monitors. And the big number is about voltage range? Other comments inline. Thanks, Jonathan > > Signed-off-by: Marius Cristea <marius.cristea@xxxxxxxxxxxxx> > --- > .../bindings/iio/adc/microchip,pac1944.yaml | 168 ++++++++++++++++++ > 1 file changed, 168 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml > new file mode 100644 > index 000000000000..e997a4d536e9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml > @@ -0,0 +1,168 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1944.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip PAC1944 and PAC1954 Power Monitors with Accumulator > + > +maintainers: > + - Marius Cristea <marius.cristea@xxxxxxxxxxxxx> > + > +description: | > + This device is part of the Microchip family of Power Monitors with Accumulator. > + The datasheet for PAC1941_1, PAC1941_1, PAC1942_1, PAC1942_2, PAC1943_1 and PAC1944_1 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC194X-Family-Data-Sheet-DS20006543.pdf > + The datasheet for PAC1951_1, PAC1951_1, PAC1952_1, PAC1952_2, PAC1953_1 and PAC1954_1 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC195X-Family-Data-Sheet-DS20006539.pdf > + > +properties: > + compatible: > + enum: > + - microchip,pac1941_1 > + - microchip,pac1941_2 > + - microchip,pac1942_1 > + - microchip,pac1942_2 > + - microchip,pac1943_1 > + - microchip,pac1944_1 > + - microchip,pac1951_1 > + - microchip,pac1951_2 > + - microchip,pac1952_1 > + - microchip,pac1952_2 > + - microchip,pac1953_1 > + - microchip,pac1954_1 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + interrupts: > + maxItems: 2 Need names as annoyingly people often wire just the second one (I've never understood why!) > + > + slow-io-gpios: > + description: > + A GPIO used to trigger a change is sampling rate (lowering the chip power > + consumption). If configured in SLOW 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. > + > + microchip,gpio: > + type: boolean > + description: > + In default mode, this pin is a GPIO input pin. It can be > + configured to be an output pin, as well as the ALERT2 > + function. This pin is an open-drain configuration and > + needs a pull-up resistor to VDD. This one is a bit odd. Can we do that configuration based on whether anyone requests it or if it is wired as alert2? So I don't think we should need this binding, but we may need the stuff to provide a gpio. Also, oddly short wrap. > + > +patternProperties: > + "^channel@[1-4]+$": > + type: object > + $ref: adc.yaml > + description: > + Represents the external channels which are connected to the ADC. > + > + properties: > + reg: > + items: > + minimum: 1 > + maximum: 4 > + > + shunt-resistor-micro-ohms: > + description: > + Value in micro Ohms of the shunt resistor connected between > + the SENSE+ and SENSE- inputs, across which the current is measured. > + Value is needed to compute the scaling of the measured current. > + > + microchip,vbus-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + The VBUS could be configured into the following full scale range (FSR) Key with these range things is to make it clear why they are a wiring thing rather than something userspace should control. Perhaps you can add some detail on that here? > + <0> - VBUS has unipolar +32V to 0V FSR (default) > + <1> - VBUS has bipolar +32V to -32V FSR > + <2> - VBUS has bipolar +16V to -16V FSR > + maximum: 2 > + > + microchip,vsense-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + The VSENSE could be configured into the following full scale range (FSR) > + <0> - VSENSE has unipolar +100 mV to 0V FSR (default) > + <1> - VSENSE has bipolar +100 mV to -100 mV FSR > + <2> - VSENSE has bipolar +50 mV to -50 mV FSR > + maximum: 2 > + > + microchip,accumulation-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 Why is this a hardware thing? Is it specific to particular wiring? Not sure this one belong in DT. > + description: > + The Hardware Accumulator could be configured to accumulate > + VPOWER, VSENSE or VBUS > + <0> - Accumulator accumulates VPOWER (default) > + <1> - Accumulator accumulates VSENSE Add some info on what this is for (charge measurement) Thankfully there is a nice section on why you'd want to do this in the datasheet as only the power option made sense to me. There is a nice statement that you might do this as an extreme form of oversampling as well. > + <2> - Accumulator accumulates VBUS > + maximum: 2 > + > + required: > + - reg > + - shunt-resistor-micro-ohms > + > + unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" Basic power supplies should always be here as chips tend not to run without power. Note that doesn't mean a dts needs to actually list them if they are always on as the regulator framework will provide stubs etc. Jonathan
