On Wed, 23 Sep 2020 08:00:29 -0600 Rob Herring <robh@xxxxxxxxxx> wrote: > On Wed, Sep 23, 2020 at 3:07 AM Dmitry Baryshkov > <dmitry.baryshkov@xxxxxxxxxx> wrote: > > > > On 23/09/2020 02:40, Rob Herring wrote: > > > On Mon, Sep 14, 2020 at 06:48:01PM +0300, Dmitry Baryshkov wrote: > > >> Add bindings for thermal monitor, part of Qualcomm PMIC5 chips. It is a > > >> close counterpart of VADC part of those PMICs. > > >> > > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > >> --- > > >> .../bindings/thermal/qcom-spmi-adc-tm5.yaml | 151 ++++++++++++++++++ > > >> 1 file changed, 151 insertions(+) > > >> create mode 100644 Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > > >> > > >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > > >> new file mode 100644 > > >> index 000000000000..432a65839b89 > > >> --- /dev/null > > >> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > > >> @@ -0,0 +1,151 @@ > > >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > >> +%YAML 1.2 > > >> +--- > > >> +$id: http://devicetree.org/schemas/thermal/qcom-spmi-adc-tm5.yaml# > > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >> + > > >> +title: Qualcomm's SPMI PMIC ADC Thermal Monitoring > > >> +maintainers: > > >> + - Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > >> + > > >> +properties: > > >> + compatible: > > >> + const: qcom,spmi-adc-tm5 > > >> + > > >> + reg: > > >> + maxItems: 1 > > >> + > > >> + interrupts: > > >> + maxItems: 1 > > >> + > > >> + "#thermal-sensor-cells": > > >> + const: 1 > > >> + description: > > >> + Number of cells required to uniquely identify the thermal sensors. Since > > >> + we have multiple sensors this is set to 1 > > >> + > > >> + "#address-cells": > > >> + const: 1 > > >> + > > >> + "#size-cells": > > >> + const: 0 > > >> + > > >> + qcom,avg-samples: > > >> + $ref: /schemas/types.yaml#/definitions/uint32 > > >> + description: Number of samples to be used for measurement. > > >> + enum: > > >> + - 1 > > >> + - 2 > > >> + - 4 > > >> + - 8 > > >> + - 16 > > >> + default: 1 > > >> + > > >> + qcom,decimation: > > >> + $ref: /schemas/types.yaml#/definitions/uint32 > > >> + description: This parameter is used to decrease ADC sampling rate. > > >> + Quicker measurements can be made by reducing decimation ratio. > > >> + enum: > > >> + - 250 > > >> + - 420 > > >> + - 840 > > >> + default: 840 > > >> + > > >> +patternProperties: > > >> + "^([-a-z0-9]*)@[0-9]+$": > > > > > > Less than 10 as unit-addresses are hex? > > > > 8 channels at max currently. I'll fix to use hex though. > > Then it should be @[0-7]$ > > > >> + type: object > > >> + description: > > >> + Represent one thermal sensor. > > >> + > > >> + properties: > > >> + reg: > > >> + description: Specify the sensor channel. > > >> + maxItems: 1 > > > > > > You need a range of values here. > > > > ok. > > > > > > > >> + > > >> + io-channels: > > >> + description: > > >> + From common IIO binding. Used to pipe PMIC ADC channel to thermal monitor > > >> + > > >> + qcom,adc-channel: > > >> + $ref: /schemas/types.yaml#/definitions/uint32 > > >> + description: Corresponding ADC channel ID. > > > > > > Why is this not a cell in io-channels? > > > > > > Do you mean parsing a cell from io-channels rather than specifying it > > again? Sounds like a good idea. > > Yes. > > > >> + qcom,ratiometric: > > >> + $ref: /schemas/types.yaml#/definitions/flag > > >> + description: > > >> + Channel calibration type. > > >> + 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. > > >> + > > >> + qcom,hw-settle-time: > > >> + $ref: /schemas/types.yaml#/definitions/uint32 > > >> + description: Time between AMUX getting configured and the ADC starting conversion. > > > > > > Time values should have a unit suffix. Seems like a commmon ADC > > > property... > > > > Could you please be more specific here? Would you like for me to just > > specify the unit in the description? > > More a question for Jonathan I guess as to whether this should be > common or not. Maybe we have something already. Settle or acquisition > time is a common thing for ADCs, right? It's not common in my experience, but not unheard of. Only cases currently supporting controlling it explicitly in the kernel that I can spot, are currently all qcom parts (I'd guess they are all using the same IP underneath) I think it is usually it's a case of building your analog circuitry to match the spec of your ADC / MUX rather than relaxing that spec by adding a delay. It's a property with obvious enough meaning though that I wouldn't have a problem with it being a generic property even it is one that doesn't actually get used much. > > Properties with units need a suffix as defined in > .../bindings/property-units.txt. We have a bunch of legacy bindings that don't have units but all new ones certainly should! Thanks, Jonathan > > Rob
