| Message ID | 20231110151905.1659873-2-nuno.sa@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for LTC4282 | expand |
Yo, On Fri, Nov 10, 2023 at 04:18:45PM +0100, Nuno Sa wrote: > Add bindings for the LTC4282 High Current Hot Swap Controller with I2C > Compatible Monitoring. > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > --- > .../bindings/hwmon/adi,ltc4282.yaml | 228 ++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 234 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > new file mode 100644 > index 000000000000..0a5d540f014e > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > @@ -0,0 +1,228 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/adi,ltc4282.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C > + > +maintainers: > + - Nuno Sa <nuno.sa@analog.com> > + > +description: | > + Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C. > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4282.pdf > + > + Extra blank line here FYI. > +properties: > + compatible: > + enum: > + - adi,ltc4282 > + > + reg: > + maxItems: 1 > + > + vdd-supply: true > + > + clocks: > + maxItems: 1 > + > + adi,clkout-mode: > + description: | > + Controls in which mode the CLKOUT PIN should work: > + 0 - Configures the CLKOUT pin to output the internal system clock > + 1 - Configures the CLKOUT pin to output the internal conversion time > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1] I really am not a fan of these types of properties. Part of me says that if you're outputting clocks from this device, then you should be a clock controller. How do consumers of this @clkout@ pin get the rate of the clock? I'd kinda be expecting to see a clocks property with a maxItems of 1 and clock-names with two, mutually exclusive, options. The other part says, and it applies in multiple places here, that having integer properties with non-integer meanings is a poor ABI. I'd be vastly happier if the various instances in this file became enums of strings, or $re┤evant-unit so that a dts containing these properties is immediately understandable. Cheers, COnor. > + > + adi,rsense-nano-ohms: > + description: Value of the sense resistor. > + > + adi,vin-mode-microvolt: > + description: > + Selects operating range for the Undervoltage, Overvoltage and Foldback > + pins. Also for the ADC. Should be set to the nominal input voltage. > + enum: [3300000, 5000000, 12000000, 24000000] > + default: 12000000 > + > + adi,fet-bad-timeout-ms: > + description: > + From the moment a FET bad conditions is present, this property selects the > + wait time/timeout for a FET-bad fault to be signaled. Setting this to 0, > + disables FET bad faults to be reported. > + default: 255 > + maximum: 255 > + > + adi,overvoltage-dividers: > + description: | > + Select which dividers to use for VDD Overvoltage detection. Note that > + when the internal dividers are used the threshold is referenced to VDD. > + The percentages in the datasheet are misleading since the actual values > + to look for are in the "Absolute Maximum Ratings" table in the > + "Comparator Inputs" section. In there there's a line for each of the 5%, > + 10% and 15% settings with the actual min, typical and max tolerances. > + 0 - Use the external dividers. > + 1 - Internal dividers 5% > + 2 - Internal dividers 10% > + 3 - Internal dividers 15% > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + default: 0 > + > + adi,undervoltage-dividers: > + description: | > + Select which dividers to use for VDD Overvoltage detection. Note that > + when the internal dividers are used the threshold is referenced to VDD. > + The percentages in the datasheet are misleading since the actual values > + to look for are in the "Absolute Maximum Ratings" table in the > + "Comparator Inputs" section. In there there's a line for each of the 5%, > + 10% and 15% settings with the actual min, typical and max tolerances. > + 0 - Use the external dividers. > + 1 - Internal dividers 5% > + 2 - Internal dividers 10% > + 3 - Internal dividers 15% > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + default: 0 > + > + adi,current-limit-microvolt: > + description: > + The current limit sense voltage of the chip is adjustable between > + 12.5mV and 34.4mV in 3.1mV steps. This effectively limits the current > + on the load. > + enum: [12500, 15625, 18750, 21875, 25000, 28125, 31250, 34375] > + default: 25000 > + > + adi,overcurrent-retry: > + description: > + If set, enables the chip to auto-retry 256 timer cycles after an > + Overcurrent fault. > + type: boolean > + > + adi,overvoltage-retry-disable: > + description: > + If set, disables the chip to auto-retry 50ms after an Overvoltage fault. > + It's enabled by default. > + type: boolean > + > + adi,undervoltage-retry-disable: > + description: > + If set, disables the chip to auto-retry 50ms after an Undervoltage fault. > + It's enabled by default. > + type: boolean > + > + adi,fault-log-enable: > + description: > + If set, enables the FAULT_LOG and ADC_ALERT_LOG registers to be written > + to the EEPROM when a fault bit transitions high and hence, will be > + available after a power cycle (the chip loads the contents of > + the EE_FAULT_LOG register - the one in EEPROM - into FAULT_LOG at boot). > + type: boolean > + > + adi,gpio-alert: > + description: Use the ALERT pin as a GPIO. > + type: boolean > + > + adi,gpio1-mode: > + description: | > + Defines the function of the Pin. > + 0 - GPIO Mode. > + 1 - Power Bad. Goes into high-z to indicate that the output power is > + no longer good. > + 2 - Power Good. Pulls low to indicate that the output power is no > + longer good. > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 2 > + > + adi,gpio2-mode: > + description: | > + Defines the function of the Pin. > + 0 - GPIO Mode. > + 1 - Acts as an input pin and it is feeded into the ADC. > + 2 - Pulls Low when the MOSFET is dissipating power (stress). > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 2 > + > + adi,gpio3-mode: > + description: | > + Defines the function of the Pin. > + 0 - GPIO Mode. > + 1 - Acts as an input pin and it is feeded into the ADC. > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 1 > + > + gpio-controller: > + description: > + This property applies if some of the pins are used as GPIOs. > + > + '#gpio-cells': > + const: 2 > + > +required: > + - compatible > + - reg > + - adi,rsense-nano-ohms > + > +dependencies: > + adi,alert-as-gpio: [gpio-controller, '#gpio-cells'] > + > +allOf: > + - if: > + required: > + - adi,gpio1-mode > + then: > + allOf: > + - if: > + properties: > + adi,gpio0-mode: > + const: 0 > + then: > + dependencies: > + adi,gpio0-mode: [gpio-controller, '#gpio-cells'] > + - if: > + required: > + - adi,gpio2-mode > + then: > + allOf: > + - if: > + properties: > + adi,gpio1-mode: > + const: 0 > + then: > + dependencies: > + adi,gpio1-mode: [gpio-controller, '#gpio-cells'] > + - if: > + required: > + - adi,gpio3-mode > + then: > + allOf: > + - if: > + properties: > + adi,gpio2-mode: > + const: 0 > + then: > + dependencies: > + adi,gpio2-mode: [gpio-controller, '#gpio-cells'] > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + hwmon@50 { > + compatible = "adi,ltc4282"; > + reg = <0x50>; > + adi,rsense-nano-ohms = <500>; > + > + gpio-controller; > + #gpio-cells = <2>; > + > + adi,gpio1-mode = <2>; > + adi,gpio2-mode = <0>; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 43121073390c..9f9527f6057b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12481,6 +12481,12 @@ S: Maintained > F: Documentation/hwmon/ltc4261.rst > F: drivers/hwmon/ltc4261.c > > +LTC4282 HARDWARE MONITOR DRIVER > +M: Nuno Sa <nuno.sa@analog.com> > +L: linux-hwmon@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > + > LTC4306 I2C MULTIPLEXER DRIVER > M: Michael Hennerich <michael.hennerich@analog.com> > L: linux-i2c@vger.kernel.org > -- > 2.42.1 >
On Fri, 2023-11-10 at 18:42 +0000, Conor Dooley wrote: > Yo, > > On Fri, Nov 10, 2023 at 04:18:45PM +0100, Nuno Sa wrote: > > Add bindings for the LTC4282 High Current Hot Swap Controller with I2C > > Compatible Monitoring. > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > > --- > > .../bindings/hwmon/adi,ltc4282.yaml | 228 ++++++++++++++++++ > > MAINTAINERS | 6 + > > 2 files changed, 234 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > new file mode 100644 > > index 000000000000..0a5d540f014e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > @@ -0,0 +1,228 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc4282.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C > > + > > +maintainers: > > + - Nuno Sa <nuno.sa@analog.com> > > + > > +description: | > > + Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C. > > + > > + > > https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4282.pdf > > + > > + > > Extra blank line here FYI. > Right... > > +properties: > > + compatible: > > + enum: > > + - adi,ltc4282 > > + > > + reg: > > + maxItems: 1 > > + > > + vdd-supply: true > > + > > + clocks: > > + maxItems: 1 > > + > > + adi,clkout-mode: > > + description: | > > + Controls in which mode the CLKOUT PIN should work: > > + 0 - Configures the CLKOUT pin to output the internal system clock > > + 1 - Configures the CLKOUT pin to output the internal conversion > > time > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > I really am not a fan of these types of properties. Part of me says that > if you're outputting clocks from this device, then you should be a clock > controller. How do consumers of this @clkout@ pin get the rate of the > clock? I explained it to Guenter as he also argued about this. I'll wait for more feedback but it's likely this will just turn into a clock provider, yes. > I'd kinda be expecting to see a clocks property with a maxItems of 1 and > clock-names with two, mutually exclusive, options. > > The other part says, and it applies in multiple places here, that having > integer properties with non-integer meanings is a poor ABI. I'd be vastly > happier if the various instances in this file became enums of strings, > or $re┤evant-unit so that a dts containing these properties is > immediately understandable. Well, I think you're mentioning the 'gpio-mode' 'and under/over-voltage- dividers'. I think for both it's clear that having the relevant units is not feasible (at least I'm not seeing a way of properly do it). As for the strings, well, I don't have any much to argue other than: 1) It's pattern seen in a lot of bindings - yes, that's not an excuse to copy bad/wrong things over new bindings - but, honestly, it's the first time I have someone complaining about it so I never thought it was wrong. 2) It makes much more easier to handle the properties in the driver (yeah, I know that, as far as you're concerned, this does not matter to you :)) So yeah, if you insist on it, no strong reasons on my side to not do it. As long as I see some consistency down the road :)). - Nuno Sá
On Mon, Nov 13, 2023 at 10:32:17AM +0100, Nuno Sá wrote: > On Fri, 2023-11-10 at 18:42 +0000, Conor Dooley wrote: > > On Fri, Nov 10, 2023 at 04:18:45PM +0100, Nuno Sa wrote: > > > + adi,clkout-mode: > > > + description: | > > > + Controls in which mode the CLKOUT PIN should work: > > > + 0 - Configures the CLKOUT pin to output the internal system clock > > > + 1 - Configures the CLKOUT pin to output the internal conversion > > > time > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [0, 1] > > > > I really am not a fan of these types of properties. Part of me says that > > if you're outputting clocks from this device, then you should be a clock > > controller. How do consumers of this @clkout@ pin get the rate of the > > clock? > > I explained it to Guenter as he also argued about this. I'll wait for more > feedback but it's likely this will just turn into a clock provider, yes. > > > I'd kinda be expecting to see a clocks property with a maxItems of 1 and > > clock-names with two, mutually exclusive, options. > > > > The other part says, and it applies in multiple places here, that having > > integer properties with non-integer meanings is a poor ABI. I'd be vastly > > happier if the various instances in this file became enums of strings, > > or $re┤evant-unit so that a dts containing these properties is > > immediately understandable. > > Well, I think you're mentioning the 'gpio-mode' 'and under/over-voltage- > dividers'. I think for both it's clear that having the relevant units is not > feasible (at least I'm not seeing a way of properly do it). As for the strings, > well, I don't have any much to argue other than: Yeah, sorry - I was kinda making a general point there about not liking having integer values mapped to some sort of meaning, when units or some other sort of more meaningful property is possible. I really do not like these sorts of properties that you go and put "gpio-mode = <3>;" or whatever in the devicetree. I know its not quite units, but you could use 5, 10 & 15 as the allowable values for the divider property and I think that'd be fine. Oh and now that I think of it - for the divider property, how does "adi,undervoltage-dividers = 0" differ from omitting the property altogether? It's not entirely apparently from the binding what that actually means. If they do differ, I think you need to mention what the omission of the property means, and if they do not, then that = 0 case should be removed IMO. > 1) It's pattern seen in a lot of bindings - yes, that's not an excuse to copy > bad/wrong things over new bindings - but, honestly, it's the first time I have > someone complaining about it so I never thought it was wrong. > > 2) It makes much more easier to handle the properties in the driver (yeah, I > know that, as far as you're concerned, this does not matter to you :)) Yeah, with one hat on, sure, I don't care. Realistically I am aware that having these integers makes your life a little easier though. > > So yeah, if you insist on it, no strong reasons on my side to not do it. As long > as I see some consistency down the road :)). From me at least, I try to push people away from these sorts of integer properties and will continue to do so. Cheers, Conor.
On 11/13/23 01:32, Nuno Sá wrote: >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + adi,clkout-mode: >>> + description: | >>> + Controls in which mode the CLKOUT PIN should work: >>> + 0 - Configures the CLKOUT pin to output the internal system clock >>> + 1 - Configures the CLKOUT pin to output the internal conversion >>> time >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1] >> >> I really am not a fan of these types of properties. Part of me says that >> if you're outputting clocks from this device, then you should be a clock >> controller. How do consumers of this @clkout@ pin get the rate of the >> clock? > > I explained it to Guenter as he also argued about this. I'll wait for more > feedback but it's likely this will just turn into a clock provider, yes. > That wasn't an argument, it was a request. If the chip acts as clock provider, it needs to register as one if that functionality is used/provided. Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml new file mode 100644 index 000000000000..0a5d540f014e --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml @@ -0,0 +1,228 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,ltc4282.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C + +maintainers: + - Nuno Sa <nuno.sa@analog.com> + +description: | + Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C. + + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4282.pdf + + +properties: + compatible: + enum: + - adi,ltc4282 + + reg: + maxItems: 1 + + vdd-supply: true + + clocks: + maxItems: 1 + + adi,clkout-mode: + description: | + Controls in which mode the CLKOUT PIN should work: + 0 - Configures the CLKOUT pin to output the internal system clock + 1 - Configures the CLKOUT pin to output the internal conversion time + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + + adi,rsense-nano-ohms: + description: Value of the sense resistor. + + adi,vin-mode-microvolt: + description: + Selects operating range for the Undervoltage, Overvoltage and Foldback + pins. Also for the ADC. Should be set to the nominal input voltage. + enum: [3300000, 5000000, 12000000, 24000000] + default: 12000000 + + adi,fet-bad-timeout-ms: + description: + From the moment a FET bad conditions is present, this property selects the + wait time/timeout for a FET-bad fault to be signaled. Setting this to 0, + disables FET bad faults to be reported. + default: 255 + maximum: 255 + + adi,overvoltage-dividers: + description: | + Select which dividers to use for VDD Overvoltage detection. Note that + when the internal dividers are used the threshold is referenced to VDD. + The percentages in the datasheet are misleading since the actual values + to look for are in the "Absolute Maximum Ratings" table in the + "Comparator Inputs" section. In there there's a line for each of the 5%, + 10% and 15% settings with the actual min, typical and max tolerances. + 0 - Use the external dividers. + 1 - Internal dividers 5% + 2 - Internal dividers 10% + 3 - Internal dividers 15% + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + default: 0 + + adi,undervoltage-dividers: + description: | + Select which dividers to use for VDD Overvoltage detection. Note that + when the internal dividers are used the threshold is referenced to VDD. + The percentages in the datasheet are misleading since the actual values + to look for are in the "Absolute Maximum Ratings" table in the + "Comparator Inputs" section. In there there's a line for each of the 5%, + 10% and 15% settings with the actual min, typical and max tolerances. + 0 - Use the external dividers. + 1 - Internal dividers 5% + 2 - Internal dividers 10% + 3 - Internal dividers 15% + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + default: 0 + + adi,current-limit-microvolt: + description: + The current limit sense voltage of the chip is adjustable between + 12.5mV and 34.4mV in 3.1mV steps. This effectively limits the current + on the load. + enum: [12500, 15625, 18750, 21875, 25000, 28125, 31250, 34375] + default: 25000 + + adi,overcurrent-retry: + description: + If set, enables the chip to auto-retry 256 timer cycles after an + Overcurrent fault. + type: boolean + + adi,overvoltage-retry-disable: + description: + If set, disables the chip to auto-retry 50ms after an Overvoltage fault. + It's enabled by default. + type: boolean + + adi,undervoltage-retry-disable: + description: + If set, disables the chip to auto-retry 50ms after an Undervoltage fault. + It's enabled by default. + type: boolean + + adi,fault-log-enable: + description: + If set, enables the FAULT_LOG and ADC_ALERT_LOG registers to be written + to the EEPROM when a fault bit transitions high and hence, will be + available after a power cycle (the chip loads the contents of + the EE_FAULT_LOG register - the one in EEPROM - into FAULT_LOG at boot). + type: boolean + + adi,gpio-alert: + description: Use the ALERT pin as a GPIO. + type: boolean + + adi,gpio1-mode: + description: | + Defines the function of the Pin. + 0 - GPIO Mode. + 1 - Power Bad. Goes into high-z to indicate that the output power is + no longer good. + 2 - Power Good. Pulls low to indicate that the output power is no + longer good. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 2 + + adi,gpio2-mode: + description: | + Defines the function of the Pin. + 0 - GPIO Mode. + 1 - Acts as an input pin and it is feeded into the ADC. + 2 - Pulls Low when the MOSFET is dissipating power (stress). + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 2 + + adi,gpio3-mode: + description: | + Defines the function of the Pin. + 0 - GPIO Mode. + 1 - Acts as an input pin and it is feeded into the ADC. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 1 + + gpio-controller: + description: + This property applies if some of the pins are used as GPIOs. + + '#gpio-cells': + const: 2 + +required: + - compatible + - reg + - adi,rsense-nano-ohms + +dependencies: + adi,alert-as-gpio: [gpio-controller, '#gpio-cells'] + +allOf: + - if: + required: + - adi,gpio1-mode + then: + allOf: + - if: + properties: + adi,gpio0-mode: + const: 0 + then: + dependencies: + adi,gpio0-mode: [gpio-controller, '#gpio-cells'] + - if: + required: + - adi,gpio2-mode + then: + allOf: + - if: + properties: + adi,gpio1-mode: + const: 0 + then: + dependencies: + adi,gpio1-mode: [gpio-controller, '#gpio-cells'] + - if: + required: + - adi,gpio3-mode + then: + allOf: + - if: + properties: + adi,gpio2-mode: + const: 0 + then: + dependencies: + adi,gpio2-mode: [gpio-controller, '#gpio-cells'] + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + hwmon@50 { + compatible = "adi,ltc4282"; + reg = <0x50>; + adi,rsense-nano-ohms = <500>; + + gpio-controller; + #gpio-cells = <2>; + + adi,gpio1-mode = <2>; + adi,gpio2-mode = <0>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 43121073390c..9f9527f6057b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12481,6 +12481,12 @@ S: Maintained F: Documentation/hwmon/ltc4261.rst F: drivers/hwmon/ltc4261.c +LTC4282 HARDWARE MONITOR DRIVER +M: Nuno Sa <nuno.sa@analog.com> +L: linux-hwmon@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml + LTC4306 I2C MULTIPLEXER DRIVER M: Michael Hennerich <michael.hennerich@analog.com> L: linux-i2c@vger.kernel.org
Add bindings for the LTC4282 High Current Hot Swap Controller with I2C Compatible Monitoring. Signed-off-by: Nuno Sa <nuno.sa@analog.com> --- .../bindings/hwmon/adi,ltc4282.yaml | 228 ++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 234 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml