Messages in this thread | | | Date | Wed, 16 Nov 2022 02:00:44 +0530 | Subject | Re: [PATCH v10 1/2] dt-bindings: mfd: Add MAX5970 and MAX5978 | From | Naresh Solanki <> |
| |
Hi Krzysztof,
On 15-11-2022 07:34 pm, Krzysztof Kozlowski wrote: > On 15/11/2022 12:09, Naresh Solanki wrote: >> From: Marcello Sylvester Bauer <sylv@sylv.io> >> >> The MAX597x is a hot swap controller with configurable fault protection. >> It also has 10bit ADC for current & voltage measurements. >> >> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io> >> Co-developed-by: Patrick Rudolph <patrick.rudolph@9elements.com> >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> >> Co-developed-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> --- >> .../bindings/mfd/maxim,max5970.yaml | 154 ++++++++++++++++++ >> 1 file changed, 154 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max5970.yaml >> >> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml >> new file mode 100644 >> index 000000000000..edf0c23db4ca >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml >> @@ -0,0 +1,154 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mfd/maxim,max5970.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Regulator for MAX5970 smart switch from Maxim Integrated. >> + >> +maintainers: >> + - Patrick Rudolph <patrick.rudolph@9elements.com> >> + >> +description: | >> + The smart switch provides no output regulation, but independent fault protection >> + and voltage and current sensing. >> + Programming is done through I2C bus. >> + >> + Datasheets: >> + https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf >> + https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf >> + >> +properties: >> + compatible: >> + enum: >> + - maxim,max5970 >> + - maxim,max5978 >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + leds: >> + type: object >> + description: >> + Properties for four LEDS. >> + >> + properties: >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> + patternProperties: >> + "^led@[0-3]$": >> + $ref: /schemas/leds/common.yaml# >> + type: object >> + >> + additionalProperties: false >> + >> + vss1-supply: >> + description: Supply of the first channel. >> + >> + vss2-supply: >> + description: Supply of the second channel. >> + >> + regulators: >> + type: object >> + description: >> + Properties for both regulators. Also set value for shunt resistor used. > > You should explain not the syntax, but what part of hardware this nodes > represents. Therefore "Also set value" does not fit at all. Hardware > sets value? You mean something like: Properties for power switch > > I looked at datasheets to figure it out but they do not refer to any > configurable regulator, LDO nor "sw0/sw1/sw2". Therefore I have no clue > what to expect here... Yes this is for power switch part of max5970/8 > >> + >> + patternProperties: >> + "^sw[0-1]$": >> + $ref: /schemas/regulator/regulator.yaml# >> + type: object >> + properties: >> + shunt-resistor-micro-ohms: >> + description: | >> + The value of current sense resistor in microohms. >> + >> + required: >> + - shunt-resistor-micro-ohms >> + >> + unevaluatedProperties: false > > I don't think it has proper indentation. Did you test the binding? Definitely tested the bindings before I push the patch. > >> + >> + additionalProperties: false >> + >> +required: >> + - compatible >> + - reg >> + - regulators >> + - vss1-supply >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + enum: >> + - maxim,max5970 >> + then: >> + required: >> + - vss2-supply >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + regulator@3a { >> + compatible = "maxim,max5978"; >> + reg = <0x3a>; >> + vss1-supply = <&p3v3>; >> + >> + regulators { >> + sw0_ref_0: sw0 { >> + regulator-compatible = "SW0"; >> + shunt-resistor-micro-ohms = <12000>; >> + }; >> + }; >> + >> + leds { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + led@0 { >> + reg = <0>; >> + label = "led0"; >> + default-state = "on"; >> + }; >> + led@1 { >> + reg = <1>; >> + label = "led1"; >> + default-state = "on"; >> + }; >> + }; >> + }; >> + }; >> + >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + regulator@3a { >> + compatible = "maxim,max5970"; >> + reg = <0x3a>; >> + vss1-supply = <&p3v3>; >> + vss2-supply = <&p5v>; >> + >> + regulators { >> + sw0_ref_1: sw0 { >> + regulator-compatible = "SW0"; > > This property is deprecated, isn't it? Again - did you test this? Yes tested. > > Best regards, > Krzysztof >
Regards, Naresh
| |