On 30/10/2024 19:58, Jishnu Prakash wrote: > For the PMIC5-Gen3 type PMICs, ADC peripheral is present in HW for the > following PMICs: PMK8550, PM8550, PM8550B and PM8550VX PMICs. > > It is similar to PMIC5-Gen2, with SW communication to ADCs on all PMICs > going through PBS(Programmable Boot Sequence) firmware through a single > register interface. This interface is implemented on an SDAM (Shared > Direct Access Memory) peripheral on the master PMIC PMK8550 rather > than a dedicated ADC peripheral. > > Add documentation for PMIC5 Gen3 ADC and macro definitions for ADC > channels and virtual channels (combination of ADC channel number and > PMIC SID number) per PMIC, to be used by clients of this device. > > Co-developed-by: Anjelique Melendez <quic_amelende@xxxxxxxxxxx> > Signed-off-by: Anjelique Melendez <quic_amelende@xxxxxxxxxxx> > Signed-off-by: Jishnu Prakash <quic_jprakash@xxxxxxxxxxx> > --- This has still test failures, so limited review follows. > properties: > compatible: > @@ -23,14 +27,20 @@ properties: > - const: qcom,pms405-adc > - const: qcom,spmi-adc-rev2 > - enum: > - - qcom,spmi-vadc > - - qcom,spmi-adc5 > - qcom,spmi-adc-rev2 > + - qcom,spmi-adc5 > + - qcom,spmi-adc5-gen3 > - qcom,spmi-adc7 > + - qcom,spmi-vadc > > reg: > - description: VADC base address in the SPMI PMIC register map > - maxItems: 1 > + description: > + For compatible properties "qcom,spmi-vadc", "qcom,spmi-adc5", "qcom,spmi-adc-rev2" > + and "qcom,spmi-adc7", reg is the VADC base address in the SPMI PMIC register map. > + For compatible property "qcom,spmi-adc5-gen3", each reg corresponds to an SDAM > + peripheral base address that is being used for ADC functionality. This description is not really needed. You need to provide constraints in schema. > + minItems: 1 > + maxItems: 2 > > '#address-cells': > const: 1 > @@ -38,20 +48,28 @@ properties: > '#size-cells': > const: 0 > > + "#thermal-sensor-cells": > + const: 1 > + description: > + Number of cells required to uniquely identify the thermal sensors. Drop, redundant. > + For compatible property "qcom,spmi-adc5-gen3", this property is > + required for if any channels under it are used for ADC_TM. > + Since we have multiple sensors this is set to 1. Drop sentence, redundant. > + > '#io-channel-cells': > const: 1 > > interrupts: > - maxItems: 1 > description: > End of conversion interrupt. > + For compatible property "qcom,spmi-adc5-gen3", interrupts are defined > + for each SDAM being used. Drop descriptions and instead rather list and describe items. You keep repeating schema in free form text. That's not the point. > + minItems: 1 > + maxItems: 2 > > -required: > - - compatible > - - reg > - - '#address-cells' > - - '#size-cells' > - - '#io-channel-cells' > + interrupt-names: > + minItems: 1 > + maxItems: 2 > > patternProperties: > "^channel@[0-9a-f]+$": > @@ -71,8 +89,8 @@ patternProperties: > description: | > ADC channel number. > See include/dt-bindings/iio/adc/qcom,spmi-vadc.h > - For PMIC7 ADC, the channel numbers are specified separately per PMIC > - in the PMIC-specific files in include/dt-bindings/iio/adc. > + For PMIC7 and PMIC5 Gen3 ADC, the channel numbers are specified separately > + per PMIC in the PMIC-specific files in include/dt-bindings/iio/adc. > > label: > description: | > @@ -113,11 +131,11 @@ patternProperties: > channel calibration. If property is not found, channel will be > calibrated with 0.625V and 1.25V reference channels, also > known as absolute calibration. > - - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc7" and > - "qcom,spmi-adc-rev2", if this property is specified VADC will use > - the VDD reference (1.875V) and GND for channel calibration. If > - property is not found, channel will be calibrated with 0V and 1.25V > - reference channels, also known as absolute calibration. > + - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc7", > + "qcom,spmi-adc-rev2" and "qcom,spmi-adc5-gen3", if this property is > + specified VADC will use the VDD reference (1.875V) and GND for channel > + calibration. If property is not found, channel will be calibrated with > + 0V and 1.25V reference channels, also known as absolute calibration. > type: boolean > > qcom,hw-settle-time: > @@ -135,9 +153,24 @@ patternProperties: > from the ADC that is an average of multiple samples. The value > selected is 2^(value). > > + qcom,adc-tm: > + description: > + Indicates if ADC_TM monitoring is done on this channel. What is "ADC_TM"? Why this would be property of a board? This does not look like suitable for DT, at least based on such very vague explanation. > + Defined for compatible property "qcom,spmi-adc5-gen3". Drop redundant. > + This is the same functionality as in the existing QCOM ADC_TM > + device, documented at devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml. What does it mean? How property can represent functionality of entire binding? BTW, use full paths when refering to files. > + type: boolean > + > required: > - reg > > +required: > + - compatible > + - reg > + - '#address-cells' > + - '#size-cells' > + - '#io-channel-cells' > + > allOf: > - if: > properties: > @@ -146,6 +179,15 @@ allOf: > const: qcom,spmi-vadc > > then: > + properties: > + reg: > + minItems: 1 min is redundant. > + maxItems: 1 > + interrupts: > + minItems: 1 > + maxItems: 1 So here you list and describe items instead. > + "#thermal-sensor-cells": false > + interrupt-names: false Keep things properly ordered. xxx-names is always next to xxx. > patternProperties: > "^channel@[0-9a-f]+$": > properties: > @@ -162,6 +204,8 @@ allOf: > enum: [ 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 ] > default: 1 > > + qcom,adc-tm: false > + > - if: > properties: > compatible: > @@ -169,6 +213,15 @@ allOf: > const: qcom,spmi-adc-rev2 > > then: > + properties: > + reg: > + minItems: 1 > + maxItems: 1 > + interrupts: > + minItems: 1 > + maxItems: 1 > + "#thermal-sensor-cells": false > + interrupt-names: false > patternProperties: > "^channel@[0-9a-f]+$": > properties: > @@ -185,6 +238,8 @@ allOf: > enum: [ 1, 2, 4, 8, 16 ] > default: 1 > > + qcom,adc-tm: false > + > - if: > properties: > compatible: > @@ -192,6 +247,15 @@ allOf: > const: qcom,spmi-adc5 > > then: > + properties: > + reg: > + minItems: 1 > + maxItems: 1 > + interrupts: > + minItems: 1 > + maxItems: 1 > + "#thermal-sensor-cells": false > + interrupt-names: false > patternProperties: > "^channel@[0-9a-f]+$": > properties: > @@ -208,6 +272,8 @@ allOf: > enum: [ 1, 2, 4, 8, 16 ] > default: 1 > > + qcom,adc-tm: false > + > - if: > properties: > compatible: > @@ -215,6 +281,59 @@ allOf: > const: qcom,spmi-adc7 > > then: > + properties: > + reg: > + minItems: 1 > + maxItems: 1 > + interrupts: > + minItems: 1 > + maxItems: 1 > + "#thermal-sensor-cells": false > + interrupt-names: false > + patternProperties: > + "^channel@[0-9a-f]+$": > + properties: > + qcom,decimation: > + enum: [ 85, 340, 1360 ] > + default: 1360 > + > + qcom,hw-settle-time: > + enum: [ 15, 100, 200, 300, 400, 500, 600, 700, 1000, 2000, 4000, > + 8000, 16000, 32000, 64000, 128000 ] > + default: 15 > + > + qcom,avg-samples: > + enum: [ 1, 2, 4, 8, 16 ] > + default: 1 > + > + qcom,adc-tm: false > + > + - if: > + properties: > + compatible: > + contains: > + const: qcom,spmi-adc5-gen3 > + > + then: > + properties: > + reg: > + minItems: 1 Why this is flexible? > + items: > + - description: SDAM0 base address in the SPMI PMIC register map > + - description: SDAM1 base address > + interrupts: > + minItems: 1 Why this is flexible? > + items: > + - description: SDAM0 end of conversion (EOC) interrupt > + - description: SDAM1 EOC interrupt > + interrupt-names: > + minItems: 1 > + items: > + - const: adc-sdam0 sdam0 > + - const: adc-sdam1 sdam1 > + required: > + - interrupts > + - interrupt-names > patternProperties: > "^channel@[0-9a-f]+$": > properties: > @@ -307,3 +426,64 @@ examples: Best regards, Krzysztof
