[PATCH v3 0/2] LTC4286 and LTC4287 driver support

[PATCH v3 0/2] LTC4286 and LTC4287 driver support

[Delphine CC Chiu]

v3 - Add LTC4286 and LTC4287 binding document
   - Add LTC4286 and LTC4287 driver

Delphine CC Chiu (2):
  dt-bindings: hwmon: Add lltc ltc4286 driver bindings
  hwmon: pmbus: Add ltc4286 driver

 .../bindings/hwmon/lltc,ltc4286.yaml          |  52 +++++
 Documentation/hwmon/ltc4286.rst               |  95 ++++++++++
 MAINTAINERS                                   |  10 +
 drivers/hwmon/pmbus/Kconfig                   |   9 +
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/ltc4286.c                 | 178 ++++++++++++++++++
 6 files changed, 345 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
 create mode 100644 Documentation/hwmon/ltc4286.rst
 create mode 100644 drivers/hwmon/pmbus/ltc4286.c

-- 
2.25.1

[PATCH v3 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings

[Delphine CC Chiu]

Add a device tree bindings for ltc4286 device.

Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>

Changelog:
  v3 - Revise adi,vrange-select-25p6 to adi,vrange-low-enable
  v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
     - Add type for adi,vrange-select-25p6
     - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
---
 .../bindings/hwmon/lltc,ltc4286.yaml          | 52 +++++++++++++++++++
 MAINTAINERS                                   | 10 ++++
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
new file mode 100644
index 000000000000..4695bca77c05
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LTC4286 power monitors
+
+maintainers:
+  - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
+
+properties:
+  compatible:
+    enum:
+      - lltc,ltc4286
+      - lltc,ltc4287
+
+  reg:
+    maxItems: 1
+
+  adi,vrange-low-enable:
+    description:
+      This property is a bool parameter to represent the
+      voltage range is 25.6 volts or 102.4 volts for
+      this chip.
+      The default is 102.4 volts.
+    type: boolean
+
+  shunt-resistor-micro-ohms:
+    description:
+      Resistor value in micro-Ohm
+
+required:
+  - compatible
+  - reg
+  - shunt-resistor-micro-ohms
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-sensor@40 {
+            compatible = "lltc,ltc4286";
+            reg = <0x40>;
+            adi,vrange-low-enable;
+            shunt-resistor-micro-ohms = <300>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 55d9d860f1a7..c60e29dc04ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12489,6 +12489,16 @@ S:	Maintained
 F:	Documentation/hwmon/ltc4261.rst
 F:	drivers/hwmon/ltc4261.c
 
+LTC4286 HARDWARE MONITOR DRIVER
+M:	Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
+L:	linux-i2c@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
+F:	Documentation/devicetree/bindings/hwmon/ltc4286.rst
+F:	drivers/hwmon/pmbus/Kconfig
+F:	drivers/hwmon/pmbus/Makefile
+F:	drivers/hwmon/pmbus/ltc4286.c
+
 LTC4306 I2C MULTIPLEXER DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 L:	linux-i2c@vger.kernel.org
-- 
2.25.1

[PATCH v3 2/2] hwmon: pmbus: Add ltc4286 driver

[Delphine CC Chiu]

Add a driver to support ltc4286 chip

Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>

Changelog:
  v3 - Use dev_err_probe() instead of dev_err()
     - The VRANGE_SELECT bit only be written if it actually changed
     - Avoid the info pointer being overwritten
     - Check the MBR value range to avoid overflow
     - Revise ltc4286.rst to corrcet description
  v2 - Revise Linear Technologies LTC4286 to
       Analog Devices LTC4286 in Kconfig
     - Add more description for this driver in Kconfig
     - Add some comments for MBR setting in ltc4286.c
     - Add ltc4286.rst
---
 Documentation/hwmon/ltc4286.rst |  95 +++++++++++++++++
 drivers/hwmon/pmbus/Kconfig     |   9 ++
 drivers/hwmon/pmbus/Makefile    |   1 +
 drivers/hwmon/pmbus/ltc4286.c   | 178 ++++++++++++++++++++++++++++++++
 4 files changed, 283 insertions(+)
 create mode 100644 Documentation/hwmon/ltc4286.rst
 create mode 100644 drivers/hwmon/pmbus/ltc4286.c

diff --git a/Documentation/hwmon/ltc4286.rst b/Documentation/hwmon/ltc4286.rst
new file mode 100644
index 000000000000..2cd149676d86
--- /dev/null
+++ b/Documentation/hwmon/ltc4286.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver ltc4286
+=====================
+
+Supported chips:
+
+  * Analog Devices LTC4286
+
+    Prefix: 'ltc4286'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4286.pdf
+
+  * Analog Devices LTC4287
+
+    Prefix: 'ltc4287'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4287.pdf
+
+Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
+
+
+Description
+-----------
+
+This driver supports hardware monitoring for Analog Devices LTC4286
+and LTC4287 Hot-Swap Controller and Digital Power Monitors.
+
+LTC4286 and LTC4287 are hot-swap controllers that allow a circuit board
+to be removed from or inserted into a live backplane. They also feature
+current and voltage readback via an integrated 12 bit analog-to-digital
+converter (ADC), accessed using a PMBus interface.
+
+The driver is a client driver to the core PMBus driver. Please see
+Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
+
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate the
+devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
+details.
+
+The shunt value in micro-ohms can be set via device tree at compile-time. Please
+refer to the Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings
+if the device tree is used.
+
+
+Platform data support
+---------------------
+
+The driver supports standard PMBus driver platform data. Please see
+Documentation/hwmon/pmbus.rst for details.
+
+
+Sysfs entries
+-------------
+
+The following attributes are supported. Limits are read-write, history reset
+attributes are write-only, all other attributes are read-only.
+
+======================= =======================================================
+in1_label		"vin"
+in1_input		Measured voltage.
+in1_alarm		Input voltage alarm.
+in1_min 		Minimum input voltage.
+in1_max 		Maximum input voltage.
+
+in2_label		"vout1"
+in2_input		Measured voltage.
+in2_alarm		Output voltage alarm.
+in2_min 		Minimum output voltage.
+in2_max 		Maximum output voltage.
+
+curr1_label		"iout1"
+curr1_input		Measured current.
+curr1_alarm		Output current alarm.
+curr1_max		Maximum current.
+
+power1_label		"pin"
+power1_input		Input power.
+power1_alarm		Input power alarm.
+power1_max		Maximum poewr.
+
+temp1_input		Chip temperature.
+temp1_min		Minimum chip temperature.
+temp1_max		Maximum chip temperature.
+temp1_crit		Critical chip temperature.
+temp1_alarm		Chip temperature alarm.
+======================= =======================================================
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index b4e93bd5835e..f2b53e8abc3c 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -226,6 +226,15 @@ config SENSORS_LTC3815
 
 	  This driver can also be built as a module. If so, the module will
 	  be called ltc3815.
+config SENSORS_LTC4286
+	bool "Analog Devices LTC4286"
+	help
+	  LTC4286 is an integrated solution for hot swap applications that
+	  allows a board to be safely inserted and removed from a live
+	  backplane.
+	  This chip could be used to monitor voltage, current, ...etc.
+	  If you say yes here you get hardware monitoring support for Analog
+	  Devices LTC4286.
 
 config SENSORS_MAX15301
 	tristate "Maxim MAX15301"
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 84ee960a6c2d..94e28f6d6a61 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066)	+= lm25066.o
 obj-$(CONFIG_SENSORS_LT7182S)	+= lt7182s.o
 obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
 obj-$(CONFIG_SENSORS_LTC3815)	+= ltc3815.o
+obj-$(CONFIG_SENSORS_LTC4286)	+= ltc4286.o
 obj-$(CONFIG_SENSORS_MAX15301)	+= max15301.o
 obj-$(CONFIG_SENSORS_MAX16064)	+= max16064.o
 obj-$(CONFIG_SENSORS_MAX16601)	+= max16601.o
diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c
new file mode 100644
index 000000000000..042d3af99489
--- /dev/null
+++ b/drivers/hwmon/pmbus/ltc4286.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pmbus.h>
+#include "pmbus.h"
+
+/* LTC4286 register */
+#define LTC4286_MFR_CONFIG1	0xF2
+
+/* LTC4286 configuration */
+#define VRANGE_SELECT_BIT	BIT(1)
+
+#define LTC4286_MFR_ID_SIZE	3
+#define VRANGE_25P6		0
+
+enum chips { ltc4286, ltc4287 };
+
+/*
+ * Initialize the MBR as default settings which is referred to LTC4286 datasheet
+ * (March 22, 2022 version) table 3 page 16
+ */
+static struct pmbus_driver_info ltc4286_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_VOLTAGE_OUT] = direct,
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_POWER] = direct,
+	.format[PSC_TEMPERATURE] = direct,
+	.m[PSC_VOLTAGE_IN] = 32,
+	.b[PSC_VOLTAGE_IN] = 0,
+	.R[PSC_VOLTAGE_IN] = 1,
+	.m[PSC_VOLTAGE_OUT] = 32,
+	.b[PSC_VOLTAGE_OUT] = 0,
+	.R[PSC_VOLTAGE_OUT] = 1,
+	.m[PSC_CURRENT_OUT] = 1024,
+	.b[PSC_CURRENT_OUT] = 0,
+	/*
+	 * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit.
+	 * However, the rsense value that user input is mirco ohm.
+	 * Thus, the MBR setting which involves rsense should be shifted by 6 digits.
+	 */
+	.R[PSC_CURRENT_OUT] = 3 - 6,
+	.m[PSC_POWER] = 1,
+	.b[PSC_POWER] = 0,
+	/*
+	 * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit.
+	 * However, the rsense value that user input is mirco ohm.
+	 * Thus, the MBR setting which involves rsense should be shifted by 6 digits.
+	 */
+	.R[PSC_POWER] = 4 - 6,
+	.m[PSC_TEMPERATURE] = 1,
+	.b[PSC_TEMPERATURE] = 273,
+	.R[PSC_TEMPERATURE] = 0,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
+		   PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP,
+};
+
+static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", ltc4286 },
+						   { "ltc4287", ltc4287 },
+						   {} };
+MODULE_DEVICE_TABLE(i2c, ltc4286_id);
+
+static int ltc4286_probe(struct i2c_client *client)
+{
+	int ret;
+	const struct i2c_device_id *mid;
+	u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
+	struct pmbus_driver_info *info;
+	u32 rsense;
+
+	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer);
+	if (ret < 0) {
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read manufacturer id\n");
+	}
+
+	/*
+	 * Refer to ltc4286 datasheet page 20
+	 * the manufacturer id is LTC
+	 */
+	if (ret != LTC4286_MFR_ID_SIZE ||
+	    strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) {
+		return dev_err_probe(&client->dev, ret,
+				     "Manufacturer id mismatch\n");
+	}
+
+	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
+	if (ret < 0) {
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read manufacturer model\n");
+	}
+
+	for (mid = ltc4286_id; mid->name[0]; mid++) {
+		if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
+			break;
+	}
+	if (!mid->name[0])
+		return dev_err_probe(&client->dev, -ENODEV,
+				     "Unsupported device\n");
+
+	ret = of_property_read_u32(client->dev.of_node,
+				   "shunt-resistor-micro-ohms", &rsense);
+	if (ret < 0)
+		return ret;
+
+	if (rsense == 0)
+		return -EINVAL;
+
+	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	memcpy(info, &ltc4286_info, sizeof(*info));
+
+	/* Default of VRANGE_SELECT = 1, 102.4V */
+	if (device_property_read_bool(&client->dev, "adi,vrange-low-enable")) {
+		/* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */
+		ret = i2c_smbus_read_word_data(client, LTC4286_MFR_CONFIG1);
+		if (ret < 0) {
+			return dev_err_probe(
+				&client->dev, ret,
+				"Failed to read manufacturer configuration one\n");
+		}
+
+		if ((ret & VRANGE_SELECT_BIT) != VRANGE_25P6) {
+			ret &= ~VRANGE_SELECT_BIT; /* VRANGE_SELECT = 0, 25.6V */
+			ret = i2c_smbus_write_word_data(
+				client, LTC4286_MFR_CONFIG1, ret);
+			if (ret < 0)
+				return dev_err_probe(&client->dev, ret,
+						     "Failed to set vrange\n");
+		}
+
+		info->m[PSC_VOLTAGE_IN] = 128;
+		info->m[PSC_VOLTAGE_OUT] = 128;
+		info->m[PSC_POWER] = 4 * rsense;
+		if (info->m[PSC_POWER] > INT_MAX)
+			return dev_err_probe(&client->dev, -ERANGE,
+					     "Power coefficient overflow\n");
+	} else {
+		info->m[PSC_POWER] = rsense;
+		if (info->m[PSC_POWER] > INT_MAX)
+			return dev_err_probe(&client->dev, -ERANGE,
+					     "Power coefficient overflow\n");
+	}
+
+	info->m[PSC_CURRENT_OUT] = 1024 * rsense;
+	if (info->m[PSC_CURRENT_OUT] > INT_MAX)
+		return dev_err_probe(&client->dev, -ERANGE,
+				     "Current coefficient overflow\n");
+
+	return pmbus_do_probe(client, info);
+}
+
+static const struct of_device_id ltc4286_of_match[] = {
+	{ .compatible = "lltc,ltc4286" },
+	{ .compatible = "lltc,ltc4287" },
+	{}
+};
+
+static struct i2c_driver ltc4286_driver = {
+	.driver = {
+		.name = "ltc4286",
+		.of_match_table = ltc4286_of_match,
+	},
+	.probe = ltc4286_probe,
+	.id_table = ltc4286_id,
+};
+
+module_i2c_driver(ltc4286_driver);
+
+MODULE_AUTHOR("Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>");
+MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v3 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings

[Conor Dooley]

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

On Tue, Oct 31, 2023 at 03:21:21PM +0800, Delphine CC Chiu wrote:
> Add a device tree bindings for ltc4286 device.
> 
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> 
> Changelog:
>   v3 - Revise adi,vrange-select-25p6 to adi,vrange-low-enable
>   v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
>      - Add type for adi,vrange-select-25p6
>      - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> ---
>  .../bindings/hwmon/lltc,ltc4286.yaml          | 52 +++++++++++++++++++
>  MAINTAINERS                                   | 10 ++++
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> new file mode 100644
> index 000000000000..4695bca77c05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LTC4286 power monitors
> +
> +maintainers:
> +  - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - lltc,ltc4286
> +      - lltc,ltc4287
> +
> +  reg:
> +    maxItems: 1
> +
> +  adi,vrange-low-enable:
> +    description:
> +      This property is a bool parameter to represent the
> +      voltage range is 25.6 volts or 102.4 volts for
> +      this chip.
> +      The default is 102.4 volts.

You've got weird wrapping of text here (short lines). Either this
property or the corollary work for me, depending on what Guenter
wants. With two nits below,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>


> +    type: boolean
> +
> +  shunt-resistor-micro-ohms:
> +    description:
> +      Resistor value in micro-Ohm

micro-ohms

> +
> +required:
> +  - compatible
> +  - reg
> +  - shunt-resistor-micro-ohms
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        power-sensor@40 {

the generic node name is "power-monitor".

Cheers,
Conor.

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

Re: [PATCH v3 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings

[Guenter Roeck]

On 10/31/23 10:06, Conor Dooley wrote:
> On Tue, Oct 31, 2023 at 03:21:21PM +0800, Delphine CC Chiu wrote:
>> Add a device tree bindings for ltc4286 device.
>>
>> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
>>
>> Changelog:
>>    v3 - Revise adi,vrange-select-25p6 to adi,vrange-low-enable
>>    v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
>>       - Add type for adi,vrange-select-25p6
>>       - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
>> ---
>>   .../bindings/hwmon/lltc,ltc4286.yaml          | 52 +++++++++++++++++++
>>   MAINTAINERS                                   | 10 ++++
>>   2 files changed, 62 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>> new file mode 100644
>> index 000000000000..4695bca77c05
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>> @@ -0,0 +1,52 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: LTC4286 power monitors
>> +
>> +maintainers:
>> +  - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - lltc,ltc4286
>> +      - lltc,ltc4287
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  adi,vrange-low-enable:
>> +    description:
>> +      This property is a bool parameter to represent the
>> +      voltage range is 25.6 volts or 102.4 volts for
>> +      this chip.
>> +      The default is 102.4 volts.
> 
> You've got weird wrapping of text here (short lines). Either this
> property or the corollary work for me, depending on what Guenter
> wants. With two nits below,
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 

Oh, I am tired of arguing, so I'll accept whatever gets a Reviewed-by:
tag from a DT maintainer (people do a pretty good job of wearing me down
lately).

Guenter

Re: [PATCH v3 2/2] hwmon: pmbus: Add ltc4286 driver

[Guenter Roeck]

On 10/31/23 00:21, Delphine CC Chiu wrote:
> Add a driver to support ltc4286 chip
> 
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> 
> Changelog:
>    v3 - Use dev_err_probe() instead of dev_err()
>       - The VRANGE_SELECT bit only be written if it actually changed
>       - Avoid the info pointer being overwritten
>       - Check the MBR value range to avoid overflow
>       - Revise ltc4286.rst to corrcet description
>    v2 - Revise Linear Technologies LTC4286 to
>         Analog Devices LTC4286 in Kconfig
>       - Add more description for this driver in Kconfig
>       - Add some comments for MBR setting in ltc4286.c
>       - Add ltc4286.rst
> ---
>   Documentation/hwmon/ltc4286.rst |  95 +++++++++++++++++

Needs to be added to Documentation/hwmon/index.rst

>   drivers/hwmon/pmbus/Kconfig     |   9 ++
>   drivers/hwmon/pmbus/Makefile    |   1 +
>   drivers/hwmon/pmbus/ltc4286.c   | 178 ++++++++++++++++++++++++++++++++
>   4 files changed, 283 insertions(+)
>   create mode 100644 Documentation/hwmon/ltc4286.rst
>   create mode 100644 drivers/hwmon/pmbus/ltc4286.c
> 
> diff --git a/Documentation/hwmon/ltc4286.rst b/Documentation/hwmon/ltc4286.rst
> new file mode 100644
> index 000000000000..2cd149676d86
> --- /dev/null
> +++ b/Documentation/hwmon/ltc4286.rst
> @@ -0,0 +1,95 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver ltc4286
> +=====================
> +
> +Supported chips:
> +
> +  * Analog Devices LTC4286
> +
> +    Prefix: 'ltc4286'
> +
> +    Addresses scanned: -
> +
> +    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4286.pdf
> +
> +  * Analog Devices LTC4287
> +
> +    Prefix: 'ltc4287'
> +
> +    Addresses scanned: -
> +
> +    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4287.pdf
> +
> +Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> +
> +
> +Description
> +-----------
> +
> +This driver supports hardware monitoring for Analog Devices LTC4286
> +and LTC4287 Hot-Swap Controller and Digital Power Monitors.
> +
> +LTC4286 and LTC4287 are hot-swap controllers that allow a circuit board
> +to be removed from or inserted into a live backplane. They also feature
> +current and voltage readback via an integrated 12 bit analog-to-digital
> +converter (ADC), accessed using a PMBus interface.
> +
> +The driver is a client driver to the core PMBus driver. Please see
> +Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
> +
> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
> +details.
> +
> +The shunt value in micro-ohms can be set via device tree at compile-time. Please
> +refer to the Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings
> +if the device tree is used.
> +
> +
> +Platform data support
> +---------------------
> +
> +The driver supports standard PMBus driver platform data. Please see
> +Documentation/hwmon/pmbus.rst for details.
> +
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported. Limits are read-write, history reset
> +attributes are write-only, all other attributes are read-only.
> +
> +======================= =======================================================
> +in1_label		"vin"
> +in1_input		Measured voltage.
> +in1_alarm		Input voltage alarm.
> +in1_min 		Minimum input voltage.
> +in1_max 		Maximum input voltage.
> +
> +in2_label		"vout1"
> +in2_input		Measured voltage.
> +in2_alarm		Output voltage alarm.
> +in2_min 		Minimum output voltage.
> +in2_max 		Maximum output voltage.
> +
> +curr1_label		"iout1"
> +curr1_input		Measured current.
> +curr1_alarm		Output current alarm.
> +curr1_max		Maximum current.
> +
> +power1_label		"pin"
> +power1_input		Input power.
> +power1_alarm		Input power alarm.
> +power1_max		Maximum poewr.
> +
> +temp1_input		Chip temperature.
> +temp1_min		Minimum chip temperature.
> +temp1_max		Maximum chip temperature.
> +temp1_crit		Critical chip temperature.
> +temp1_alarm		Chip temperature alarm.
> +======================= =======================================================
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index b4e93bd5835e..f2b53e8abc3c 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -226,6 +226,15 @@ config SENSORS_LTC3815
>   
>   	  This driver can also be built as a module. If so, the module will
>   	  be called ltc3815.

Add empty line

> +config SENSORS_LTC4286
> +	bool "Analog Devices LTC4286"
> +	help
> +	  LTC4286 is an integrated solution for hot swap applications that
> +	  allows a board to be safely inserted and removed from a live
> +	  backplane.
> +	  This chip could be used to monitor voltage, current, ...etc.
> +	  If you say yes here you get hardware monitoring support for Analog
> +	  Devices LTC4286.
>   
>   config SENSORS_MAX15301
>   	tristate "Maxim MAX15301"
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 84ee960a6c2d..94e28f6d6a61 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066)	+= lm25066.o
>   obj-$(CONFIG_SENSORS_LT7182S)	+= lt7182s.o
>   obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
>   obj-$(CONFIG_SENSORS_LTC3815)	+= ltc3815.o
> +obj-$(CONFIG_SENSORS_LTC4286)	+= ltc4286.o
>   obj-$(CONFIG_SENSORS_MAX15301)	+= max15301.o
>   obj-$(CONFIG_SENSORS_MAX16064)	+= max16064.o
>   obj-$(CONFIG_SENSORS_MAX16601)	+= max16601.o
> diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c
> new file mode 100644
> index 000000000000..042d3af99489
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/ltc4286.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include "pmbus.h"
> +
> +/* LTC4286 register */
> +#define LTC4286_MFR_CONFIG1	0xF2
> +
> +/* LTC4286 configuration */
> +#define VRANGE_SELECT_BIT	BIT(1)
> +
> +#define LTC4286_MFR_ID_SIZE	3
> +#define VRANGE_25P6		0
> +
> +enum chips { ltc4286, ltc4287 };

Not really used anywhere and can be dropped.

> +
> +/*
> + * Initialize the MBR as default settings which is referred to LTC4286 datasheet
> + * (March 22, 2022 version) table 3 page 16
> + */
> +static struct pmbus_driver_info ltc4286_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_IN] = direct,
> +	.format[PSC_VOLTAGE_OUT] = direct,
> +	.format[PSC_CURRENT_OUT] = direct,
> +	.format[PSC_POWER] = direct,
> +	.format[PSC_TEMPERATURE] = direct,
> +	.m[PSC_VOLTAGE_IN] = 32,
> +	.b[PSC_VOLTAGE_IN] = 0,
> +	.R[PSC_VOLTAGE_IN] = 1,
> +	.m[PSC_VOLTAGE_OUT] = 32,
> +	.b[PSC_VOLTAGE_OUT] = 0,
> +	.R[PSC_VOLTAGE_OUT] = 1,
> +	.m[PSC_CURRENT_OUT] = 1024,
> +	.b[PSC_CURRENT_OUT] = 0,
> +	/*
> +	 * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit.
> +	 * However, the rsense value that user input is mirco ohm.

micro

> +	 * Thus, the MBR setting which involves rsense should be shifted by 6 digits.
> +	 */
> +	.R[PSC_CURRENT_OUT] = 3 - 6,
> +	.m[PSC_POWER] = 1,
> +	.b[PSC_POWER] = 0,
> +	/*
> +	 * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit.
> +	 * However, the rsense value that user input is mirco ohm.

micro

> +	 * Thus, the MBR setting which involves rsense should be shifted by 6 digits.
> +	 */
> +	.R[PSC_POWER] = 4 - 6,
> +	.m[PSC_TEMPERATURE] = 1,
> +	.b[PSC_TEMPERATURE] = 273,
> +	.R[PSC_TEMPERATURE] = 0,
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> +		   PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT |
> +		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP,
> +};
> +
> +static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", ltc4286 },
> +						   { "ltc4287", ltc4287 },
> +						   {} };
> +MODULE_DEVICE_TABLE(i2c, ltc4286_id);
> +
> +static int ltc4286_probe(struct i2c_client *client)
> +{
> +	int ret;
> +	const struct i2c_device_id *mid;
> +	u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
> +	struct pmbus_driver_info *info;
> +	u32 rsense;
> +
> +	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer);
> +	if (ret < 0) {
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to read manufacturer id\n");
> +	}
> +
> +	/*
> +	 * Refer to ltc4286 datasheet page 20
> +	 * the manufacturer id is LTC
> +	 */
> +	if (ret != LTC4286_MFR_ID_SIZE ||
> +	    strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) {
> +		return dev_err_probe(&client->dev, ret,
> +				     "Manufacturer id mismatch\n");
> +	}
> +
> +	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
> +	if (ret < 0) {
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to read manufacturer model\n");
> +	}
> +
> +	for (mid = ltc4286_id; mid->name[0]; mid++) {
> +		if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
> +			break;
> +	}
> +	if (!mid->name[0])
> +		return dev_err_probe(&client->dev, -ENODEV,
> +				     "Unsupported device\n");
> +
> +	ret = of_property_read_u32(client->dev.of_node,
> +				   "shunt-resistor-micro-ohms", &rsense);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (rsense == 0)
> +		return -EINVAL;
> +
> +	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +	memcpy(info, &ltc4286_info, sizeof(*info));

devm_kmemdup()

> +
> +	/* Default of VRANGE_SELECT = 1, 102.4V */
> +	if (device_property_read_bool(&client->dev, "adi,vrange-low-enable")) {
> +		/* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */
> +		ret = i2c_smbus_read_word_data(client, LTC4286_MFR_CONFIG1);
> +		if (ret < 0) {
> +			return dev_err_probe(
> +				&client->dev, ret,
> +				"Failed to read manufacturer configuration one\n");
> +		}
> +
> +		if ((ret & VRANGE_SELECT_BIT) != VRANGE_25P6) {
> +			ret &= ~VRANGE_SELECT_BIT; /* VRANGE_SELECT = 0, 25.6V */
> +			ret = i2c_smbus_write_word_data(
> +				client, LTC4286_MFR_CONFIG1, ret);
> +			if (ret < 0)
> +				return dev_err_probe(&client->dev, ret,
> +						     "Failed to set vrange\n");
> +		}
> +
> +		info->m[PSC_VOLTAGE_IN] = 128;
> +		info->m[PSC_VOLTAGE_OUT] = 128;
> +		info->m[PSC_POWER] = 4 * rsense;
> +		if (info->m[PSC_POWER] > INT_MAX)

This is too late. See below.

> +			return dev_err_probe(&client->dev, -ERANGE,
> +					     "Power coefficient overflow\n");
> +	} else {
> +		info->m[PSC_POWER] = rsense;
> +		if (info->m[PSC_POWER] > INT_MAX)
> +			return dev_err_probe(&client->dev, -ERANGE,
> +					     "Power coefficient overflow\n");

This still needs to be written into the chip. There is no guarantee that
the chip is in its default configuration when the driver is loaded.

> +	}
> +
> +	info->m[PSC_CURRENT_OUT] = 1024 * rsense;
> +	if (info->m[PSC_CURRENT_OUT] > INT_MAX)

This is too late. If rsense == INT_MAX, for example, 1024 * rsense
will be some negative number.

> +		return dev_err_probe(&client->dev, -ERANGE,
> +				     "Current coefficient overflow\n");
> +
> +	return pmbus_do_probe(client, info);
> +}
> +
> +static const struct of_device_id ltc4286_of_match[] = {
> +	{ .compatible = "lltc,ltc4286" },
> +	{ .compatible = "lltc,ltc4287" },
> +	{}
> +};
> +
> +static struct i2c_driver ltc4286_driver = {
> +	.driver = {
> +		.name = "ltc4286",
> +		.of_match_table = ltc4286_of_match,
> +	},
> +	.probe = ltc4286_probe,
> +	.id_table = ltc4286_id,
> +};
> +
> +module_i2c_driver(ltc4286_driver);
> +
> +MODULE_AUTHOR("Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>");
> +MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles");
> +MODULE_LICENSE("GPL");

RE: [PATCH v3 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings

[Delphine_CC_Chiu/WYHQ/Wiwynn]

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Wednesday, November 1, 2023 1:06 AM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>
> Cc: patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Guenter Roeck
> <linux@roeck-us.net>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
> Jonathan Corbet <corbet@lwn.net>; linux-i2c@vger.kernel.org;
> linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org
> Subject: Re: [PATCH v3 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver
> bindings
> 
> On Tue, Oct 31, 2023 at 03:21:21PM +0800, Delphine CC Chiu wrote:
> > Add a device tree bindings for ltc4286 device.
> >
> > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> >
> > Changelog:
> >   v3 - Revise adi,vrange-select-25p6 to adi,vrange-low-enable
> >   v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> >      - Add type for adi,vrange-select-25p6
> >      - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> > ---
> >  .../bindings/hwmon/lltc,ltc4286.yaml          | 52
> +++++++++++++++++++
> >  MAINTAINERS                                   | 10 ++++
> >  2 files changed, 62 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > new file mode 100644
> > index 000000000000..4695bca77c05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > @@ -0,0 +1,52 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LTC4286 power monitors
> > +
> > +maintainers:
> > +  - Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - lltc,ltc4286
> > +      - lltc,ltc4287
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  adi,vrange-low-enable:
> > +    description:
> > +      This property is a bool parameter to represent the
> > +      voltage range is 25.6 volts or 102.4 volts for
> > +      this chip.
> > +      The default is 102.4 volts.
> 
> You've got weird wrapping of text here (short lines). Either this property or the
> corollary work for me, depending on what Guenter wants. With two nits
> below,
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

We will revise to
adi,vrange-low-enable:
  description:
    This property is a bool parameter to represent the
    voltage range is 25.6 volts or 102.4 volts for this chip.

> 
> 
> > +    type: boolean
> > +
> > +  shunt-resistor-micro-ohms:
> > +    description:
> > +      Resistor value in micro-Ohm
> 
> micro-ohms

We will revise to micro-ohms

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - shunt-resistor-micro-ohms
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        power-sensor@40 {
> 
> the generic node name is "power-monitor".


We will revise to power-monitor@40

> 
> Cheers,
> Conor.

RE: [PATCH v3 2/2] hwmon: pmbus: Add ltc4286 driver

[Delphine_CC_Chiu/WYHQ/Wiwynn]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Wednesday, November 1, 2023 2:10 AM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
> patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Jonathan Corbet
> <corbet@lwn.net>
> Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
> linux-i2c@vger.kernel.org; linux-hwmon@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-doc@vger.kernel.org
> Subject: Re: [PATCH v3 2/2] hwmon: pmbus: Add ltc4286 driver
> 
>   Security Reminder: Please be aware that this email is sent by an external
> sender.
> 
> On 10/31/23 00:21, Delphine CC Chiu wrote:
> > Add a driver to support ltc4286 chip
> >
> > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> >
> > Changelog:
> >    v3 - Use dev_err_probe() instead of dev_err()
> >       - The VRANGE_SELECT bit only be written if it actually changed
> >       - Avoid the info pointer being overwritten
> >       - Check the MBR value range to avoid overflow
> >       - Revise ltc4286.rst to corrcet description
> >    v2 - Revise Linear Technologies LTC4286 to
> >         Analog Devices LTC4286 in Kconfig
> >       - Add more description for this driver in Kconfig
> >       - Add some comments for MBR setting in ltc4286.c
> >       - Add ltc4286.rst
> > ---
> >   Documentation/hwmon/ltc4286.rst |  95 +++++++++++++++++
> 
> Needs to be added to Documentation/hwmon/index.rst

We will add to Documentation/hwmon/index.rst

> 
> >   drivers/hwmon/pmbus/Kconfig     |   9 ++
> >   drivers/hwmon/pmbus/Makefile    |   1 +
> >   drivers/hwmon/pmbus/ltc4286.c   | 178
> ++++++++++++++++++++++++++++++++
> >   4 files changed, 283 insertions(+)
> >   create mode 100644 Documentation/hwmon/ltc4286.rst
> >   create mode 100644 drivers/hwmon/pmbus/ltc4286.c
> >
> > diff --git a/Documentation/hwmon/ltc4286.rst
> > b/Documentation/hwmon/ltc4286.rst new file mode 100644 index
> > 000000000000..2cd149676d86
> > --- /dev/null
> > +++ b/Documentation/hwmon/ltc4286.rst
> > @@ -0,0 +1,95 @@
> > +.. SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +Kernel driver ltc4286
> > +=====================
> > +
> > +Supported chips:
> > +
> > +  * Analog Devices LTC4286
> > +
> > +    Prefix: 'ltc4286'
> > +
> > +    Addresses scanned: -
> > +
> > +    Datasheet:
> > + https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> > +
> w.analog.com%2Fmedia%2Fen%2Ftechnical-documentation%2Fdata-sheets%2
> F
> > +
> ltc4286.pdf&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7C19174887b
> 0924
> > +
> 20a374208dbda3c8d3b%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%
> 7C638
> > +
> 343725997659187%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoi
> > +
> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=0Vi
> JT74
> > + gySA7E0eBm%2FOBDkVAxp72j1spi8UG8un6zW4%3D&reserved=0
> > +
> > +  * Analog Devices LTC4287
> > +
> > +    Prefix: 'ltc4287'
> > +
> > +    Addresses scanned: -
> > +
> > +    Datasheet:
> > + https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> > +
> w.analog.com%2Fmedia%2Fen%2Ftechnical-documentation%2Fdata-sheets%2
> F
> > +
> ltc4287.pdf&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7C19174887b
> 0924
> > +
> 20a374208dbda3c8d3b%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%
> 7C638
> > +
> 343725997659187%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoi
> > +
> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ZW
> BSxI9
> > + ZgOgKMTU0UhQal9OiYjAUTPLXijAZqzN%2Fuh0%3D&reserved=0
> > +
> > +Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> > +
> > +
> > +Description
> > +-----------
> > +
> > +This driver supports hardware monitoring for Analog Devices LTC4286
> > +and LTC4287 Hot-Swap Controller and Digital Power Monitors.
> > +
> > +LTC4286 and LTC4287 are hot-swap controllers that allow a circuit
> > +board to be removed from or inserted into a live backplane. They also
> > +feature current and voltage readback via an integrated 12 bit
> > +analog-to-digital converter (ADC), accessed using a PMBus interface.
> > +
> > +The driver is a client driver to the core PMBus driver. Please see
> > +Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
> > +
> > +
> > +Usage Notes
> > +-----------
> > +
> > +This driver does not auto-detect devices. You will have to
> > +instantiate the devices explicitly. Please see
> > +Documentation/i2c/instantiating-devices.rst for details.
> > +
> > +The shunt value in micro-ohms can be set via device tree at
> > +compile-time. Please refer to the
> > +Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings
> if the device tree is used.
> > +
> > +
> > +Platform data support
> > +---------------------
> > +
> > +The driver supports standard PMBus driver platform data. Please see
> > +Documentation/hwmon/pmbus.rst for details.
> > +
> > +
> > +Sysfs entries
> > +-------------
> > +
> > +The following attributes are supported. Limits are read-write,
> > +history reset attributes are write-only, all other attributes are read-only.
> > +
> > +=======================
> =======================================================
> > +in1_label            "vin"
> > +in1_input            Measured voltage.
> > +in1_alarm            Input voltage alarm.
> > +in1_min              Minimum input voltage.
> > +in1_max              Maximum input voltage.
> > +
> > +in2_label            "vout1"
> > +in2_input            Measured voltage.
> > +in2_alarm            Output voltage alarm.
> > +in2_min              Minimum output voltage.
> > +in2_max              Maximum output voltage.
> > +
> > +curr1_label          "iout1"
> > +curr1_input          Measured current.
> > +curr1_alarm          Output current alarm.
> > +curr1_max            Maximum current.
> > +
> > +power1_label         "pin"
> > +power1_input         Input power.
> > +power1_alarm         Input power alarm.
> > +power1_max           Maximum poewr.
> > +
> > +temp1_input          Chip temperature.
> > +temp1_min            Minimum chip temperature.
> > +temp1_max            Maximum chip temperature.
> > +temp1_crit           Critical chip temperature.
> > +temp1_alarm          Chip temperature alarm.
> > +=======================
> > +=======================================================
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index b4e93bd5835e..f2b53e8abc3c 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -226,6 +226,15 @@ config SENSORS_LTC3815
> >
> >         This driver can also be built as a module. If so, the module will
> >         be called ltc3815.
> 
> Add empty line

We will add empty line here

> 
> > +config SENSORS_LTC4286
> > +     bool "Analog Devices LTC4286"
> > +     help
> > +       LTC4286 is an integrated solution for hot swap applications that
> > +       allows a board to be safely inserted and removed from a live
> > +       backplane.
> > +       This chip could be used to monitor voltage, current, ...etc.
> > +       If you say yes here you get hardware monitoring support for Analog
> > +       Devices LTC4286.
> >
> >   config SENSORS_MAX15301
> >       tristate "Maxim MAX15301"
> > diff --git a/drivers/hwmon/pmbus/Makefile
> > b/drivers/hwmon/pmbus/Makefile index 84ee960a6c2d..94e28f6d6a61
> 100644
> > --- a/drivers/hwmon/pmbus/Makefile
> > +++ b/drivers/hwmon/pmbus/Makefile
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066)       +=
> lm25066.o
> >   obj-$(CONFIG_SENSORS_LT7182S)       += lt7182s.o
> >   obj-$(CONFIG_SENSORS_LTC2978)       += ltc2978.o
> >   obj-$(CONFIG_SENSORS_LTC3815)       += ltc3815.o
> > +obj-$(CONFIG_SENSORS_LTC4286)        += ltc4286.o
> >   obj-$(CONFIG_SENSORS_MAX15301)      += max15301.o
> >   obj-$(CONFIG_SENSORS_MAX16064)      += max16064.o
> >   obj-$(CONFIG_SENSORS_MAX16601)      += max16601.o
> > diff --git a/drivers/hwmon/pmbus/ltc4286.c
> > b/drivers/hwmon/pmbus/ltc4286.c new file mode 100644 index
> > 000000000000..042d3af99489
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus/ltc4286.c
> > @@ -0,0 +1,178 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pmbus.h>
> > +#include "pmbus.h"
> > +
> > +/* LTC4286 register */
> > +#define LTC4286_MFR_CONFIG1  0xF2
> > +
> > +/* LTC4286 configuration */
> > +#define VRANGE_SELECT_BIT    BIT(1)
> > +
> > +#define LTC4286_MFR_ID_SIZE  3
> > +#define VRANGE_25P6          0
> > +
> > +enum chips { ltc4286, ltc4287 };
> 
> Not really used anywhere and can be dropped.

We use this enum in v3 line 63-64

> 
> > +
> > +/*
> > + * Initialize the MBR as default settings which is referred to
> > +LTC4286 datasheet
> > + * (March 22, 2022 version) table 3 page 16  */ static struct
> > +pmbus_driver_info ltc4286_info = {
> > +     .pages = 1,
> > +     .format[PSC_VOLTAGE_IN] = direct,
> > +     .format[PSC_VOLTAGE_OUT] = direct,
> > +     .format[PSC_CURRENT_OUT] = direct,
> > +     .format[PSC_POWER] = direct,
> > +     .format[PSC_TEMPERATURE] = direct,
> > +     .m[PSC_VOLTAGE_IN] = 32,
> > +     .b[PSC_VOLTAGE_IN] = 0,
> > +     .R[PSC_VOLTAGE_IN] = 1,
> > +     .m[PSC_VOLTAGE_OUT] = 32,
> > +     .b[PSC_VOLTAGE_OUT] = 0,
> > +     .R[PSC_VOLTAGE_OUT] = 1,
> > +     .m[PSC_CURRENT_OUT] = 1024,
> > +     .b[PSC_CURRENT_OUT] = 0,
> > +     /*
> > +      * The rsense value used in MBR formula in LTC4286 datasheet
> should be ohm unit.
> > +      * However, the rsense value that user input is mirco ohm.
> 
> micro

We will revise this typo

> 
> > +      * Thus, the MBR setting which involves rsense should be shifted by 6
> digits.
> > +      */
> > +     .R[PSC_CURRENT_OUT] = 3 - 6,
> > +     .m[PSC_POWER] = 1,
> > +     .b[PSC_POWER] = 0,
> > +     /*
> > +      * The rsense value used in MBR formula in LTC4286 datasheet
> should be ohm unit.
> > +      * However, the rsense value that user input is mirco ohm.
> 
> micro

We will revise this typo

> 
> > +      * Thus, the MBR setting which involves rsense should be shifted by 6
> digits.
> > +      */
> > +     .R[PSC_POWER] = 4 - 6,
> > +     .m[PSC_TEMPERATURE] = 1,
> > +     .b[PSC_TEMPERATURE] = 273,
> > +     .R[PSC_TEMPERATURE] = 0,
> > +     .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT |
> PMBUS_HAVE_IOUT |
> > +                PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP |
> PMBUS_HAVE_STATUS_VOUT |
> > +                PMBUS_HAVE_STATUS_IOUT |
> PMBUS_HAVE_STATUS_TEMP, };
> > +
> > +static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", ltc4286 },
> > +                                                { "ltc4287",
> ltc4287 },
> > +                                                {} };
> > +MODULE_DEVICE_TABLE(i2c, ltc4286_id);
> > +
> > +static int ltc4286_probe(struct i2c_client *client) {
> > +     int ret;
> > +     const struct i2c_device_id *mid;
> > +     u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
> > +     struct pmbus_driver_info *info;
> > +     u32 rsense;
> > +
> > +     ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID,
> block_buffer);
> > +     if (ret < 0) {
> > +             return dev_err_probe(&client->dev, ret,
> > +                                  "Failed to read manufacturer
> id\n");
> > +     }
> > +
> > +     /*
> > +      * Refer to ltc4286 datasheet page 20
> > +      * the manufacturer id is LTC
> > +      */
> > +     if (ret != LTC4286_MFR_ID_SIZE ||
> > +         strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) {
> > +             return dev_err_probe(&client->dev, ret,
> > +                                  "Manufacturer id mismatch\n");
> > +     }
> > +
> > +     ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL,
> block_buffer);
> > +     if (ret < 0) {
> > +             return dev_err_probe(&client->dev, ret,
> > +                                  "Failed to read manufacturer
> model\n");
> > +     }
> > +
> > +     for (mid = ltc4286_id; mid->name[0]; mid++) {
> > +             if (!strncasecmp(mid->name, block_buffer,
> strlen(mid->name)))
> > +                     break;
> > +     }
> > +     if (!mid->name[0])
> > +             return dev_err_probe(&client->dev, -ENODEV,
> > +                                  "Unsupported device\n");
> > +
> > +     ret = of_property_read_u32(client->dev.of_node,
> > +                                "shunt-resistor-micro-ohms",
> &rsense);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     if (rsense == 0)
> > +             return -EINVAL;
> > +
> > +     info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> > +     if (!info)
> > +             return -ENOMEM;
> > +     memcpy(info, &ltc4286_info, sizeof(*info));
> 
> devm_kmemdup()

We will revise as below
info = devm_kmemdup(&client->dev, &ltc4286_info, sizeof(*info), GFP_KERNEL);
if (!info)
	return -ENOMEM;

> 
> > +
> > +     /* Default of VRANGE_SELECT = 1, 102.4V */
> > +     if (device_property_read_bool(&client->dev, "adi,vrange-low-enable"))
> {
> > +             /* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */
> > +             ret = i2c_smbus_read_word_data(client,
> LTC4286_MFR_CONFIG1);
> > +             if (ret < 0) {
> > +                     return dev_err_probe(
> > +                             &client->dev, ret,
> > +                             "Failed to read manufacturer
> configuration one\n");
> > +             }
> > +
> > +             if ((ret & VRANGE_SELECT_BIT) != VRANGE_25P6) {
> > +                     ret &= ~VRANGE_SELECT_BIT; /*
> VRANGE_SELECT = 0, 25.6V */
> > +                     ret = i2c_smbus_write_word_data(
> > +                             client, LTC4286_MFR_CONFIG1, ret);
> > +                     if (ret < 0)
> > +                             return dev_err_probe(&client->dev, ret,
> > +                                                  "Failed to set
> vrange\n");
> > +             }
> > +
> > +             info->m[PSC_VOLTAGE_IN] = 128;
> > +             info->m[PSC_VOLTAGE_OUT] = 128;
> > +             info->m[PSC_POWER] = 4 * rsense;
> > +             if (info->m[PSC_POWER] > INT_MAX)
> 
> This is too late. See below.

We will check sooner.

> 
> > +                     return dev_err_probe(&client->dev, -ERANGE,
> > +                                          "Power coefficient
> overflow\n");
> > +     } else {
> > +             info->m[PSC_POWER] = rsense;
> > +             if (info->m[PSC_POWER] > INT_MAX)
> > +                     return dev_err_probe(&client->dev, -ERANGE,
> > +                                          "Power coefficient
> > + overflow\n");
> 
> This still needs to be written into the chip. There is no guarantee that the chip
> is in its default configuration when the driver is loaded.

We will check sooner.

> 
> > +     }
> > +
> > +     info->m[PSC_CURRENT_OUT] = 1024 * rsense;
> > +     if (info->m[PSC_CURRENT_OUT] > INT_MAX)
> 
> This is too late. If rsense == INT_MAX, for example, 1024 * rsense will be some
> negative number.

We will check sooner.

> 
> > +             return dev_err_probe(&client->dev, -ERANGE,
> > +                                  "Current coefficient overflow\n");
> > +
> > +     return pmbus_do_probe(client, info); }
> > +
> > +static const struct of_device_id ltc4286_of_match[] = {
> > +     { .compatible = "lltc,ltc4286" },
> > +     { .compatible = "lltc,ltc4287" },
> > +     {}
> > +};
> > +
> > +static struct i2c_driver ltc4286_driver = {
> > +     .driver = {
> > +             .name = "ltc4286",
> > +             .of_match_table = ltc4286_of_match,
> > +     },
> > +     .probe = ltc4286_probe,
> > +     .id_table = ltc4286_id,
> > +};
> > +
> > +module_i2c_driver(ltc4286_driver);
> > +
> > +MODULE_AUTHOR("Delphine CC Chiu
> <Delphine_CC_Chiu@wiwynn.com>");
> > +MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles");
> > +MODULE_LICENSE("GPL");