[PATCH v3 0/2] Add support for LTC4282

[PATCH v3 0/2] Add support for LTC4282

[Nuno Sa via B4 Relay]

v1:
 * https://lore.kernel.org/linux-hwmon/20231110151905.1659873-1-nuno.sa@analog.com/

v2:
 * https://lore.kernel.org/linux-hwmon/20231124-ltc4282-support-v2-0-952bf926f83c@analog.com 

Changes in V3:
- Bindings:
   * Remove 'default' from string types;
   * Update gpios descriptions and removed leftovers from when they were
     integer types.
   * Dropped gpio function. With that, adi,gpio3-mode and adi,gpio-alert were
     dropped.
- Driver:
   * Make all clock ops static;
   * Dropped macro to create debugfs attributes;
   * Dropped GPIO support;
   * Removed regulator consumer header (leftover from v1).

There are still some possible interfaces which are dubious from an hwmon
point view. Namely:

* fet_short_fault
* fet_bad_fault
* power1_good

power1_good and fet_short can be "monitored" with gpio1 and 2
respectively so maybe we could remove the. OTHO, some users might want
the pins free for GPIO usage. fet_bad_fault is a status bit (we also have it in
fault_logs) that might be meaningful to monitor (I think).

Also to note, I'm still seeing the following sparse issue:

"CHECK   drivers/hwmon/ltc4282.c
drivers/hwmon/ltc4282.c:805:34: warning: dubious: x & !y
drivers/hwmon/ltc4282.c:895:34: warning: dubious: x & !y" 

However, this does not look to be directly related with the driver. It's
on FIED_PREP() taking values like !val or !!val.

I'm also removing the GPIO maintainers/reviewers from Cc since there's no
gpiochip support anymore. If not adequate, I'll Cc them again...

---
Nuno Sa (2):
      dt-bindings: hwmon: Add LTC4282 bindings
      hwmon: ltc4282: add support for the LTC4282 chip

 .../devicetree/bindings/hwmon/adi,ltc4282.yaml     |  142 ++
 Documentation/hwmon/index.rst                      |    1 +
 Documentation/hwmon/ltc4282.rst                    |  108 ++
 MAINTAINERS                                        |    8 +
 drivers/hwmon/Kconfig                              |   11 +
 drivers/hwmon/Makefile                             |    1 +
 drivers/hwmon/ltc4282.c                            | 1705 ++++++++++++++++++++
 7 files changed, 1976 insertions(+)
---
base-commit: 44482310b7f8ac4cd8fa7be4cee8c1b260ea5ee9
change-id: 20231124-ltc4282-support-8c1675e31508
--

Thanks!
- Nuno Sá

[PATCH v3 1/2] dt-bindings: hwmon: Add LTC4282 bindings

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Add bindings for the LTC4282 High Current Hot Swap Controller with I2C
Compatible Monitoring.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 .../devicetree/bindings/hwmon/adi,ltc4282.yaml     | 142 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 148 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml
new file mode 100644
index 000000000000..be3db64b9c18
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml
@@ -0,0 +1,142 @@
+# 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
+
+  '#clock-cells':
+    const: 0
+
+  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.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
+
+  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.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
+
+  adi,current-limit-sense-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,gpio1-mode:
+    description: Defines the function of the Pin. It can indicate that power is
+      good (PULL the pin low when power is not good) or that power is bad (Go
+      into high-z when power is not good).
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [power_bad, power_good]
+
+  adi,gpio2-mode:
+    description: Defines the function of the Pin. It can be set as the input for
+      the ADC or indicating that the MOSFET is in stress (dissipating power).
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [adc_input, stress_fet]
+
+required:
+  - compatible
+  - reg
+  - adi,rsense-nano-ohms
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hwmon@50 {
+            compatible = "adi,ltc4282";
+            reg = <0x50>;
+            adi,rsense-nano-ohms = <500>;
+
+            adi,gpio1-mode = "power_good";
+            adi,gpio2-mode = "adc_input";
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index cd9591df77d7..1d209f226ffa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12636,6 +12636,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.43.0

Re: [PATCH v3 1/2] dt-bindings: hwmon: Add LTC4282 bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]

On Tue, Dec 05, 2023 at 04:22:55PM +0100, Nuno Sa via B4 Relay wrote:

> +  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.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
> +
> +  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.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]

> +  adi,gpio1-mode:
> +    description: Defines the function of the Pin. It can indicate that power is
> +      good (PULL the pin low when power is not good) or that power is bad (Go
> +      into high-z when power is not good).
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [power_bad, power_good]
> +
> +  adi,gpio2-mode:
> +    description: Defines the function of the Pin. It can be set as the input for
> +      the ADC or indicating that the MOSFET is in stress (dissipating power).
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [adc_input, stress_fet]

None of these properties have a default nor are required.
What happens when they are omitted?

Thanks,
Conor.

> +
> +required:
> +  - compatible
> +  - reg
> +  - adi,rsense-nano-ohms

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 1/2] dt-bindings: hwmon: Add LTC4282 bindings

[Nuno Sá]

On Tue, 2023-12-05 at 16:18 +0000, Conor Dooley wrote:
> On Tue, Dec 05, 2023 at 04:22:55PM +0100, Nuno Sa via B4 Relay wrote:
> 
> > +  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.
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
> > +
> > +  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.
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
> 
> > +  adi,gpio1-mode:
> > +    description: Defines the function of the Pin. It can indicate that power is
> > +      good (PULL the pin low when power is not good) or that power is bad (Go
> > +      into high-z when power is not good).
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [power_bad, power_good]
> > +
> > +  adi,gpio2-mode:
> > +    description: Defines the function of the Pin. It can be set as the input for
> > +      the ADC or indicating that the MOSFET is in stress (dissipating power).
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [adc_input, stress_fet]
> 
> None of these properties have a default nor are required.
> What happens when they are omitted?
> 

Yeah, they get the power on reset default values (or whatever is in eeprom but that's
not the typical usecase). I'll add a default for all these properties. Your question
already made me realize something I need to fix for v4. For some reason, I thought
gpio3 was the default pin to be muxed into the ADC (and hence monitored by hwmon) but
I realized that gpio2 is the default one. Hence, I need a boolean property to allow
for users to mux gpio3 instead of gpio2.

- Nuno Sá

Re: [PATCH v3 1/2] dt-bindings: hwmon: Add LTC4282 bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]

On Wed, Dec 06, 2023 at 09:38:11AM +0100, Nuno Sá wrote:
> On Tue, 2023-12-05 at 16:18 +0000, Conor Dooley wrote:
> > On Tue, Dec 05, 2023 at 04:22:55PM +0100, Nuno Sa via B4 Relay wrote:
> > 
> > > +  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.
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
> > > +
> > > +  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.
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: [external, vdd_5_percent, vdd_10_percent, vdd_15_percent]
> > 
> > > +  adi,gpio1-mode:
> > > +    description: Defines the function of the Pin. It can indicate that power is
> > > +      good (PULL the pin low when power is not good) or that power is bad (Go
> > > +      into high-z when power is not good).
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: [power_bad, power_good]
> > > +
> > > +  adi,gpio2-mode:
> > > +    description: Defines the function of the Pin. It can be set as the input for
> > > +      the ADC or indicating that the MOSFET is in stress (dissipating power).
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: [adc_input, stress_fet]
> > 
> > None of these properties have a default nor are required.
> > What happens when they are omitted?
> > 
> 
> Yeah, they get the power on reset default values (or whatever is in eeprom but that's
> not the typical usecase). I'll add a default for all these properties. Your question
> already made me realize something I need to fix for v4. For some reason, I thought
> gpio3 was the default pin to be muxed into the ADC (and hence monitored by hwmon) but
> I realized that gpio2 is the default one. Hence, I need a boolean property to allow
> for users to mux gpio3 instead of gpio2.

:+1: patch should be good to go then IMO with those defaults added and
the mux one fixed.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

The LTC4282 hot swap controller allows a board to be safely inserted and
removed from a live backplane. Using one or more external N-channel pass
transistors, board supply voltage and inrush current are ramped up at an
adjustable rate. An I2C interface and onboard ADC allows for monitoring
of board current, voltage, power, energy and fault status.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 Documentation/hwmon/index.rst   |    1 +
 Documentation/hwmon/ltc4282.rst |  108 +++
 MAINTAINERS                     |    2 +
 drivers/hwmon/Kconfig           |   11 +
 drivers/hwmon/Makefile          |    1 +
 drivers/hwmon/ltc4282.c         | 1705 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 1828 insertions(+)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 72f4e6065bae..389ced170802 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -128,6 +128,7 @@ Hardware Monitoring Kernel Drivers
    ltc4245
    ltc4260
    ltc4261
+   ltc4282
    max127
    max15301
    max16064
diff --git a/Documentation/hwmon/ltc4282.rst b/Documentation/hwmon/ltc4282.rst
new file mode 100644
index 000000000000..1dfd2e54b405
--- /dev/null
+++ b/Documentation/hwmon/ltc4282.rst
@@ -0,0 +1,108 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel drivers ltc4282
+==========================================
+
+Supported chips:
+
+  * Analog Devices LTC4282
+
+    Prefix: 'ltc4282'
+
+    Addresses scanned: - I2C 0x40 - 0x5A (7-bit)
+    Addresses scanned: - I2C 0x80 - 0xB4 with a step of 2 (8-bit)
+
+    Datasheet:
+
+        https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4282.pdf
+
+Author: Nuno Sá <nuno.sa@analog.com>
+
+Description
+___________
+
+The LTC4282 hot swap controller allows a board to be safely inserted and removed
+from a live backplane. Using one or more external N-channel pass transistors,
+board supply voltage and inrush current are ramped up at an adjustable rate. An
+I2C interface and onboard ADC allows for monitoring of board current, voltage,
+power, energy and fault status. The device features analog foldback current
+limiting and supply monitoring for applications from 2.9V to 33V. Dual 12V gate
+drive allows high power applications to either share safe operating area across
+parallel MOSFETs or support a 2-stage start-up that first charges the load
+capacitance followed by enabling a low on-resistance path to the load. The
+LTC4282 is well suited to high power applications because the precise monitoring
+capability and accurate current limiting reduce the extremes in which both loads
+and power supplies must safely operate. Non-volatile configuration allows for
+flexibility in the autonomous generation of alerts and response to faults.
+
+Sysfs entries
+_____________
+
+The following attributes are supported. Limits are read-write and all the other
+attributes are read-only. Note that in0 and in1 are mutually exclusive. Enabling
+one disables the other and disabling one enables the other.
+
+======================= ==========================================
+in0_input		Output voltage (mV).
+in0_min			Undervoltage threshold
+in0_max                 Overvoltage threshold
+in0_lowest		Lowest measured voltage
+in0_highest		Highest measured voltage
+in0_reset_history	Write 1 to reset in0 history
+in0_min_alarm		Undervoltage alarm
+in0_max_alarm           Overvoltage alarm
+in0_enable		Enable/Disable VSOURCE monitoring
+in0_label		Channel label (VSOURCE)
+
+in1_input		Input voltage (mV).
+in1_min			Undervoltage threshold
+in1_max                 Overvoltage threshold
+in1_lowest		Lowest measured voltage
+in1_highest		Highest measured voltage
+in1_reset_history	Write 1 to reset in1 history
+in1_min_alarm		Undervoltage alarm
+in1_max_alarm           Overvoltage alarm
+in1_lcrit_alarm         Critical Undervoltage alarm
+in1_crit_alarm          Critical Overvoltage alarm
+in1_enable		Enable/Disable VDD monitoring
+in1_label		Channel label (VDD)
+
+in2_input		GPIO voltage (mV)
+in2_min			Undervoltage threshold
+in2_max			Overvoltage threshold
+in2_lowest		Lowest measured voltage
+in2_highest		Highest measured voltage
+in2_reset_history	Write 1 to reset in2 history
+in2_min_alarm		Undervoltage alarm
+in2_max_alarm		Overvoltage alarm
+in2_label		Channel label (VGPIO)
+
+curr1_input		Sense current (mA)
+curr1_min		Undercurrent threshold
+curr1_max		Overcurrent threshold
+curr1_lowest		Lowest measured current
+curr1_highest		Highest measured current
+curr1_reset_history	Write 1 to reset curr1 history
+curr1_min_alarm		Undercurrent alarm
+curr1_max_alarm		Overcurrent alarm
+curr1_crit_alarm        Critical Overcurrent alarm
+curr1_label		Channel label (ISENSE)
+
+power1_input		Power (in uW)
+power1_min		Low power threshold
+power1_max		High power threshold
+power1_input_lowest	Historical minimum power use
+power1_input_highest	Historical maximum power use
+power1_reset_history	Write 1 to reset power1 history
+power1_min_alarm	Low power alarm
+power1_max_alarm	High power alarm
+power1_good		Power considered good
+power1_label		Channel label (Power)
+
+energy1_input		Measured energy over time (in microJoule)
+energy1_enable		Enable/Disable Energy accumulation
+
+fet_short_fault		FET short alarm
+fet_bad_fault		FET bad alarm
+fault_logs_reset	Clears all the Logged Faults
+======================= ==========================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 1d209f226ffa..a03d85572c80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12641,6 +12641,8 @@ M:	Nuno Sa <nuno.sa@analog.com>
 L:	linux-hwmon@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml
+F:	Documentation/hwmon/ltc4282.rst
+F:	drivers/hwmon/ltc4282.c
 
 LTC4306 I2C MULTIPLEXER DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index cf27523eed5a..2f1a0cf2e1e4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1027,6 +1027,17 @@ config SENSORS_LTC4261
 	  This driver can also be built as a module. If so, the module will
 	  be called ltc4261.
 
+config SENSORS_LTC4282
+	tristate "Analog Devices LTC4282"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for Analog Devices LTC4282
+	  High Current Hot Swap Controller I2C interface.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called ltc4282.
+
 config SENSORS_LTQ_CPUTEMP
 	bool "Lantiq cpu temperature sensor driver"
 	depends on SOC_XWAY
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e84bd9685b5c..cbf22d1d736f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_LTC4222)	+= ltc4222.o
 obj-$(CONFIG_SENSORS_LTC4245)	+= ltc4245.o
 obj-$(CONFIG_SENSORS_LTC4260)	+= ltc4260.o
 obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
+obj-$(CONFIG_SENSORS_LTC4282)	+= ltc4282.o
 obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
 obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
 obj-$(CONFIG_SENSORS_MAX127)	+= max127.o
diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
new file mode 100644
index 000000000000..6a5714d25664
--- /dev/null
+++ b/drivers/hwmon/ltc4282.c
@@ -0,0 +1,1705 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C
+ *
+ * Copyright 2023 Analog Devices Inc.
+ */
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/property.h>
+#include <linux/string.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+
+#define LTC4282_CTRL_LSB			0x00
+  #define LTC4282_CTRL_OV_RETRY_MASK		BIT(0)
+  #define LTC4282_CTRL_UV_RETRY_MASK		BIT(1)
+  #define LTC4282_CTRL_OC_RETRY_MASK		BIT(2)
+  #define LTC4282_CTRL_ON_ACTIVE_LOW_MASK	BIT(5)
+  #define LTC4282_CTRL_ON_DELAY_MASK		BIT(6)
+#define LTC4282_CTRL_MSB			0x01
+  #define LTC4282_CTRL_VIN_MODE_MASK		GENMASK(1, 0)
+  #define LTC4282_CTRL_OV_MODE_MASK		GENMASK(3, 2)
+  #define LTC4282_CTRL_UV_MODE_MASK		GENMASK(5, 4)
+#define LTC4282_FAULT_LOG			0x04
+  #define LTC4282_OV_FAULT_MASK			BIT(0)
+  #define LTC4282_UV_FAULT_MASK			BIT(1)
+  #define LTC4282_OC_FAULT_MASK			BIT(2)
+  #define LTC4282_POWER_BAD_FAULT_MASK		BIT(3)
+  #define LTC4282_FET_SHORT_FAULT_MASK		BIT(5)
+  #define LTC4282_FET_BAD_FAULT_MASK		BIT(6)
+#define LTC4282_ADC_ALERT_LOG			0x05
+  #define LTC4282_GPIO_ALARM_L_MASK		BIT(0)
+  #define LTC4282_GPIO_ALARM_H_MASK		BIT(1)
+  #define LTC4282_VSOURCE_ALARM_L_MASK		BIT(2)
+  #define LTC4282_VSOURCE_ALARM_H_MASK		BIT(3)
+  #define LTC4282_VSENSE_ALARM_L_MASK		BIT(4)
+  #define LTC4282_VSENSE_ALARM_H_MASK		BIT(5)
+  #define LTC4282_POWER_ALARM_L_MASK		BIT(6)
+  #define LTC4282_POWER_ALARM_H_MASK		BIT(7)
+#define LTC4282_FET_BAD_FAULT_TIMEOUT		0x06
+  #define LTC4282_FET_BAD_MAX_TIMEOUT		255
+#define LTC4282_GPIO_CONFIG			0x07
+  #define LTC4282_GPIO_2_FET_STRESS_MASK	BIT(1)
+  #define LTC4282_GPIO_1_CONFIG_MASK		GENMASK(5, 4)
+#define LTC4282_VGPIO_MIN			0x08
+#define LTC4282_VGPIO_MAX			0x09
+#define LTC4282_VSOURCE_MIN			0x0a
+#define LTC4282_VSOURCE_MAX			0x0b
+#define LTC4282_VSENSE_MIN			0x0c
+#define LTC4282_VSENSE_MAX			0x0d
+#define LTC4282_POWER_MIN			0x0e
+#define LTC4282_POWER_MAX			0x0f
+#define LTC4282_CLK_DIV				0x10
+  #define LTC4282_CLK_DIV_MASK			GENMASK(4, 0)
+  #define LTC4282_CLKOUT_MASK			GENMASK(6, 5)
+#define LTC4282_ILIM_ADJUST			0x11
+  #define LTC4282_GPIO_MODE_MASK		BIT(1)
+  #define LTC4282_VDD_MONITOR_MASK		BIT(2)
+  #define LTC4282_FOLDBACK_MODE_MASK		GENMASK(4, 3)
+  #define LTC4282_ILIM_ADJUST_MASK		GENMASK(7, 5)
+#define LTC4282_ENERGY				0x12
+#define LTC4282_TIME_COUNTER			0x18
+#define LTC4282_ALERT_CTRL			0x1c
+  #define LTC4282_ALERT_OUT_MASK		BIT(6)
+#define LTC4282_ADC_CTRL			0x1d
+  #define LTC4282_FAULT_LOG_EN_MASK		BIT(2)
+  #define LTC4282_METER_HALT_MASK		BIT(5)
+  #define LTC4282_METER_RESET_MASK		BIT(6)
+  #define LTC4282_RESET_MASK			BIT(7)
+#define LTC4282_STATUS_LSB			0x1e
+  #define LTC4282_OV_STATUS_MASK		BIT(0)
+  #define LTC4282_UV_STATUS_MASK		BIT(1)
+  #define LTC4282_VDD_STATUS_MASK \
+		(LTC4282_OV_STATUS_MASK | LTC4282_UV_STATUS_MASK)
+  #define LTC4282_OC_STATUS_MASK		BIT(2)
+  #define LTC4282_POWER_GOOD_MASK		BIT(3)
+  #define LTC4282_FET_SHORT_MASK		BIT(5)
+  #define LTC4282_FET_BAD_STATUS_MASK		BIT(6)
+#define LTC4282_STATUS_MSB			0x1f
+#define LTC4282_RESERVED_1			0x32
+#define LTC4282_RESERVED_2			0x33
+#define LTC4282_VGPIO				0x34
+#define LTC4282_VGPIO_LOWEST			0x36
+#define LTC4282_VGPIO_HIGHEST			0x38
+#define LTC4282_VSOURCE				0x3a
+#define LTC4282_VSOURCE_LOWEST			0x3c
+#define LTC4282_VSOURCE_HIGHEST			0x3e
+#define LTC4282_VSENSE				0x40
+#define LTC4282_VSENSE_LOWEST			0x42
+#define LTC4282_VSENSE_HIGHEST			0x44
+#define LTC4282_POWER				0x46
+#define LTC4282_POWER_LOWEST			0x48
+#define LTC4282_POWER_HIGHEST			0x4a
+#define LTC4282_RESERVED_3			0x50
+
+#define LTC4282_CLKIN_MIN	(250 * KILO)
+#define LTC4282_CLKIN_MAX	(15500 * KILO)
+#define LTC4282_CLKIN_RANGE	(LTC4282_CLKIN_MAX - LTC4282_CLKIN_MIN + 1)
+#define LTC4282_CLKOUT_SYSTEM	(250 * KILO)
+#define LTC4282_CLKOUT_CNV	15
+
+enum {
+	LTC4282_CHAN_VSOURCE,
+	LTC4282_CHAN_VDD,
+	LTC4282_CHAN_VGPIO,
+};
+
+struct ltc4282_cache {
+	u32 in_max_raw;
+	u32 in_min_raw;
+	long in_highest;
+	long in_lowest;
+	bool en;
+};
+
+struct ltc4282_state {
+	struct regmap *map;
+	/* Protect against multiple accesses to the device registers */
+	struct mutex lock;
+	struct clk_hw clk_hw;
+	/*
+	 * Used to cache values for VDD/VSOURCE depending which will be used
+	 * when hwmon is not enabled for that channel. Needed because they share
+	 * the same registers.
+	 */
+	struct ltc4282_cache in0_1_cache[LTC4282_CHAN_VGPIO];
+	long power_max;
+	u32 rsense;
+	u32 vin_mode;
+	u16 vfs_out;
+	bool energy_en;
+};
+
+enum {
+	LTC4282_VIN_3_3V,
+	LTC4282_VIN_5V,
+	LTC4282_VIN_12V,
+	LTC4282_VIN_24V,
+};
+
+enum {
+	LTC4282_CLKOUT_NONE,
+	LTC4282_CLKOUT_INT,
+	LTC4282_CLKOUT_TICK,
+};
+
+static int ltc4282_set_rate(struct clk_hw *hw,
+			    unsigned long rate, unsigned long parent_rate)
+{
+	struct ltc4282_state *st = container_of(hw, struct ltc4282_state,
+						clk_hw);
+	u32 val = LTC4282_CLKOUT_INT;
+
+	if (rate == LTC4282_CLKOUT_CNV)
+		val = LTC4282_CLKOUT_TICK;
+
+	return regmap_update_bits(st->map, LTC4282_CLK_DIV, LTC4282_CLKOUT_MASK,
+				  FIELD_PREP(LTC4282_CLKOUT_MASK, val));
+}
+
+/*
+ * Note the 15HZ conversion rate assumes 12bit ADC which is what we are
+ * supporting for now.
+ */
+static unsigned int ltc4282_out_rates[] = {
+	LTC4282_CLKOUT_CNV, LTC4282_CLKOUT_SYSTEM
+};
+
+static long ltc4282_round_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long *parent_rate)
+{
+	int idx = find_closest(rate, ltc4282_out_rates,
+			       ARRAY_SIZE(ltc4282_out_rates));
+
+	return ltc4282_out_rates[idx];
+}
+
+static unsigned long ltc4282_recalc_rate(struct clk_hw *hw,
+					 unsigned long parent)
+{
+	struct ltc4282_state *st = container_of(hw, struct ltc4282_state,
+						clk_hw);
+	u32 clkdiv;
+	int ret;
+
+	ret = regmap_read(st->map, LTC4282_CLK_DIV, &clkdiv);
+	if (ret)
+		return 0;
+
+	clkdiv = FIELD_GET(LTC4282_CLKOUT_MASK, clkdiv);
+	if (!clkdiv)
+		return 0;
+	if (clkdiv == LTC4282_CLKOUT_INT)
+		return LTC4282_CLKOUT_SYSTEM;
+
+	return LTC4282_CLKOUT_CNV;
+}
+
+static void ltc4282_disable(struct clk_hw *clk_hw)
+{
+	struct ltc4282_state *st = container_of(clk_hw, struct ltc4282_state,
+						clk_hw);
+
+	regmap_clear_bits(st->map, LTC4282_CLK_DIV, LTC4282_CLKOUT_MASK);
+}
+
+static int ltc4282_read_voltage_word(const struct ltc4282_state *st, u32 reg,
+				     u32 fs, long *val)
+{
+	__be16 in;
+	int ret;
+
+	ret = regmap_bulk_read(st->map, reg, &in, sizeof(in));
+	if (ret)
+		return ret;
+
+	/*
+	 * This is also used to calculate current in which case fs comes in
+	 * 10 * uV. Hence the ULL usage.
+	 */
+	*val = DIV_ROUND_CLOSEST_ULL(be16_to_cpu(in) * (u64)fs, U16_MAX);
+	return 0;
+}
+
+static int ltc4282_read_voltage_byte_cached(const struct ltc4282_state *st,
+					    u32 reg, u32 fs, long *val,
+					    u32 *cached_raw)
+{
+	int ret;
+	u32 in;
+
+	if (cached_raw) {
+		in = *cached_raw;
+	} else {
+		ret = regmap_read(st->map, reg, &in);
+		if (ret)
+			return ret;
+	}
+
+	*val = DIV_ROUND_CLOSEST(in * fs, U8_MAX);
+	return 0;
+}
+
+static int ltc4282_read_voltage_byte(const struct ltc4282_state *st, u32 reg,
+				     u32 fs, long *val)
+{
+	return ltc4282_read_voltage_byte_cached(st, reg, fs, val, NULL);
+}
+
+static int __ltc4282_read_alarm(struct ltc4282_state *st, u32 reg, u32 mask,
+				long *val)
+{
+	u32 alarm;
+	int ret;
+
+	ret = regmap_read(st->map, reg, &alarm);
+	if (ret)
+		return ret;
+
+	*val = !!(alarm & mask);
+
+	/* if not status/fault logs, clear the alarm after reading it */
+	if (reg != LTC4282_STATUS_LSB && reg != LTC4282_FAULT_LOG)
+		return regmap_clear_bits(st->map, reg, mask);
+
+	return 0;
+}
+
+static int ltc4282_read_alarm(struct ltc4282_state *st, u32 reg, u32 mask,
+			      long *val)
+{
+	guard(mutex)(&st->lock);
+	return __ltc4282_read_alarm(st, reg, mask, val);
+}
+
+static int ltc4282_vdd_source_read_in(struct ltc4282_state *st, u32 channel,
+				      long *val)
+{
+	guard(mutex)(&st->lock);
+	if (!st->in0_1_cache[channel].en)
+		return -ENODATA;
+
+	return ltc4282_read_voltage_word(st, LTC4282_VSOURCE, st->vfs_out, val);
+}
+
+static int ltc4282_vdd_source_read_hist(struct ltc4282_state *st, u32 reg,
+					u32 channel, long *cached, long *val)
+{
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (!st->in0_1_cache[channel].en) {
+		*val = *cached;
+		return 0;
+	}
+
+	ret = ltc4282_read_voltage_word(st, reg, st->vfs_out, val);
+	if (ret)
+		return ret;
+
+	*cached = *val;
+	return 0;
+}
+
+static int ltc4282_vdd_source_read_lim(struct ltc4282_state *st, u32 reg,
+				       u32 channel, u32 *cached, long *val)
+{
+	guard(mutex)(&st->lock);
+	if (!st->in0_1_cache[channel].en)
+		return ltc4282_read_voltage_byte_cached(st, reg, st->vfs_out,
+							val, cached);
+
+	return ltc4282_read_voltage_byte(st, reg, st->vfs_out, val);
+}
+
+static int ltc4282_vdd_source_read_alm(struct ltc4282_state *st, u32 mask,
+				       u32 channel, long *val)
+{
+	guard(mutex)(&st->lock);
+	if (!st->in0_1_cache[channel].en) {
+		/*
+		 * Do this otherwise alarms can get confused because we clear
+		 * them after reading them. So, if someone mistakenly reads
+		 * VSOURCE right before VDD (or the other way around), we might
+		 * get no alarm just because it was cleared when reading VSOURCE
+		 * and had no time for a new conversion and thus having the
+		 * alarm again.
+		 */
+		*val = 0;
+		return 0;
+	}
+
+	return __ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG, mask, val);
+}
+
+static int ltc4282_read_in(struct ltc4282_state *st, u32 attr, long *val,
+			   u32 channel)
+{
+	switch (attr) {
+	case hwmon_in_input:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_read_voltage_word(st, LTC4282_VGPIO,
+							 1280, val);
+
+		return ltc4282_vdd_source_read_in(st, channel, val);
+	case hwmon_in_highest:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_read_voltage_word(st,
+							 LTC4282_VGPIO_HIGHEST,
+							 1280, val);
+
+		return ltc4282_vdd_source_read_hist(st, LTC4282_VSOURCE_HIGHEST,
+						    channel,
+						    &st->in0_1_cache[channel].in_highest, val);
+	case hwmon_in_lowest:
+		if (channel == LTC4282_CHAN_VGPIO)
+			ltc4282_read_voltage_word(st, LTC4282_VGPIO_LOWEST,
+						  1280, val);
+
+		return ltc4282_vdd_source_read_hist(st, LTC4282_VSOURCE_LOWEST,
+						    channel,
+						    &st->in0_1_cache[channel].in_lowest, val);
+	case hwmon_in_max_alarm:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+						  LTC4282_GPIO_ALARM_H_MASK,
+						  val);
+
+		return ltc4282_vdd_source_read_alm(st,
+						   LTC4282_VSOURCE_ALARM_H_MASK,
+						   channel, val);
+	case hwmon_in_min_alarm:
+		if (channel == LTC4282_CHAN_VGPIO)
+			ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+					   LTC4282_GPIO_ALARM_L_MASK, val);
+
+		return ltc4282_vdd_source_read_alm(st,
+						   LTC4282_VSOURCE_ALARM_L_MASK,
+						   channel, val);
+	case hwmon_in_crit_alarm:
+		return ltc4282_read_alarm(st, LTC4282_STATUS_LSB,
+					  LTC4282_OV_STATUS_MASK, val);
+	case hwmon_in_lcrit_alarm:
+		return ltc4282_read_alarm(st, LTC4282_STATUS_LSB,
+					  LTC4282_UV_STATUS_MASK, val);
+	case hwmon_in_max:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_read_voltage_byte(st, LTC4282_VGPIO_MAX,
+							 1280, val);
+
+		return ltc4282_vdd_source_read_lim(st, LTC4282_VSOURCE_MAX,
+						   channel,
+						   &st->in0_1_cache[channel].in_max_raw, val);
+	case hwmon_in_min:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_read_voltage_byte(st, LTC4282_VGPIO_MIN,
+							 1280, val);
+
+		return ltc4282_vdd_source_read_lim(st, LTC4282_VSOURCE_MIN,
+						   channel,
+						   &st->in0_1_cache[channel].in_min_raw, val);
+	case hwmon_in_enable:
+		scoped_guard(mutex, &st->lock) {
+			*val = st->in0_1_cache[channel].en;
+		}
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_read_current_word(const struct ltc4282_state *st, u32 reg,
+				     long *val)
+{
+	long in;
+	int ret;
+
+	/*
+	 * We pass in full scale in 10 * micro (note that 40 is already
+	 * millivolt) so we have better approximations to calculate current.
+	 */
+	ret = ltc4282_read_voltage_word(st, reg, DECA * 40 * MILLI, &in);
+	if (ret)
+		return ret;
+
+	*val = DIV_ROUND_CLOSEST(in * MILLI, st->rsense);
+
+	return 0;
+}
+
+static int ltc4282_read_current_byte(const struct ltc4282_state *st, u32 reg,
+				     long *val)
+{
+	long in;
+	int ret;
+
+	ret = ltc4282_read_voltage_byte(st, reg, DECA * 40 * MILLI, &in);
+	if (ret)
+		return ret;
+
+	*val = DIV_ROUND_CLOSEST(in * MILLI, st->rsense);
+
+	return 0;
+}
+
+static int ltc4282_read_curr(struct ltc4282_state *st, const u32 attr,
+			     long *val)
+{
+	switch (attr) {
+	case hwmon_curr_input:
+		return ltc4282_read_current_word(st, LTC4282_VSENSE, val);
+	case hwmon_curr_highest:
+		return ltc4282_read_current_word(st, LTC4282_VSENSE_HIGHEST,
+						 val);
+	case hwmon_curr_lowest:
+		return ltc4282_read_current_word(st, LTC4282_VSENSE_LOWEST,
+						 val);
+	case hwmon_curr_max:
+		return ltc4282_read_current_byte(st, LTC4282_VSENSE_MAX, val);
+	case hwmon_curr_min:
+		return ltc4282_read_current_byte(st, LTC4282_VSENSE_MIN, val);
+	case hwmon_curr_max_alarm:
+		return ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+					  LTC4282_VSENSE_ALARM_H_MASK, val);
+	case hwmon_curr_min_alarm:
+		return ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+					  LTC4282_VSENSE_ALARM_L_MASK, val);
+	case hwmon_curr_crit_alarm:
+		return ltc4282_read_alarm(st, LTC4282_STATUS_LSB,
+					  LTC4282_OC_STATUS_MASK, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_read_power_word(const struct ltc4282_state *st, u32 reg,
+				   long *val)
+{
+	u64 temp =  DECA * 40ULL * st->vfs_out * BIT(16), temp_2;
+	__be16 raw;
+	u16 power;
+	int ret;
+
+	ret = regmap_bulk_read(st->map, reg, &raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	power = be16_to_cpu(raw);
+	/*
+	 * Power is given by:
+	 *     P = CODE(16b) * 0.040 * Vfs(out) * 2^16 / ((2^16 - 1)^2 * Rsense)
+	 */
+	if (check_mul_overflow(power * temp, MICRO, &temp_2)) {
+		temp = DIV_ROUND_CLOSEST_ULL(power * temp, U16_MAX);
+		*val = DIV64_U64_ROUND_CLOSEST(temp * MICRO,
+					       U16_MAX * (u64)st->rsense);
+		return 0;
+	}
+
+	*val = DIV64_U64_ROUND_CLOSEST(temp_2,
+				       st->rsense * int_pow(U16_MAX, 2));
+
+	return 0;
+}
+
+static int ltc4282_read_power_byte(const struct ltc4282_state *st, u32 reg,
+				   long *val)
+{
+	u32 power;
+	u64 temp;
+	int ret;
+
+	ret = regmap_read(st->map, reg, &power);
+	if (ret)
+		return ret;
+
+	temp = power * 40 * DECA * st->vfs_out * BIT_ULL(8);
+	*val = DIV64_U64_ROUND_CLOSEST(temp * MICRO,
+				       int_pow(U8_MAX, 2) * st->rsense);
+
+	return 0;
+}
+
+static int ltc4282_read_energy(const struct ltc4282_state *st, u64 *val)
+{
+	u64 temp, energy;
+	__be64 raw;
+	int ret;
+
+	ret = regmap_bulk_read(st->map, LTC4282_ENERGY, &raw, 6);
+	if (ret)
+		return ret;
+
+	energy =  be64_to_cpu(raw) >> 16;
+	/*
+	 * The formula for energy is given by:
+	 *	E = CODE(48b) * 0.040 * Vfs(out) * Tconv * 256 /
+	 *						((2^16 - 1)^2 * Rsense)
+	 *
+	 * Since we only support 12bit ADC, Tconv = 0.065535s. Passing Vfs(out)
+	 * and 0.040 to mV and Tconv to us, we can simplify the formula to:
+	 *	E = CODE(48b) * 40 * Vfs(out) * 256 / (U16_MAX * Rsense)
+	 *
+	 * As Rsense can have tenths of micro-ohm resolution, we need to
+	 * multiply by DECA to get microujoule.
+	 */
+	if (check_mul_overflow(DECA * st->vfs_out * 40 * BIT(8), energy, &temp)) {
+		temp = DIV_ROUND_CLOSEST(DECA * st->vfs_out * 40 * BIT(8), U16_MAX);
+		*val = DIV_ROUND_CLOSEST_ULL(temp * energy, st->rsense);
+		return 0;
+	}
+
+	*val = DIV64_U64_ROUND_CLOSEST(temp, U16_MAX * (u64)st->rsense);
+
+	return 0;
+}
+
+static int ltc4282_read_power(struct ltc4282_state *st, const u32 attr,
+			      long *val)
+{
+	switch (attr) {
+	case hwmon_power_input:
+		return ltc4282_read_power_word(st, LTC4282_POWER, val);
+	case hwmon_power_input_highest:
+		return ltc4282_read_power_word(st, LTC4282_POWER_HIGHEST, val);
+	case hwmon_power_input_lowest:
+		return ltc4282_read_power_word(st, LTC4282_POWER_LOWEST, val);
+	case hwmon_power_max_alarm:
+		return ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+					  LTC4282_POWER_ALARM_H_MASK, val);
+	case hwmon_power_min_alarm:
+		return ltc4282_read_alarm(st, LTC4282_ADC_ALERT_LOG,
+					  LTC4282_POWER_ALARM_L_MASK, val);
+	case hwmon_power_max:
+		return ltc4282_read_power_byte(st, LTC4282_POWER_MAX, val);
+	case hwmon_power_min:
+		return ltc4282_read_power_byte(st, LTC4282_POWER_MIN, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_read(struct device *dev, enum hwmon_sensor_types type,
+			u32 attr, int channel, long *val)
+{
+	struct ltc4282_state *st = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_in:
+		return ltc4282_read_in(st, attr, val, channel);
+	case hwmon_curr:
+		return ltc4282_read_curr(st, attr, val);
+	case hwmon_power:
+		return ltc4282_read_power(st, attr, val);
+	case hwmon_energy:
+		scoped_guard(mutex, &st->lock) {
+			*val = st->energy_en;
+		}
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_write_power_byte(const struct ltc4282_state *st, u32 reg,
+				    long val)
+{
+	u32 power;
+	u64 temp;
+
+	if (val > st->power_max)
+		val = st->power_max;
+
+	temp = val * int_pow(U8_MAX, 2) * st->rsense;
+	power = DIV64_U64_ROUND_CLOSEST(temp,
+					MICRO * DECA * 256ULL * st->vfs_out * 40);
+
+	return regmap_write(st->map, reg, power);
+}
+
+static int __ltc4282_write_history(const struct ltc4282_state *st, u32 reg,
+				   long lowest, long highest)
+{
+	__be16 __raw;
+	u16 tmp;
+	int ret;
+
+	tmp = DIV_ROUND_CLOSEST(U16_MAX * lowest, st->vfs_out);
+
+	__raw = cpu_to_be16(tmp);
+
+	ret = regmap_bulk_write(st->map, reg, &__raw, 2);
+	if (ret)
+		return ret;
+
+	tmp = DIV_ROUND_CLOSEST(U16_MAX * highest, st->vfs_out);
+
+	__raw = cpu_to_be16(tmp);
+
+	return regmap_bulk_write(st->map, reg + 2, &__raw, 2);
+}
+
+static int ltc4282_write_history(struct ltc4282_state *st, u32 reg, long lowest,
+				 long highest)
+{
+	guard(mutex)(&st->lock);
+	return __ltc4282_write_history(st, reg, lowest, highest);
+}
+
+static int ltc4282_write_power(struct ltc4282_state *st, u32 attr,
+			       long val)
+{
+	switch (attr) {
+	case hwmon_power_max:
+		return ltc4282_write_power_byte(st, LTC4282_POWER_MAX, val);
+	case hwmon_power_min:
+		return ltc4282_write_power_byte(st, LTC4282_POWER_MIN, val);
+	case hwmon_power_reset_history:
+		return ltc4282_write_history(st, LTC4282_POWER_LOWEST, 0, 0);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_write_voltage_byte_cached(const struct ltc4282_state *st,
+					     u32 reg, u32 fs, long val,
+					     u32 *cache_raw)
+{
+	u32 in;
+
+	if (val >= fs)
+		in = U8_MAX;
+	else
+		in = DIV_ROUND_CLOSEST(val * U8_MAX, fs);
+
+	if (cache_raw) {
+		*cache_raw = in;
+		return 0;
+	}
+
+	return regmap_write(st->map, reg, in);
+}
+
+static int ltc4282_write_voltage_byte(const struct ltc4282_state *st, u32 reg,
+				      u32 fs, long val)
+{
+	return ltc4282_write_voltage_byte_cached(st, reg, fs, val, NULL);
+}
+
+static int ltc4282_cache_history(struct ltc4282_state *st, u32 channel)
+{
+	long val;
+	int ret;
+
+	ret = ltc4282_read_voltage_word(st, LTC4282_VSOURCE_LOWEST, st->vfs_out,
+					&val);
+	if (ret)
+		return ret;
+
+	st->in0_1_cache[channel].in_lowest = val;
+
+	ret = ltc4282_read_voltage_word(st, LTC4282_VSOURCE_HIGHEST,
+					st->vfs_out, &val);
+	if (ret)
+		return ret;
+
+	st->in0_1_cache[channel].in_highest = val;
+
+	ret = regmap_read(st->map, LTC4282_VSOURCE_MIN,
+			  &st->in0_1_cache[channel].in_min_raw);
+	if (ret)
+		return ret;
+
+	return regmap_read(st->map, LTC4282_VSOURCE_MAX,
+			  &st->in0_1_cache[channel].in_max_raw);
+}
+
+static int ltc4282_cache_sync(struct ltc4282_state *st, u32 channel)
+{
+	int ret;
+
+	ret = __ltc4282_write_history(st, LTC4282_VSOURCE_LOWEST,
+				      st->in0_1_cache[channel].in_lowest,
+				      st->in0_1_cache[channel].in_highest);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->map, LTC4282_VSOURCE_MIN,
+			   st->in0_1_cache[channel].in_min_raw);
+
+	return regmap_write(st->map, LTC4282_VSOURCE_MAX,
+			    st->in0_1_cache[channel].in_max_raw);
+}
+
+static int ltc4282_vdd_source_write_lim(struct ltc4282_state *st, u32 reg,
+					int channel, u32 *cache, long val)
+{
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (st->in0_1_cache[channel].en)
+		ret = ltc4282_write_voltage_byte(st, reg, st->vfs_out, val);
+	else
+		ret = ltc4282_write_voltage_byte_cached(st, reg, st->vfs_out,
+							val, cache);
+
+	return ret;
+}
+
+static int ltc4282_vdd_source_write_hist(struct ltc4282_state *st, int channel)
+{
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (st->in0_1_cache[channel].en) {
+		ret = __ltc4282_write_history(st, LTC4282_VSOURCE_LOWEST, 0, 0);
+		if (ret)
+			return ret;
+	}
+
+	st->in0_1_cache[channel].in_lowest = 0;
+	st->in0_1_cache[channel].in_highest = 0;
+
+	return 0;
+}
+
+/*
+ * We need to mux between VSOURCE and VDD which means they are mutually
+ * exclusive. Moreover, we can't really disable both VDD and VSOURCE as the ADC
+ * is continuously running (we cannot independently halt it without also
+ * stopping VGPIO). Hence, the logic is that disabling or enabling VDD will
+ * automatically have the reverse effect on VSOURCE and vice-versa.
+ */
+static int ltc4282_vdd_source_enable(struct ltc4282_state *st, int channel,
+				     long val)
+{
+	int ret, other_chan = ~channel & 0x1;
+	u8 __val = val;
+
+	guard(mutex)(&st->lock);
+	if (st->in0_1_cache[channel].en == !!val)
+		return 0;
+
+	/* clearing the bit makes the ADC to monitor VDD */
+	if (channel == LTC4282_CHAN_VDD)
+		__val = !__val;
+
+	ret = regmap_update_bits(st->map, LTC4282_ILIM_ADJUST,
+				 LTC4282_VDD_MONITOR_MASK,
+				 FIELD_PREP(LTC4282_VDD_MONITOR_MASK, !!__val));
+	if (ret)
+		return ret;
+
+	st->in0_1_cache[channel].en = !!val;
+	st->in0_1_cache[other_chan].en = !val;
+
+	if (st->in0_1_cache[channel].en) {
+		/*
+		 * Then, we are disabling @other_chan. Let's save it's current
+		 * history.
+		 */
+		ret = ltc4282_cache_history(st, other_chan);
+		if (ret)
+			return ret;
+
+		return ltc4282_cache_sync(st, channel);
+	}
+	/*
+	 * Then, we are enabling @other_chan. We need to do the opposite from
+	 * above.
+	 */
+	ret = ltc4282_cache_history(st, channel);
+	if (ret)
+		return ret;
+
+	return ltc4282_cache_sync(st, other_chan);
+}
+
+static int ltc4282_write_in(struct ltc4282_state *st, u32 attr, long val,
+			    int channel)
+{
+	switch (attr) {
+	case hwmon_in_max:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_write_voltage_byte(st, LTC4282_VGPIO_MAX,
+							  1280, val);
+
+		return ltc4282_vdd_source_write_lim(st, LTC4282_VSOURCE_MAX,
+						    channel,
+						    &st->in0_1_cache[channel].in_max_raw, val);
+	case hwmon_in_min:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_write_voltage_byte(st, LTC4282_VGPIO_MIN,
+							  1280, val);
+
+		return ltc4282_vdd_source_write_lim(st, LTC4282_VSOURCE_MIN,
+						    channel,
+						    &st->in0_1_cache[channel].in_min_raw, val);
+	case hwmon_in_reset_history:
+		if (channel == LTC4282_CHAN_VGPIO)
+			return ltc4282_write_history(st, LTC4282_VGPIO_LOWEST,
+						     0, 0);
+
+		return ltc4282_vdd_source_write_hist(st, channel);
+	case hwmon_in_enable:
+		return ltc4282_vdd_source_enable(st, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_write_curr(struct ltc4282_state *st, u32 attr,
+			      long val)
+{
+	/* need to pass it in millivolt */
+	u32 in = DIV_ROUND_CLOSEST_ULL((u64)val * st->rsense, DECA * MICRO);
+
+	switch (attr) {
+	case hwmon_curr_max:
+		return ltc4282_write_voltage_byte(st, LTC4282_VSENSE_MAX, 40,
+						  in);
+	case hwmon_curr_min:
+		return ltc4282_write_voltage_byte(st, LTC4282_VSENSE_MIN, 40,
+						  in);
+	case hwmon_curr_reset_history:
+		return ltc4282_write_history(st, LTC4282_VSENSE_LOWEST, 0, 0);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ltc4282_energy_enable_set(struct ltc4282_state *st, long val)
+{
+	int ret;
+
+	guard(mutex)(&st->lock);
+	/* setting the bit halts the meter */
+	ret = regmap_update_bits(st->map, LTC4282_ADC_CTRL,
+				 LTC4282_METER_HALT_MASK,
+				 FIELD_PREP(LTC4282_METER_HALT_MASK, !val));
+	if (ret)
+		return ret;
+
+	st->energy_en = !!val;
+
+	return 0;
+}
+
+static int ltc4282_write(struct device *dev,
+			 enum hwmon_sensor_types type,
+			 u32 attr, int channel, long val)
+{
+	struct ltc4282_state *st = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_power:
+		return ltc4282_write_power(st, attr, val);
+	case hwmon_in:
+		return ltc4282_write_in(st, attr, val, channel);
+	case hwmon_curr:
+		return ltc4282_write_curr(st, attr, val);
+	case hwmon_energy:
+		return ltc4282_energy_enable_set(st, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t ltc4282_in_is_visible(const struct ltc4282_state *st, u32 attr)
+{
+	switch (attr) {
+	case hwmon_in_input:
+	case hwmon_in_highest:
+	case hwmon_in_lowest:
+	case hwmon_in_max_alarm:
+	case hwmon_in_min_alarm:
+	case hwmon_in_label:
+	case hwmon_in_lcrit_alarm:
+	case hwmon_in_crit_alarm:
+		return 0444;
+	case hwmon_in_max:
+	case hwmon_in_min:
+	case hwmon_in_enable:
+	case hwmon_in_reset_history:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static umode_t ltc4282_curr_is_visible(u32 attr)
+{
+	switch (attr) {
+	case hwmon_curr_input:
+	case hwmon_curr_highest:
+	case hwmon_curr_lowest:
+	case hwmon_curr_max_alarm:
+	case hwmon_curr_min_alarm:
+	case hwmon_curr_crit_alarm:
+	case hwmon_curr_label:
+		return 0444;
+	case hwmon_curr_max:
+	case hwmon_curr_min:
+	case hwmon_curr_reset_history:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static umode_t ltc4282_power_is_visible(u32 attr)
+{
+	switch (attr) {
+	case hwmon_power_input:
+	case hwmon_power_input_highest:
+	case hwmon_power_input_lowest:
+	case hwmon_power_label:
+	case hwmon_power_max_alarm:
+	case hwmon_power_min_alarm:
+		return 0444;
+	case hwmon_power_max:
+	case hwmon_power_min:
+	case hwmon_power_reset_history:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static umode_t ltc4282_is_visible(const void *data,
+				  enum hwmon_sensor_types type,
+				  u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_in:
+		return ltc4282_in_is_visible(data, attr);
+	case hwmon_curr:
+		return ltc4282_curr_is_visible(attr);
+	case hwmon_power:
+		return ltc4282_power_is_visible(attr);
+	case hwmon_energy:
+		/* hwmon_energy_enable */
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static const char * const ltc4282_in_strs[] = {
+	"VSOURCE", "VDD", "VGPIO"
+};
+
+static int ltc4282_read_labels(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, const char **str)
+{
+	switch (type) {
+	case hwmon_in:
+		*str = ltc4282_in_strs[channel];
+		return 0;
+	case hwmon_curr:
+		*str = "ISENSE";
+		return 0;
+	case hwmon_power:
+		*str = "Power";
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static ssize_t ltc4282_energy_show(struct device *dev,
+				   struct device_attribute *da, char *buf)
+{
+	struct ltc4282_state *st = dev_get_drvdata(dev);
+	u64 energy;
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (!st->energy_en)
+		return -ENODATA;
+
+	ret = ltc4282_read_energy(st, &energy);
+	if (ret < 0)
+		return ret;
+
+	return sysfs_emit(buf, "%llu\n", energy);
+}
+
+static ssize_t ltc4282_status_show(struct device *dev,
+				   struct device_attribute *da, char *buf)
+{
+	struct ltc4282_state *st = dev_get_drvdata(dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	long alarm;
+	int ret;
+
+	ret = ltc4282_read_alarm(st, LTC4282_STATUS_LSB, attr->index, &alarm);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", alarm);
+}
+
+static ssize_t ltc4282_clear_faults_store(struct device *dev,
+					  struct device_attribute *da,
+					  const char *buf, size_t len)
+{
+	struct ltc4282_state *st = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_write(st->map, LTC4282_FAULT_LOG, 0);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static const struct clk_ops ltc4282_ops = {
+	.recalc_rate = ltc4282_recalc_rate,
+	.round_rate = ltc4282_round_rate,
+	.set_rate = ltc4282_set_rate,
+	.disable = ltc4282_disable,
+};
+
+static int ltc428_clk_provider_setup(struct ltc4282_state *st,
+				     struct device *dev)
+{
+	struct clk_init_data init;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_COMMON_CLK))
+		return 0;
+
+	init.name = "ltc4282-clk";
+	init.ops = &ltc4282_ops;
+	init.flags = CLK_GET_RATE_NOCACHE;
+	st->clk_hw.init = &init;
+
+	ret = devm_clk_hw_register(dev, &st->clk_hw);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+					   &st->clk_hw);
+}
+
+static int ltc428_clks_setup(struct ltc4282_state *st, struct device *dev)
+{
+	unsigned long rate;
+	struct clk *clkin;
+	u32 val;
+	int ret;
+
+	ret = ltc428_clk_provider_setup(st, dev);
+	if (ret)
+		return ret;
+
+	clkin = devm_clk_get_optional_enabled(dev, NULL);
+	if (IS_ERR(clkin))
+		return dev_err_probe(dev, PTR_ERR(clkin),
+				     "Failed to get clkin");
+	if (!clkin)
+		return 0;
+
+	rate = clk_get_rate(clkin);
+	if (in_range(rate, LTC4282_CLKIN_MIN, LTC4282_CLKIN_RANGE))
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid clkin range(%lu) [%lu %lu]\n",
+				     rate, LTC4282_CLKIN_MIN,
+				     LTC4282_CLKIN_MAX);
+
+	/*
+	 * Clocks faster than 250KHZ should be reduced to 250KHZ. The clock
+	 * frequency is divided by twice the value in the register.
+	 */
+	val = rate / (2 * LTC4282_CLKIN_MIN);
+
+	return regmap_update_bits(st->map, LTC4282_CLK_DIV,
+				  LTC4282_CLK_DIV_MASK,
+				  FIELD_PREP(LTC4282_CLK_DIV_MASK, val));
+}
+
+static const int ltc4282_curr_lim_uv[] = {
+	12500, 15625, 18750, 21875, 25000, 28125, 31250, 34375
+};
+
+static int ltc4282_get_defaults(struct ltc4282_state *st, u32 *curr_lim_uv)
+{
+	u32 reg_val, ilm_adjust;
+	int ret;
+
+	ret = regmap_read(st->map, LTC4282_ADC_CTRL, &reg_val);
+	if (ret)
+		return ret;
+
+	st->energy_en = !FIELD_GET(LTC4282_METER_HALT_MASK, reg_val);
+
+	ret = regmap_read(st->map, LTC4282_CTRL_MSB, &reg_val);
+	if (ret)
+		return ret;
+
+	st->vin_mode = FIELD_GET(LTC4282_CTRL_VIN_MODE_MASK, reg_val);
+
+	ret = regmap_read(st->map, LTC4282_ILIM_ADJUST, &reg_val);
+	if (ret)
+		return ret;
+
+	ilm_adjust = FIELD_GET(LTC4282_ILIM_ADJUST_MASK, reg_val);
+	*curr_lim_uv = ltc4282_curr_lim_uv[ilm_adjust];
+
+	st->in0_1_cache[LTC4282_CHAN_VSOURCE].en = FIELD_GET(LTC4282_VDD_MONITOR_MASK,
+							     ilm_adjust);
+	if (!st->in0_1_cache[LTC4282_CHAN_VSOURCE].en) {
+		st->in0_1_cache[LTC4282_CHAN_VDD].en = true;
+		return regmap_read(st->map, LTC4282_VSOURCE_MAX,
+				   &st->in0_1_cache[LTC4282_CHAN_VSOURCE].in_max_raw);
+	}
+
+	return regmap_read(st->map, LTC4282_VSOURCE_MAX,
+			   &st->in0_1_cache[LTC4282_CHAN_VDD].in_max_raw);
+}
+
+/*
+ * Set max limits for ISENSE and Power as that depends on the max voltage on
+ * rsense that is defined in ILIM_ADJUST. This is specially important for power
+ * because for some rsense and vfsout values, if we allow the default raw 255
+ * value, that would overflow long in 32bit archs when reading back the max
+ * power limit.
+ */
+static int ltc4282_set_max_limits(struct ltc4282_state *st, u32 val_uv)
+{
+	int ret;
+
+	ret = ltc4282_write_voltage_byte(st, LTC4282_VSENSE_MAX, 40 * MILLI,
+					 val_uv);
+	if (ret)
+		return ret;
+
+	/* Power is given by ISENSE * Vout. */
+	st->power_max = DIV_ROUND_CLOSEST(val_uv * DECA * MILLI, st->rsense) * st->vfs_out;
+	return ltc4282_write_power_byte(st, LTC4282_POWER_MAX, st->power_max);
+}
+
+static const char * const ltc4282_gpio1_modes[] = {
+	"power_bad", "power_good"
+};
+
+static const char * const ltc4282_gpio2_modes[] = {
+	"adc_input", "stress_fet"
+};
+
+static int ltc4282_gpio_setup(struct ltc4282_state *st, struct device *dev)
+{
+	const char *func;
+	int ret;
+
+	ret = device_property_read_string(dev, "adi,gpio1-mode", &func);
+	if (!ret) {
+		ret = match_string(ltc4282_gpio1_modes,
+				   ARRAY_SIZE(ltc4282_gpio1_modes), func);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "Invalid func(%s) for gpio1\n",
+					     func);
+
+		ret = regmap_update_bits(st->map, LTC4282_GPIO_CONFIG,
+					 LTC4282_GPIO_1_CONFIG_MASK,
+					 FIELD_PREP(LTC4282_GPIO_1_CONFIG_MASK, ret));
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_string(dev, "adi,gpio2-mode", &func);
+	if (ret)
+		return 0;
+
+	ret = match_string(ltc4282_gpio2_modes, ARRAY_SIZE(ltc4282_gpio2_modes),
+			   func);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Invalid func(%s) for gpio2\n",
+				     func);
+	if (!ret)
+		/* setting the bit to 1 cause the ADC to monitor GPIO2 */
+		return regmap_set_bits(st->map, LTC4282_ILIM_ADJUST,
+				       LTC4282_GPIO_MODE_MASK);
+
+	return regmap_update_bits(st->map, LTC4282_GPIO_CONFIG,
+				  LTC4282_GPIO_2_FET_STRESS_MASK,
+				  FIELD_PREP(LTC4282_GPIO_2_FET_STRESS_MASK, 1));
+}
+
+static const char * const ltc4282_dividers[] = {
+	"external", "vdd_5_percent", "vdd_10_percent", "vdd_15_percent"
+};
+
+/* This maps the Vout full scale for the given Vin mode */
+static const u16 ltc4282_vfs_milli[] = { 5540, 8320, 16640, 33280 };
+
+static int ltc4282_setup(struct ltc4282_state *st, struct device *dev)
+{
+	u32 val, curr_lim_uv;
+	const char *divider;
+	int ret;
+
+	/* The part has an eeprom so let's get the needed defaults from it */
+	ret = ltc4282_get_defaults(st, &curr_lim_uv);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u32(dev, "adi,rsense-nano-ohms",
+				       &st->rsense);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to read adi,rsense-nano-ohms\n");
+	if (st->rsense < CENTI)
+		return dev_err_probe(dev, -EINVAL,
+				     "adi,rsense-nano-ohms too small (< %lu)\n",
+				     CENTI);
+
+	/*
+	 * The resolution for rsense is tenths of micro (eg: 62.5 uOhm) which
+	 * means we need nano in the bindings. However, to make things easier to
+	 * handle (with respect to overflows) we divide it by 100 as we don't
+	 * really need the last two digits.
+	 */
+	st->rsense /= CENTI;
+
+	ret = device_property_read_u32(dev, "adi,vin-mode-microvolt", &val);
+	if (!ret) {
+		switch (val) {
+		case 3300000:
+			st->vin_mode = LTC4282_VIN_3_3V;
+			break;
+		case 5000000:
+			st->vin_mode = LTC4282_VIN_5V;
+			break;
+		case 12000000:
+			st->vin_mode = LTC4282_VIN_12V;
+			break;
+		case 24000000:
+			st->vin_mode = LTC4282_VIN_24V;
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid val(%u) for vin-mode-microvolt\n",
+					     val);
+		}
+
+		ret = regmap_update_bits(st->map, LTC4282_CTRL_MSB,
+					 LTC4282_CTRL_VIN_MODE_MASK,
+					 FIELD_PREP(LTC4282_CTRL_VIN_MODE_MASK, st->vin_mode));
+		if (ret)
+			return ret;
+
+		/* Foldback mode should also be set to the input voltage */
+		ret = regmap_update_bits(st->map, LTC4282_ILIM_ADJUST,
+					 LTC4282_FOLDBACK_MODE_MASK,
+					 FIELD_PREP(LTC4282_FOLDBACK_MODE_MASK, st->vin_mode));
+		if (ret)
+			return ret;
+	}
+
+	st->vfs_out = ltc4282_vfs_milli[st->vin_mode];
+
+	ret = device_property_read_u32(dev, "adi,current-limit-sense-microvolt",
+				       &curr_lim_uv);
+	if (!ret) {
+		int reg_val;
+
+		switch (val) {
+		case 12500:
+			reg_val = 0;
+			break;
+		case 15625:
+			reg_val = 1;
+			break;
+		case 18750:
+			reg_val = 2;
+			break;
+		case 21875:
+			reg_val = 3;
+			break;
+		case 25000:
+			reg_val = 4;
+			break;
+		case 28125:
+			reg_val = 5;
+			break;
+		case 31250:
+			reg_val = 6;
+			break;
+		case 34375:
+			reg_val = 7;
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid val(%u) for adi,current-limit-microvolt\n",
+					     val);
+		}
+
+		ret = regmap_update_bits(st->map, LTC4282_ILIM_ADJUST,
+					 LTC4282_ILIM_ADJUST_MASK,
+					 FIELD_PREP(LTC4282_ILIM_ADJUST_MASK, reg_val));
+		if (ret)
+			return ret;
+	}
+
+	ret = ltc4282_set_max_limits(st, curr_lim_uv);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_string(dev, "adi,overvoltage-dividers",
+					  &divider);
+	if (!ret) {
+		int div = match_string(ltc4282_dividers,
+				       ARRAY_SIZE(ltc4282_dividers), divider);
+		if (div < 0)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid val(%s) for adi,overvoltage-divider\n",
+					     divider);
+
+		ret = regmap_update_bits(st->map, LTC4282_CTRL_MSB,
+					 LTC4282_CTRL_OV_MODE_MASK,
+					 FIELD_PREP(LTC4282_CTRL_OV_MODE_MASK, div));
+	}
+
+	ret = device_property_read_string(dev, "adi,undervoltage-dividers",
+					  &divider);
+	if (!ret) {
+		int div = match_string(ltc4282_dividers,
+				       ARRAY_SIZE(ltc4282_dividers), divider);
+		if (div < 0)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid val(%s) for adi,undervoltage-divider\n",
+					     divider);
+
+		ret = regmap_update_bits(st->map, LTC4282_CTRL_MSB,
+					 LTC4282_CTRL_UV_MODE_MASK,
+					 FIELD_PREP(LTC4282_CTRL_UV_MODE_MASK, div));
+	}
+
+	if (device_property_read_bool(dev, "adi,overcurrent-retry")) {
+		ret = regmap_set_bits(st->map, LTC4282_CTRL_LSB,
+				      LTC4282_CTRL_OC_RETRY_MASK);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "adi,overvoltage-retry-disable")) {
+		ret = regmap_clear_bits(st->map, LTC4282_CTRL_LSB,
+					LTC4282_CTRL_OV_RETRY_MASK);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "adi,undervoltage-retry-disable")) {
+		ret = regmap_clear_bits(st->map, LTC4282_CTRL_LSB,
+					LTC4282_CTRL_UV_RETRY_MASK);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "adi,fault-log-enable")) {
+		ret = regmap_set_bits(st->map, LTC4282_ADC_CTRL,
+				      LTC4282_FAULT_LOG_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "adi,fault-log-enable")) {
+		ret = regmap_set_bits(st->map, LTC4282_ADC_CTRL, LTC4282_FAULT_LOG_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_u32(dev, "adi,fet-bad-timeout-ms", &val);
+	if (!ret) {
+		if (val > LTC4282_FET_BAD_MAX_TIMEOUT)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid value(%u) for adi,fet-bad-timeout-ms",
+					     val);
+
+		ret = regmap_write(st->map, LTC4282_FET_BAD_FAULT_TIMEOUT, val);
+		if (ret)
+			return ret;
+	}
+
+	return ltc4282_gpio_setup(st, dev);
+}
+
+static bool ltc4282_readable_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == LTC4282_RESERVED_1 || reg == LTC4282_RESERVED_2)
+		return false;
+
+	return true;
+}
+
+static bool ltc4282_writable_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == LTC4282_STATUS_LSB || reg == LTC4282_STATUS_MSB)
+		return false;
+	if (reg == LTC4282_RESERVED_1 || reg == LTC4282_RESERVED_2)
+		return false;
+
+	return true;
+}
+
+static const struct regmap_config ltc4282_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = LTC4282_RESERVED_3,
+	.readable_reg = ltc4282_readable_reg,
+	.writeable_reg = ltc4282_writable_reg,
+};
+
+static const struct hwmon_channel_info * const ltc4282_info[] = {
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
+			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
+			   HWMON_I_MAX_ALARM | HWMON_I_ENABLE |
+			   HWMON_I_RESET_HISTORY | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
+			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
+			   HWMON_I_MAX_ALARM | HWMON_I_LCRIT_ALARM |
+			   HWMON_I_CRIT_ALARM | HWMON_I_ENABLE |
+			   HWMON_I_RESET_HISTORY | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
+			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
+			   HWMON_I_MAX_ALARM | HWMON_I_LABEL),
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST |
+			   HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM |
+			   HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM |
+			   HWMON_I_RESET_HISTORY | HWMON_C_LABEL),
+	HWMON_CHANNEL_INFO(power,
+			   HWMON_P_INPUT | HWMON_P_INPUT_LOWEST |
+			   HWMON_P_INPUT_HIGHEST | HWMON_P_MAX | HWMON_P_MIN |
+			   HWMON_P_MAX_ALARM | HWMON_P_MIN_ALARM |
+			   HWMON_I_RESET_HISTORY | HWMON_P_LABEL),
+	HWMON_CHANNEL_INFO(energy,
+			   HWMON_E_ENABLE),
+	NULL
+};
+
+static const struct hwmon_ops ltc4282_hwmon_ops = {
+	.read = ltc4282_read,
+	.write = ltc4282_write,
+	.is_visible = ltc4282_is_visible,
+	.read_string = ltc4282_read_labels,
+};
+
+static const struct hwmon_chip_info ltc2947_chip_info = {
+	.ops = &ltc4282_hwmon_ops,
+	.info = ltc4282_info,
+};
+
+/* energy attributes are 6bytes wide so we need u64 */
+static SENSOR_DEVICE_ATTR_RO(energy1_input, ltc4282_energy, 0);
+/* power1_fault */
+static SENSOR_DEVICE_ATTR_RO(power1_good, ltc4282_status,
+			     LTC4282_POWER_GOOD_MASK);
+/* FET faults */
+static SENSOR_DEVICE_ATTR_RO(fet_short_fault, ltc4282_status,
+			     LTC4282_FET_SHORT_MASK);
+static SENSOR_DEVICE_ATTR_RO(fet_bad_fault, ltc4282_status,
+			     LTC4282_FET_BAD_STATUS_MASK);
+/*
+ * Fault log failures. These faults might be important in systems where
+ * auto-retry is not enabled since they will cause the part to latch off until
+ * they are cleared. Typically that happens when the system admin is close
+ * enough so he can check what happened and manually clear the faults. Hence, we
+ * provide an attribute to clear all fauls at once while still capable of
+ * checking individual faults in debugfs.
+ */
+static SENSOR_DEVICE_ATTR_WO(fault_logs_reset, ltc4282_clear_faults, 0);
+
+static struct attribute *ltc4282_attrs[] = {
+	&sensor_dev_attr_energy1_input.dev_attr.attr,
+	&sensor_dev_attr_power1_good.dev_attr.attr,
+	&sensor_dev_attr_fet_bad_fault.dev_attr.attr,
+	&sensor_dev_attr_fet_short_fault.dev_attr.attr,
+	&sensor_dev_attr_fault_logs_reset.dev_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ltc4282);
+
+static int ltc4282_show_fault_log(void *arg, u64 *val, u32 mask)
+{
+	struct ltc4282_state *st = arg;
+	long alarm;
+	int ret;
+
+	ret = ltc4282_read_alarm(st, LTC4282_FAULT_LOG,	mask, &alarm);
+	if (ret)
+		return ret;
+
+	*val = alarm;
+
+	return 0;
+}
+
+static int ltc4282_show_curr1_crit_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_OC_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_curr1_crit_fault_log,
+			 ltc4282_show_curr1_crit_fault_log, NULL, "%llu\n");
+
+static int ltc4282_show_in1_lcrit_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_UV_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_in1_lcrit_fault_log,
+			 ltc4282_show_in1_lcrit_fault_log, NULL, "%llu\n");
+
+static int ltc4282_show_in1_crit_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_OV_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_in1_crit_fault_log,
+			 ltc4282_show_in1_crit_fault_log, NULL, "%llu\n");
+
+static int ltc4282_show_fet_bad_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_FET_BAD_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_fet_bad_fault_log,
+			 ltc4282_show_fet_bad_fault_log, NULL, "%llu\n");
+
+static int ltc4282_show_fet_short_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_FET_SHORT_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_fet_short_fault_log,
+			 ltc4282_show_fet_short_fault_log, NULL, "%llu\n");
+
+static int ltc4282_show_power1_bad_fault_log(void *arg, u64 *val)
+{
+	return ltc4282_show_fault_log(arg, val, LTC4282_POWER_BAD_FAULT_MASK);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ltc4282_power1_bad_fault_log,
+			 ltc4282_show_power1_bad_fault_log, NULL, "%llu\n");
+
+static void ltc4282_debugfs_remove(void *dir)
+{
+	debugfs_remove_recursive(dir);
+}
+
+static void ltc4282_debugfs_init(struct ltc4282_state *st,
+				 struct i2c_client *i2c,
+				 const struct device *hwmon)
+{
+	const char *debugfs_name;
+	struct dentry *dentry;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_DEBUG_FS))
+		return;
+
+	debugfs_name = devm_kasprintf(&i2c->dev, GFP_KERNEL, "%s-%s",
+				      i2c->name, dev_name(hwmon));
+	if (!debugfs_name)
+		return;
+
+	dentry = debugfs_create_dir(debugfs_name, NULL);
+	if (IS_ERR(dentry))
+		return;
+
+	ret = devm_add_action_or_reset(&i2c->dev, ltc4282_debugfs_remove,
+				       dentry);
+	if (ret)
+		return;
+
+	debugfs_create_file_unsafe("power1_bad_fault_log", 0400, dentry, st,
+				   &ltc4282_power1_bad_fault_log);
+	debugfs_create_file_unsafe("fet_short_fault_log", 0400, dentry, st,
+				   &ltc4282_fet_short_fault_log);
+	debugfs_create_file_unsafe("fet_bad_fault_log", 0400, dentry, st,
+				   &ltc4282_fet_bad_fault_log);
+	debugfs_create_file_unsafe("in1_crit_fault_log", 0400, dentry, st,
+				   &ltc4282_in1_crit_fault_log);
+	debugfs_create_file_unsafe("in1_lcrit_fault_log", 0400, dentry, st,
+				   &ltc4282_in1_lcrit_fault_log);
+	debugfs_create_file_unsafe("curr1_crit_fault_log", 0400, dentry, st,
+				   &ltc4282_curr1_crit_fault_log);
+}
+
+static int ltc4282_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev, *hwmon;
+	struct ltc4282_state *st;
+	int ret;
+
+	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate memory\n");
+
+	st->map = devm_regmap_init_i2c(i2c, &ltc4282_regmap_config);
+	if (IS_ERR(st->map))
+		return dev_err_probe(dev, PTR_ERR(st->map),
+				     "failed regmap init\n");
+
+	/* Soft reset */
+	ret = regmap_set_bits(st->map, LTC4282_ADC_CTRL, LTC4282_RESET_MASK);
+	if (ret)
+		return ret;
+
+	/* Yes, it's big but it is as specified in the datasheet */
+	msleep(3200);
+
+	ret = ltc428_clks_setup(st, dev);
+	if (ret)
+		return ret;
+
+	ret = ltc4282_setup(st, dev);
+	if (ret)
+		return ret;
+
+	mutex_init(&st->lock);
+	hwmon = devm_hwmon_device_register_with_info(dev, "ltc4282", st,
+						     &ltc2947_chip_info,
+						     ltc4282_groups);
+	if (IS_ERR(hwmon))
+		return PTR_ERR(hwmon);
+
+	ltc4282_debugfs_init(st, i2c, hwmon);
+
+	return 0;
+}
+
+static const struct of_device_id ltc4282_of_match[] = {
+	{ .compatible = "adi,ltc4282" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltc4282_of_match);
+
+static struct i2c_driver ltc4282_driver = {
+	.driver = {
+		.name = "ltc4282",
+		.of_match_table = ltc4282_of_match,
+	},
+	.probe = ltc4282_probe,
+};
+module_i2c_driver(ltc4282_driver);
+
+MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
+MODULE_DESCRIPTION("LTC4282 I2C High Current Hot Swap Controller");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Guenter Roeck]

On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
> 
> The LTC4282 hot swap controller allows a board to be safely inserted and
> removed from a live backplane. Using one or more external N-channel pass
> transistors, board supply voltage and inrush current are ramped up at an
> adjustable rate. An I2C interface and onboard ADC allows for monitoring
> of board current, voltage, power, energy and fault status.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
> +

> +power1_good		Power considered good

I really don't like this attribute. Like the ones below it is non-standard
and invisible for standard applications. On top of that, I think it isn't
really related to "power" but to the output voltage. What does it actually
report that isn't included in the FET faults ?


> +fet_short_fault		FET short alarm
> +fet_bad_fault		FET bad alarm

Those attributes have little value since they are not standard attributes
and won't be seen by standard applications. On top of that, it is not clear
(not documented) what the attribute actually reports. I assume it is
associated with the output voltage, i.e., in0, but that is just an
assumption.

What do you think about introducing a standard inX_fault attribute ?
It would not be as specific as short/bad, but I think it would be more
useful and we could add it to the ABI.

> +fault_logs_reset	Clears all the Logged Faults

What exactly does that do that is user visible ?

Thanks,
Guenter

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sá]

On Mon, 2023-12-11 at 07:36 -0800, Guenter Roeck wrote:
> On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > The LTC4282 hot swap controller allows a board to be safely inserted and
> > removed from a live backplane. Using one or more external N-channel pass
> > transistors, board supply voltage and inrush current are ramped up at an
> > adjustable rate. An I2C interface and onboard ADC allows for monitoring
> > of board current, voltage, power, energy and fault status.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> > +
> 
> > +power1_good		Power considered good
> 
> I really don't like this attribute. Like the ones below it is non-standard
> and invisible for standard applications. On top of that, I think it isn't
> really related to "power" but to the output voltage. What does it actually
> report that isn't included in the FET faults ?
> 

This is detected with the FB pin and a voltage divider (from the output
voltage). Basically depending on the level of that pin, the chip indicate power
good or power bad. I was also very reluctant with this attribute (I mention it
in the v1 cover). This might not even indicate any misbehave. We also support
reporting this using the gpio1 pin (if we set it that way). So, I guess I can
just drop this one and add support for it if we ever have a real usecase where I
can actually justify having it :).

We already have the power_bad fault log in debugfs so I'm not sure if adding
this one there adds much value.

> 
> > +fet_short_fault		FET short alarm
> > +fet_bad_fault		FET bad alarm
> 
> Those attributes have little value since they are not standard attributes
> and won't be seen by standard applications. On top of that, it is not clear
> (not documented) what the attribute actually reports. I assume it is
> associated with the output voltage, i.e., in0, but that is just an
> assumption.
> 

fet_short - This is one is detected if the ADC measures a current sense voltage
> 0.25mv while the fet gate is off.

fet_bad - Is set by monitoring the voltage at the gate and the drain to source
voltage.

These ones might indicate real issues with the HW so I thought they could be
important...
 
> What do you think about introducing a standard inX_fault attribute ?
> It would not be as specific as short/bad, but I think it would be more
> useful and we could add it to the ABI.
> 

It would be better than nothing. And we do have fault logs for both these
failures so userspace could also use that to know exactly what was the issue. If
that's ok with you, I would then report this in inX_fault? Did you had in mind
putting this in in0 (vsource) or adding a new channel?

In my first draft I had another voltage channel (label: VFET) to report the
fet_bad condition. I was using the inX_crit or inX_lcrit but it felt bad so I
removed it...

> > +fault_logs_reset	Clears all the Logged Faults

> What exactly does that do that is user visible ?

Well, this one is because in some configurations the chip won't enable the
output load until you reset/clear the fault log keeping it from enabling the
output. This is the comment I have in the code:

"Fault log failures. These faults might be important in systems where auto-retry
is not enabled since they will cause the part to latch off until they are
cleared. Typically that happens when the system admin is close enough so he can
check what happened and manually clear the faults. Moreover, manually clearing
the faults might only matter when ON_FAULT_MASK in the CONTROL register is set
(which is the default) as in that case, a turn off signal from the ON pin won't
clear them."

In v1 I was allowing to clear fauls log individually and you recommended to have
an attribute to clear them all at once as that would simplify things. 

I just kept it in here because this might be important for the chip to work as
expected again so having it in debugfs might be weird.

Thanks!
- Nuno Sá

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Guenter Roeck]

On 12/12/23 06:28, Nuno Sá wrote:
> On Mon, 2023-12-11 at 07:36 -0800, Guenter Roeck wrote:
>> On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
>>> From: Nuno Sa <nuno.sa@analog.com>
>>>
>>> The LTC4282 hot swap controller allows a board to be safely inserted and
>>> removed from a live backplane. Using one or more external N-channel pass
>>> transistors, board supply voltage and inrush current are ramped up at an
>>> adjustable rate. An I2C interface and onboard ADC allows for monitoring
>>> of board current, voltage, power, energy and fault status.
>>>
>>> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
>>> ---
>>> +
>>
>>> +power1_good		Power considered good
>>
>> I really don't like this attribute. Like the ones below it is non-standard
>> and invisible for standard applications. On top of that, I think it isn't
>> really related to "power" but to the output voltage. What does it actually
>> report that isn't included in the FET faults ?
>>
> 
> This is detected with the FB pin and a voltage divider (from the output
> voltage). Basically depending on the level of that pin, the chip indicate power
> good or power bad. I was also very reluctant with this attribute (I mention it
> in the v1 cover). This might not even indicate any misbehave. We also support
> reporting this using the gpio1 pin (if we set it that way). So, I guess I can
> just drop this one and add support for it if we ever have a real usecase where I
> can actually justify having it :).
> 
> We already have the power_bad fault log in debugfs so I'm not sure if adding
> this one there adds much value.
> 
>>
>>> +fet_short_fault		FET short alarm
>>> +fet_bad_fault		FET bad alarm
>>
>> Those attributes have little value since they are not standard attributes
>> and won't be seen by standard applications. On top of that, it is not clear
>> (not documented) what the attribute actually reports. I assume it is
>> associated with the output voltage, i.e., in0, but that is just an
>> assumption.
>>
> 
> fet_short - This is one is detected if the ADC measures a current sense voltage
>> 0.25mv while the fet gate is off.
> 
> fet_bad - Is set by monitoring the voltage at the gate and the drain to source
> voltage.
> 
> These ones might indicate real issues with the HW so I thought they could be
> important...
>   
>> What do you think about introducing a standard inX_fault attribute ?
>> It would not be as specific as short/bad, but I think it would be more
>> useful and we could add it to the ABI.
>>
> 
> It would be better than nothing. And we do have fault logs for both these
> failures so userspace could also use that to know exactly what was the issue. If
> that's ok with you, I would then report this in inX_fault? Did you had in mind
> putting this in in0 (vsource) or adding a new channel?
> 
> In my first draft I had another voltage channel (label: VFET) to report the
> fet_bad condition. I was using the inX_crit or inX_lcrit but it felt bad so I
> removed it...
> 

This isn't an extra voltage channel. It should be associated with the output
voltage because that is what is affected, and that would be in0.

>>> +fault_logs_reset	Clears all the Logged Faults
> 
>> What exactly does that do that is user visible ?
> 
> Well, this one is because in some configurations the chip won't enable the
> output load until you reset/clear the fault log keeping it from enabling the
> output. This is the comment I have in the code:
> 
> "Fault log failures. These faults might be important in systems where auto-retry
> is not enabled since they will cause the part to latch off until they are
> cleared. Typically that happens when the system admin is close enough so he can
> check what happened and manually clear the faults. Moreover, manually clearing
> the faults might only matter when ON_FAULT_MASK in the CONTROL register is set
> (which is the default) as in that case, a turn off signal from the ON pin won't
> clear them."
> 
> In v1 I was allowing to clear fauls log individually and you recommended to have
> an attribute to clear them all at once as that would simplify things.
> 
> I just kept it in here because this might be important for the chip to work as
> expected again so having it in debugfs might be weird.
> 

How about using a write to in0_fault to clear the log ?

Thanks,
Guenter

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sá]

On Tue, 2023-12-12 at 07:14 -0800, Guenter Roeck wrote:
> On 12/12/23 06:28, Nuno Sá wrote:
> > On Mon, 2023-12-11 at 07:36 -0800, Guenter Roeck wrote:
> > > On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> > > > From: Nuno Sa <nuno.sa@analog.com>
> > > > 
> > > > The LTC4282 hot swap controller allows a board to be safely inserted and
> > > > removed from a live backplane. Using one or more external N-channel pass
> > > > transistors, board supply voltage and inrush current are ramped up at an
> > > > adjustable rate. An I2C interface and onboard ADC allows for monitoring
> > > > of board current, voltage, power, energy and fault status.
> > > > 
> > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > > ---
> > > > +
> > > 
> > > > +power1_good		Power considered good
> > > 
> > > I really don't like this attribute. Like the ones below it is non-standard
> > > and invisible for standard applications. On top of that, I think it isn't
> > > really related to "power" but to the output voltage. What does it actually
> > > report that isn't included in the FET faults ?
> > > 
> > 
> > This is detected with the FB pin and a voltage divider (from the output
> > voltage). Basically depending on the level of that pin, the chip indicate
> > power
> > good or power bad. I was also very reluctant with this attribute (I mention
> > it
> > in the v1 cover). This might not even indicate any misbehave. We also
> > support
> > reporting this using the gpio1 pin (if we set it that way). So, I guess I
> > can
> > just drop this one and add support for it if we ever have a real usecase
> > where I
> > can actually justify having it :).
> > 
> > We already have the power_bad fault log in debugfs so I'm not sure if adding
> > this one there adds much value.
> > 
> > > 
> > > > +fet_short_fault		FET short alarm
> > > > +fet_bad_fault		FET bad alarm
> > > 
> > > Those attributes have little value since they are not standard attributes
> > > and won't be seen by standard applications. On top of that, it is not
> > > clear
> > > (not documented) what the attribute actually reports. I assume it is
> > > associated with the output voltage, i.e., in0, but that is just an
> > > assumption.
> > > 
> > 
> > fet_short - This is one is detected if the ADC measures a current sense
> > voltage
> > > 0.25mv while the fet gate is off.
> > 
> > fet_bad - Is set by monitoring the voltage at the gate and the drain to
> > source
> > voltage.
> > 
> > These ones might indicate real issues with the HW so I thought they could be
> > important...
> >   
> > > What do you think about introducing a standard inX_fault attribute ?
> > > It would not be as specific as short/bad, but I think it would be more
> > > useful and we could add it to the ABI.
> > > 
> > 
> > It would be better than nothing. And we do have fault logs for both these
> > failures so userspace could also use that to know exactly what was the
> > issue. If
> > that's ok with you, I would then report this in inX_fault? Did you had in
> > mind
> > putting this in in0 (vsource) or adding a new channel?
> > 
> > In my first draft I had another voltage channel (label: VFET) to report the
> > fet_bad condition. I was using the inX_crit or inX_lcrit but it felt bad so
> > I
> > removed it...
> > 
> 
> This isn't an extra voltage channel. It should be associated with the output
> voltage because that is what is affected, and that would be in0.
> 

Noted... will go with that and add an introductory patch for having inX_fault in
hwmon.

> > > > +fault_logs_reset	Clears all the Logged Faults
> > 
> > > What exactly does that do that is user visible ?
> > 
> > Well, this one is because in some configurations the chip won't enable the
> > output load until you reset/clear the fault log keeping it from enabling the
> > output. This is the comment I have in the code:
> > 
> > "Fault log failures. These faults might be important in systems where auto-
> > retry
> > is not enabled since they will cause the part to latch off until they are
> > cleared. Typically that happens when the system admin is close enough so he
> > can
> > check what happened and manually clear the faults. Moreover, manually
> > clearing
> > the faults might only matter when ON_FAULT_MASK in the CONTROL register is
> > set
> > (which is the default) as in that case, a turn off signal from the ON pin
> > won't
> > clear them."
> > 
> > In v1 I was allowing to clear fauls log individually and you recommended to
> > have
> > an attribute to clear them all at once as that would simplify things.
> > 
> > I just kept it in here because this might be important for the chip to work
> > as
> > expected again so having it in debugfs might be weird.
> > 
> 
> How about using a write to in0_fault to clear the log ?
> 

But that would be just related with the output voltage. You can also have
failures on VDD (over and undervolatge fault - I'm using in1_crit and in1_lcrit)
and if you have the auto retry bit disabled, then clearing fault logs might be
important. This attribute is nice because makes it clear what will be done.
OTOH, I do understand your worries with non standard ABI...

I'm not sure how usual is for fault logs to be around that would justify for a
global standard attribute.

- Nuno Sá

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Guenter Roeck]

On 12/12/23 07:30, Nuno Sá wrote:
> On Tue, 2023-12-12 at 07:14 -0800, Guenter Roeck wrote:
>> On 12/12/23 06:28, Nuno Sá wrote:
>>> On Mon, 2023-12-11 at 07:36 -0800, Guenter Roeck wrote:
>>>> On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
>>>>> From: Nuno Sa <nuno.sa@analog.com>
>>>>>
>>>>> The LTC4282 hot swap controller allows a board to be safely inserted and
>>>>> removed from a live backplane. Using one or more external N-channel pass
>>>>> transistors, board supply voltage and inrush current are ramped up at an
>>>>> adjustable rate. An I2C interface and onboard ADC allows for monitoring
>>>>> of board current, voltage, power, energy and fault status.
>>>>>
>>>>> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
>>>>> ---
>>>>> +
>>>>
>>>>> +power1_good		Power considered good
>>>>
>>>> I really don't like this attribute. Like the ones below it is non-standard
>>>> and invisible for standard applications. On top of that, I think it isn't
>>>> really related to "power" but to the output voltage. What does it actually
>>>> report that isn't included in the FET faults ?
>>>>
>>>
>>> This is detected with the FB pin and a voltage divider (from the output
>>> voltage). Basically depending on the level of that pin, the chip indicate
>>> power
>>> good or power bad. I was also very reluctant with this attribute (I mention
>>> it
>>> in the v1 cover). This might not even indicate any misbehave. We also
>>> support
>>> reporting this using the gpio1 pin (if we set it that way). So, I guess I
>>> can
>>> just drop this one and add support for it if we ever have a real usecase
>>> where I
>>> can actually justify having it :).
>>>
>>> We already have the power_bad fault log in debugfs so I'm not sure if adding
>>> this one there adds much value.
>>>
>>>>
>>>>> +fet_short_fault		FET short alarm
>>>>> +fet_bad_fault		FET bad alarm
>>>>
>>>> Those attributes have little value since they are not standard attributes
>>>> and won't be seen by standard applications. On top of that, it is not
>>>> clear
>>>> (not documented) what the attribute actually reports. I assume it is
>>>> associated with the output voltage, i.e., in0, but that is just an
>>>> assumption.
>>>>
>>>
>>> fet_short - This is one is detected if the ADC measures a current sense
>>> voltage
>>>> 0.25mv while the fet gate is off.
>>>
>>> fet_bad - Is set by monitoring the voltage at the gate and the drain to
>>> source
>>> voltage.
>>>
>>> These ones might indicate real issues with the HW so I thought they could be
>>> important...
>>>    
>>>> What do you think about introducing a standard inX_fault attribute ?
>>>> It would not be as specific as short/bad, but I think it would be more
>>>> useful and we could add it to the ABI.
>>>>
>>>
>>> It would be better than nothing. And we do have fault logs for both these
>>> failures so userspace could also use that to know exactly what was the
>>> issue. If
>>> that's ok with you, I would then report this in inX_fault? Did you had in
>>> mind
>>> putting this in in0 (vsource) or adding a new channel?
>>>
>>> In my first draft I had another voltage channel (label: VFET) to report the
>>> fet_bad condition. I was using the inX_crit or inX_lcrit but it felt bad so
>>> I
>>> removed it...
>>>
>>
>> This isn't an extra voltage channel. It should be associated with the output
>> voltage because that is what is affected, and that would be in0.
>>
> 
> Noted... will go with that and add an introductory patch for having inX_fault in
> hwmon.
> 
>>>>> +fault_logs_reset	Clears all the Logged Faults
>>>
>>>> What exactly does that do that is user visible ?
>>>
>>> Well, this one is because in some configurations the chip won't enable the
>>> output load until you reset/clear the fault log keeping it from enabling the
>>> output. This is the comment I have in the code:
>>>
>>> "Fault log failures. These faults might be important in systems where auto-
>>> retry
>>> is not enabled since they will cause the part to latch off until they are
>>> cleared. Typically that happens when the system admin is close enough so he
>>> can
>>> check what happened and manually clear the faults. Moreover, manually
>>> clearing
>>> the faults might only matter when ON_FAULT_MASK in the CONTROL register is
>>> set
>>> (which is the default) as in that case, a turn off signal from the ON pin
>>> won't
>>> clear them."
>>>
>>> In v1 I was allowing to clear fauls log individually and you recommended to
>>> have
>>> an attribute to clear them all at once as that would simplify things.
>>>
>>> I just kept it in here because this might be important for the chip to work
>>> as
>>> expected again so having it in debugfs might be weird.
>>>
>>
>> How about using a write to in0_fault to clear the log ?
>>
> 
> But that would be just related with the output voltage. You can also have
> failures on VDD (over and undervolatge fault - I'm using in1_crit and in1_lcrit)
> and if you have the auto retry bit disabled, then clearing fault logs might be
> important. This attribute is nice because makes it clear what will be done.
> OTOH, I do understand your worries with non standard ABI...
> 

Depends on the meaning of "fault". PMBus, for example, uses the "fault" terminology
to indicate that critical limits have been exceeded. That isn't a "fault" in the
terminology used by the hwmon subsystem. Violating a voltage/current/power range
isn't a fault. A shorted FET or temperature sensor, however, is a fault.

In that sense, if the output is turned off because a voltage / current / power
limit was violated, a write into the respective alarm register would be appropriate
to reset it if the condition doesn't auto-reset. It really all depends on the context.

> I'm not sure how usual is for fault logs to be around that would justify for a
> global standard attribute.
> 

We could also use in[01]_reset_history. While that is originally intended to reset
min/max voltage history, I think it makes sense to extend the meaning to include
fault history (even more so if that history includes over/undervoltage events).
Plus, there are other _reset_history attributes which could be used to reset
power/current/temperature history separately if that is supported by the chip.

Thanks,
Guenter

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sá]

On Tue, 2023-12-12 at 09:59 -0800, Guenter Roeck wrote:
> On 12/12/23 07:30, Nuno Sá wrote:
> > On Tue, 2023-12-12 at 07:14 -0800, Guenter Roeck wrote:
> > > On 12/12/23 06:28, Nuno Sá wrote:
> > > > On Mon, 2023-12-11 at 07:36 -0800, Guenter Roeck wrote:
> > > > > On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> > > > > > From: Nuno Sa <nuno.sa@analog.com>
> > > > > > 
> > > > > > The LTC4282 hot swap controller allows a board to be safely inserted and
> > > > > > removed from a live backplane. Using one or more external N-channel pass
> > > > > > transistors, board supply voltage and inrush current are ramped up at an
> > > > > > adjustable rate. An I2C interface and onboard ADC allows for monitoring
> > > > > > of board current, voltage, power, energy and fault status.
> > > > > > 
> > > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > > > > ---
> > > > > > +
> > > > > 
> > > > > > +power1_good            Power considered good
> > > > > 
> > > > > I really don't like this attribute. Like the ones below it is non-standard
> > > > > and invisible for standard applications. On top of that, I think it isn't
> > > > > really related to "power" but to the output voltage. What does it actually
> > > > > report that isn't included in the FET faults ?
> > > > > 
> > > > 
> > > > This is detected with the FB pin and a voltage divider (from the output
> > > > voltage). Basically depending on the level of that pin, the chip indicate
> > > > power
> > > > good or power bad. I was also very reluctant with this attribute (I mention
> > > > it
> > > > in the v1 cover). This might not even indicate any misbehave. We also
> > > > support
> > > > reporting this using the gpio1 pin (if we set it that way). So, I guess I
> > > > can
> > > > just drop this one and add support for it if we ever have a real usecase
> > > > where I
> > > > can actually justify having it :).
> > > > 
> > > > We already have the power_bad fault log in debugfs so I'm not sure if adding
> > > > this one there adds much value.
> > > > 
> > > > > 
> > > > > > +fet_short_fault                FET short alarm
> > > > > > +fet_bad_fault          FET bad alarm
> > > > > 
> > > > > Those attributes have little value since they are not standard attributes
> > > > > and won't be seen by standard applications. On top of that, it is not
> > > > > clear
> > > > > (not documented) what the attribute actually reports. I assume it is
> > > > > associated with the output voltage, i.e., in0, but that is just an
> > > > > assumption.
> > > > > 
> > > > 
> > > > fet_short - This is one is detected if the ADC measures a current sense
> > > > voltage
> > > > > 0.25mv while the fet gate is off.
> > > > 
> > > > fet_bad - Is set by monitoring the voltage at the gate and the drain to
> > > > source
> > > > voltage.
> > > > 
> > > > These ones might indicate real issues with the HW so I thought they could be
> > > > important...
> > > >    
> > > > > What do you think about introducing a standard inX_fault attribute ?
> > > > > It would not be as specific as short/bad, but I think it would be more
> > > > > useful and we could add it to the ABI.
> > > > > 
> > > > 
> > > > It would be better than nothing. And we do have fault logs for both these
> > > > failures so userspace could also use that to know exactly what was the
> > > > issue. If
> > > > that's ok with you, I would then report this in inX_fault? Did you had in
> > > > mind
> > > > putting this in in0 (vsource) or adding a new channel?
> > > > 
> > > > In my first draft I had another voltage channel (label: VFET) to report the
> > > > fet_bad condition. I was using the inX_crit or inX_lcrit but it felt bad so
> > > > I
> > > > removed it...
> > > > 
> > > 
> > > This isn't an extra voltage channel. It should be associated with the output
> > > voltage because that is what is affected, and that would be in0.
> > > 
> > 
> > Noted... will go with that and add an introductory patch for having inX_fault in
> > hwmon.
> > 
> > > > > > +fault_logs_reset       Clears all the Logged Faults
> > > > 
> > > > > What exactly does that do that is user visible ?
> > > > 
> > > > Well, this one is because in some configurations the chip won't enable the
> > > > output load until you reset/clear the fault log keeping it from enabling the
> > > > output. This is the comment I have in the code:
> > > > 
> > > > "Fault log failures. These faults might be important in systems where auto-
> > > > retry
> > > > is not enabled since they will cause the part to latch off until they are
> > > > cleared. Typically that happens when the system admin is close enough so he
> > > > can
> > > > check what happened and manually clear the faults. Moreover, manually
> > > > clearing
> > > > the faults might only matter when ON_FAULT_MASK in the CONTROL register is
> > > > set
> > > > (which is the default) as in that case, a turn off signal from the ON pin
> > > > won't
> > > > clear them."
> > > > 
> > > > In v1 I was allowing to clear fauls log individually and you recommended to
> > > > have
> > > > an attribute to clear them all at once as that would simplify things.
> > > > 
> > > > I just kept it in here because this might be important for the chip to work
> > > > as
> > > > expected again so having it in debugfs might be weird.
> > > > 
> > > 
> > > How about using a write to in0_fault to clear the log ?
> > > 
> > 
> > But that would be just related with the output voltage. You can also have
> > failures on VDD (over and undervolatge fault - I'm using in1_crit and in1_lcrit)
> > and if you have the auto retry bit disabled, then clearing fault logs might be
> > important. This attribute is nice because makes it clear what will be done.
> > OTOH, I do understand your worries with non standard ABI...
> > 
> 
> Depends on the meaning of "fault". PMBus, for example, uses the "fault" terminology
> to indicate that critical limits have been exceeded. That isn't a "fault" in the
> terminology used by the hwmon subsystem. Violating a voltage/current/power range
> isn't a fault. A shorted FET or temperature sensor, however, is a fault.
> 
> In that sense, if the output is turned off because a voltage / current / power
> limit was violated, a write into the respective alarm register would be appropriate
> to reset it if the condition doesn't auto-reset. It really all depends on the
> context.
> 

Yeah, maybe I'm also failing in giving you all the "context". The device has kind of
three registers for failures/status/logs:

1. The ADC_ALERT_LOG. This is the typical one for violated min/max limits. They don't
auto-reset so I'm already doing that in the driver (after reading the alarm bit).

2. The STATUS register. In this one, we have the fet bad/short failures and critical
limits for VDD (under/over voltage detection). We also have overcurrent detection and
the power1_good stuff we already talked about. This is one auto clears and it's RO.
So we kind of have a mix of faults and critical limits in here.

3. The FAULT_LOG register. This one is more about logging and is very similar (if not
identical) in the reported bits with the STATUS register. The differences is that
this one does not auto-clears as it's about logging so we want to know about historic
failures and in some configurations (as mentioned before) we do need to clear the
fault log bit so the chip will be able to enable again the output voltage. And
regarding that we have these main bits:

 * Under voltage on VDD
 * Over voltage on VDD
 * Over current on the sense resistor
 * FET bad
 * FET short 
 * Power bad

I can say that the under/over voltage and current will prevent the output to be
enabled. I could force that state in my test environment and could see that
happening. For the FET faults I'm not sure as I cannot force those faults. The power
stuff won't affect the output.

> > I'm not sure how usual is for fault logs to be around that would justify for a
> > global standard attribute.
> > 
> 
> We could also use in[01]_reset_history. While that is originally intended to reset
> min/max voltage history, I think it makes sense to extend the meaning to include
> fault history (even more so if that history includes over/undervoltage events).
> Plus, there are other _reset_history attributes which could be used to reset
> power/current/temperature history separately if that is supported by the chip.
> 

Well, I'm already supporting _reset_history in the voltage/power/current channels so
I can easily extend that for clearing fault history if that is fine with you. I just
need to document it because it's a bit of an "hidden" thing. The question would also
be, should I just document this for this chip docs or in the general hwmon docs?

- Nuno Sá

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Guenter Roeck]

On 12/13/23 02:06, Nuno Sá wrote:
[ ... ]
>>
>> We could also use in[01]_reset_history. While that is originally intended to reset
>> min/max voltage history, I think it makes sense to extend the meaning to include
>> fault history (even more so if that history includes over/undervoltage events).
>> Plus, there are other _reset_history attributes which could be used to reset
>> power/current/temperature history separately if that is supported by the chip.
>>
> 
> Well, I'm already supporting _reset_history in the voltage/power/current channels so
> I can easily extend that for clearing fault history if that is fine with you. I just
> need to document it because it's a bit of an "hidden" thing. The question would also
> be, should I just document this for this chip docs or in the general hwmon docs?
> 

I'd say chip specific for now. We can document it in the general documentation
if/when we get more chips with the same characteristics.

Thanks,
Guenter

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sá]

On Wed, 2023-12-13 at 06:35 -0800, Guenter Roeck wrote:
> On 12/13/23 02:06, Nuno Sá wrote:
> [ ... ]
> > > 
> > > We could also use in[01]_reset_history. While that is originally intended to
> > > reset
> > > min/max voltage history, I think it makes sense to extend the meaning to
> > > include
> > > fault history (even more so if that history includes over/undervoltage events).
> > > Plus, there are other _reset_history attributes which could be used to reset
> > > power/current/temperature history separately if that is supported by the chip.
> > > 
> > 
> > Well, I'm already supporting _reset_history in the voltage/power/current channels
> > so
> > I can easily extend that for clearing fault history if that is fine with you. I
> > just
> > need to document it because it's a bit of an "hidden" thing. The question would
> > also
> > be, should I just document this for this chip docs or in the general hwmon docs?
> > 
> 
> I'd say chip specific for now. We can document it in the general documentation
> if/when we get more chips with the same characteristics.
> 

Agreed... Alright, I think I have now all I need to work on v4.

Thanks!
- Nuno Sá

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Guenter Roeck]

On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
> 
> The LTC4282 hot swap controller allows a board to be safely inserted and
> removed from a live backplane. Using one or more external N-channel pass
> transistors, board supply voltage and inrush current are ramped up at an
> adjustable rate. An I2C interface and onboard ADC allows for monitoring
> of board current, voltage, power, energy and fault status.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
[ ... ]

> +/* power1_fault */
> +static SENSOR_DEVICE_ATTR_RO(power1_good, ltc4282_status,
> +			     LTC4282_POWER_GOOD_MASK);
> +/* FET faults */
> +static SENSOR_DEVICE_ATTR_RO(fet_short_fault, ltc4282_status,
> +			     LTC4282_FET_SHORT_MASK);
> +static SENSOR_DEVICE_ATTR_RO(fet_bad_fault, ltc4282_status,
> +			     LTC4282_FET_BAD_STATUS_MASK);
> +/*
> + * Fault log failures. These faults might be important in systems where
> + * auto-retry is not enabled since they will cause the part to latch off until
> + * they are cleared. Typically that happens when the system admin is close
> + * enough so he can check what happened and manually clear the faults. Hence, we
> + * provide an attribute to clear all fauls at once while still capable of
> + * checking individual faults in debugfs.
> + */
> +static SENSOR_DEVICE_ATTR_WO(fault_logs_reset, ltc4282_clear_faults, 0);
> +
> +static struct attribute *ltc4282_attrs[] = {
> +	&sensor_dev_attr_energy1_input.dev_attr.attr,
> +	&sensor_dev_attr_power1_good.dev_attr.attr,
> +	&sensor_dev_attr_fet_bad_fault.dev_attr.attr,
> +	&sensor_dev_attr_fet_short_fault.dev_attr.attr,
> +	&sensor_dev_attr_fault_logs_reset.dev_attr.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(ltc4282);
> +

Ah, now I see what those are for. Please move all but energy1_input
to debugfs, including clearing the faults.

Thanks,
Guenter

Re: [PATCH v3 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Nuno Sá]

On Mon, 2023-12-11 at 07:41 -0800, Guenter Roeck wrote:
> On Tue, Dec 05, 2023 at 04:22:56PM +0100, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > The LTC4282 hot swap controller allows a board to be safely inserted and
> > removed from a live backplane. Using one or more external N-channel pass
> > transistors, board supply voltage and inrush current are ramped up at an
> > adjustable rate. An I2C interface and onboard ADC allows for monitoring
> > of board current, voltage, power, energy and fault status.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> [ ... ]
> 
> > +/* power1_fault */
> > +static SENSOR_DEVICE_ATTR_RO(power1_good, ltc4282_status,
> > +			     LTC4282_POWER_GOOD_MASK);
> > +/* FET faults */
> > +static SENSOR_DEVICE_ATTR_RO(fet_short_fault, ltc4282_status,
> > +			     LTC4282_FET_SHORT_MASK);
> > +static SENSOR_DEVICE_ATTR_RO(fet_bad_fault, ltc4282_status,
> > +			     LTC4282_FET_BAD_STATUS_MASK);
> > +/*
> > + * Fault log failures. These faults might be important in systems where
> > + * auto-retry is not enabled since they will cause the part to latch off
> > until
> > + * they are cleared. Typically that happens when the system admin is close
> > + * enough so he can check what happened and manually clear the faults.
> > Hence, we
> > + * provide an attribute to clear all fauls at once while still capable of
> > + * checking individual faults in debugfs.
> > + */
> > +static SENSOR_DEVICE_ATTR_WO(fault_logs_reset, ltc4282_clear_faults, 0);
> > +
> > +static struct attribute *ltc4282_attrs[] = {
> > +	&sensor_dev_attr_energy1_input.dev_attr.attr,
> > +	&sensor_dev_attr_power1_good.dev_attr.attr,
> > +	&sensor_dev_attr_fet_bad_fault.dev_attr.attr,
> > +	&sensor_dev_attr_fet_short_fault.dev_attr.attr,
> > +	&sensor_dev_attr_fault_logs_reset.dev_attr.attr,
> > +	NULL
> > +};
> > +ATTRIBUTE_GROUPS(ltc4282);
> > +
> 
> Ah, now I see what those are for. Please move all but energy1_input
> to debugfs, including clearing the faults.
> 

I'll reply in the other thread. Just wanted to make it clear (not totally sure
it is) that all of those attributes are status stuff and not fault logs. All of
the fault logs where moved to debugfs and I kept this one in here. If this was
already clear to you, sorry for the noise.

- Nuno Sá
> 
>