[PATCH 0/5] hwmon: (pmbus/pim4328) Add pim4328 PMBus driver

[PATCH 0/5] hwmon: (pmbus/pim4328) Add pim4328 PMBus driver

[Erik Rosen]

Add hardware monitoring support for the Flex power interface modules
PIM4006, PIM4328 and PIM4820.

The modules are equipped with dual feed input and has support for
hotswap, holdup and various circuit protection functionality.

[PATCH 1/5]
PIM4328 and PIM4820 use the direct mode data format so a new function
is added to the pmbus_core driver to be able to read and decode
the COEFFICIENTS command.

[PATCH 2/5]
The modules have no CAPABILITY or WRITE_PROTECT commands. If these
commands are read, the modules return invalid data (0xFF),
so in addition to the NO_CAPABILITY flag we need a NO_WRITE_PROTECT
flag to tell the pmbus_core driver to not access this register.

[PATCH 3/5]
The two inputs are modelled using virtual phases but there
is a limitation in the pmbus_core that disallows monitoring
of phase functions if there is no corresponding function on
the page level.

In this specific case the PIM4006 module allows
monitoring of current on each input separately,
but there is no corresponding command on the page level.

Is there a specific reason for this limitation?
Otherwise we suggest relaxing this criteria.

[PATCH 4/5]
All modules use manufacturer specific registers (mfr) for
status data and only supports the CML bit in the PMBus
STATUS register. The driver overrides reading the STATUS
register and maps the bits in the mfr registers to the STATUS
register alarm bits.


Addendum
In view of the comment in the generic pmbus driver on adding
support for direct mode, we did look into it but decided against
it because (1) we do not really have a usecase for it and (2) to
implement truly generic direct mode support we need to handle
the siuation when there are different coefficients for reading
or writing a register.

This patch has been tested with PIM4406, PIM4280 and PIM4328
modules. 

Erik Rosen (5):
  Add function for reading direct mode coefficients
  Add new pmbus flag NO_WRITE_PROTECT
  Allow phase function even if it's not on page
  Add PMBus driver for PIM4006, PIM4328 and PIM4820
  Add documentation for the pim4328 PMBus driver

 Documentation/hwmon/index.rst    |   1 +
 Documentation/hwmon/pim4328.rst  | 105 +++++++++++
 MAINTAINERS                      |   7 +
 drivers/hwmon/pmbus/Kconfig      |   9 +
 drivers/hwmon/pmbus/Makefile     |   1 +
 drivers/hwmon/pmbus/pim4328.c    | 299 +++++++++++++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus.h      |   4 +
 drivers/hwmon/pmbus/pmbus_core.c |  63 +++++--
 include/linux/pmbus.h            |   9 +
 9 files changed, 487 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/hwmon/pim4328.rst
 create mode 100644 drivers/hwmon/pmbus/pim4328.c


base-commit: 6efb943b8616ec53a5e444193dccf1af9ad627b5
-- 
2.20.1

[PATCH 1/5] hwmon: (pmbus/pim4328) Add function for reading direct mode coefficients

[Erik Rosen]

Add the function pmbus_read_coefficients to pmbus_core to be able to
read and decode the coefficients for the direct format for a certain
command and read/write direction.

Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
---
 drivers/hwmon/pmbus/pmbus.h      |  4 ++++
 drivers/hwmon/pmbus/pmbus_core.c | 38 ++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 3968924f8533..a131b253ebf9 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -499,6 +499,10 @@ int pmbus_get_fan_rate_cached(struct i2c_client *client, int page, int id,
 			      enum pmbus_fan_mode mode);
 int pmbus_update_fan(struct i2c_client *client, int page, int id,
 		     u8 config, u8 mask, u16 command);
+int pmbus_read_coefficients(struct i2c_client *client,
+			    struct pmbus_driver_info *info,
+			    enum pmbus_sensor_classes sensor_class,
+			    u8 command, bool for_reading);
 struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
 
 #endif /* PMBUS_H */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index bbd745178147..14d3d3352aac 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -301,6 +301,44 @@ int pmbus_update_fan(struct i2c_client *client, int page, int id,
 }
 EXPORT_SYMBOL_NS_GPL(pmbus_update_fan, PMBUS);
 
+/*
+ * Read the coefficients for direct mode.
+ */
+int pmbus_read_coefficients(struct i2c_client *client,
+			    struct pmbus_driver_info *info,
+			    enum pmbus_sensor_classes sensor_class,
+			    u8 command, bool for_reading)
+{
+	int rv;
+	union i2c_smbus_data data;
+	s8 R;
+	s16 m, b;
+
+	data.block[0] = 2;
+	data.block[1] = command;
+	data.block[2] = for_reading ? 0x01 : 0x00;
+
+	rv = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+			    I2C_SMBUS_WRITE, PMBUS_COEFFICIENTS,
+			    I2C_SMBUS_BLOCK_PROC_CALL, &data);
+
+	if (rv < 0)
+		return rv;
+
+	if (data.block[0] != 5)
+		return -EIO;
+
+	m = data.block[1] | (data.block[2] << 8);
+	b = data.block[3] | (data.block[4] << 8);
+	R = data.block[5];
+	info->m[sensor_class] = m;
+	info->b[sensor_class] = b;
+	info->R[sensor_class] = R;
+
+	return rv;
+}
+EXPORT_SYMBOL_GPL(pmbus_read_coefficients);
+
 int pmbus_read_word_data(struct i2c_client *client, int page, int phase, u8 reg)
 {
 	int rv;
-- 
2.20.1

[PATCH 2/5] hwmon: (pmbus/pim4328) Add new pmbus flag NO_WRITE_PROTECT

[Erik Rosen]

Some PMBus chips respond with invalid data when reading the WRITE_PROTECT
register. For such chips, this flag should be set so that the PMBus core
driver doesn't use the WRITE_PROTECT command to determine it's behavior.

Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
---
 drivers/hwmon/pmbus/pmbus_core.c | 9 ++++++---
 include/linux/pmbus.h            | 9 +++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index c92ad5764301..944570edf37f 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2265,9 +2265,12 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
 	 * faults, and we should not try it. Also, in that case, writes into
 	 * limit registers need to be disabled.
 	 */
-	ret = i2c_smbus_read_byte_data(client, PMBUS_WRITE_PROTECT);
-	if (ret > 0 && (ret & PB_WP_ANY))
-		data->flags |= PMBUS_WRITE_PROTECTED | PMBUS_SKIP_STATUS_CHECK;
+	if (!(data->flags & PMBUS_NO_WRITE_PROTECT)) {
+		ret = i2c_smbus_read_byte_data(client, PMBUS_WRITE_PROTECT);
+		if (ret > 0 && (ret & PB_WP_ANY))
+			data->flags |= PMBUS_WRITE_PROTECTED
+				    | PMBUS_SKIP_STATUS_CHECK;
+	}
 
 	if (data->info->pages)
 		pmbus_clear_faults(client);
diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
index 12cbbf305969..f720470b1bab 100644
--- a/include/linux/pmbus.h
+++ b/include/linux/pmbus.h
@@ -43,6 +43,15 @@
  */
 #define PMBUS_NO_CAPABILITY			BIT(2)
 
+/*
+ * PMBUS_NO_WRITE_PROTECT
+ *
+ * Some PMBus chips respond with invalid data when reading the WRITE_PROTECT
+ * register. For such chips, this flag should be set so that the PMBus core
+ * driver doesn't use the WRITE_PROTECT command to determine it's behavior.
+ */
+#define PMBUS_NO_WRITE_PROTECT			BIT(4)
+
 struct pmbus_platform_data {
 	u32 flags;		/* Device specific flags */
 
-- 
2.20.1

[PATCH 3/5] hwmon: (pmbus/pim4328) Allow phase function even if it's not on page

[Erik Rosen]

Allow the use of a phase function even if it does not exist not on
the associated page.

Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
---
 drivers/hwmon/pmbus/pmbus_core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 944570edf37f..2f355d5d83bc 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1366,14 +1366,14 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client,
 
 		pages = paged ? info->pages : 1;
 		for (page = 0; page < pages; page++) {
-			if (!(info->func[page] & attrs->func))
-				continue;
-			ret = pmbus_add_sensor_attrs_one(client, data, info,
-							 name, index, page,
-							 0xff, attrs, paged);
-			if (ret)
-				return ret;
-			index++;
+			if (info->func[page] & attrs->func) {
+				ret = pmbus_add_sensor_attrs_one(client, data, info,
+								 name, index, page,
+								 0xff, attrs, paged);
+				if (ret)
+					return ret;
+				index++;
+			}
 			if (info->phases[page]) {
 				int phase;
 
-- 
2.20.1

[PATCH 4/5] hwmon: (pmbus/pim4328) Add PMBus driver for PIM4006, PIM4328 and PIM4820

[Erik Rosen]

Add hardware monitoring support for Flex power interface modules PIM4006,
PIM4328 and PIM4820.

Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
---
 drivers/hwmon/pmbus/Kconfig   |   9 +
 drivers/hwmon/pmbus/Makefile  |   1 +
 drivers/hwmon/pmbus/pim4328.c | 310 ++++++++++++++++++++++++++++++++++
 3 files changed, 320 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/pim4328.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 37a5c39784fa..001527c71269 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -257,6 +257,15 @@ config SENSORS_MP2975
 	  This driver can also be built as a module. If so, the module will
 	  be called mp2975.
 
+config SENSORS_PIM4328
+	tristate "Flex PIM4328 and compatibles"
+	help
+	  If you say yes here you get hardware monitoring support for Flex
+	  PIM4328, PIM4820 and PIM4006 Power Interface Modules.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called pim4328.
+
 config SENSORS_PM6764TR
 	tristate "ST PM6764TR"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index f8dcc27cd56a..2a12397535ba 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
 obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
 obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
 obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
+obj-$(CONFIG_SENSORS_PIM4328)   += pim4328.o
diff --git a/drivers/hwmon/pmbus/pim4328.c b/drivers/hwmon/pmbus/pim4328.c
new file mode 100644
index 000000000000..b9aa4f76f6cd
--- /dev/null
+++ b/drivers/hwmon/pmbus/pim4328.c
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hardware monitoring driver for PIM4006, PIM4328 and PIM4820
+ *
+ * Copyright (c) 2021 Flextronics International Sweden AB
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/pmbus.h>
+#include "pmbus.h"
+
+enum chips { pim4006, pim4328, pim4820 };
+
+struct pim4328_data {
+	enum chips id;
+	struct pmbus_driver_info info;
+};
+
+#define to_pim4328_data(x)  container_of(x, struct pim4328_data, info)
+
+/* PIM4006 and PIM4328 */
+#define PIM4328_MFR_READ_VINA		0xd3
+#define PIM4328_MFR_READ_VINB		0xd4
+
+/* PIM4006 */
+#define PIM4328_MFR_READ_IINA		0xd6
+#define PIM4328_MFR_READ_IINB		0xd7
+#define PIM4328_MFR_FET_CHECKSTATUS     0xd9
+
+/* PIM4328 */
+#define PIM4328_MFR_STATUS_BITS		0xd5
+
+/* PIM4820 */
+#define PIM4328_MFR_READ_STATUS		0xd0
+
+static const struct i2c_device_id pim4328_id[] = {
+	{"bmr455", pim4328},
+	{"pim4006", pim4006},
+	{"pim4106", pim4006},
+	{"pim4206", pim4006},
+	{"pim4306", pim4006},
+	{"pim4328", pim4328},
+	{"pim4406", pim4006},
+	{"pim4820", pim4820},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, pim4328_id);
+
+static int pim4328_read_word_data(struct i2c_client *client, int page,
+				  int phase, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	struct pim4328_data *data = to_pim4328_data(info);
+	int ret, status;
+
+	if (page > 0)
+		return -ENXIO;
+
+	switch (reg) {
+	case PMBUS_STATUS_WORD:
+		ret = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
+		if (ret >= 0) {
+			if (data->id == pim4006) {
+				status = pmbus_read_word_data(client, page, 0xff,
+							      PIM4328_MFR_FET_CHECKSTATUS);
+				if (status > 0) {
+					if (status & 0x0630) /* Input UV */
+						ret |= 0x08;
+				}
+			} else if (data->id == pim4328) {
+				status = pmbus_read_byte_data(client, page,
+							      PIM4328_MFR_STATUS_BITS);
+				if (status > 0) {
+					if (status & 0x04) /* Input UV */
+						ret |= 0x08;
+					if (status & 0x40) /* Output UV */
+						ret |= 0x80;
+				}
+			} else if (data->id == pim4820) {
+				status = pmbus_read_byte_data(client, page,
+							      PIM4328_MFR_READ_STATUS);
+				if (status > 0) {
+					if (status & 0x05) /* Input OV or OC */
+						ret |= 0x2001;
+					if (status & 0x1a) /* Input UV */
+						ret |= 0x2008;
+					if (status & 0x40) /* OT */
+						ret |= 0x0004;
+				}
+			}
+		}
+		break;
+	case PMBUS_READ_VIN:
+		if (phase != 0xff) {
+			ret = pmbus_read_word_data(client, page, phase,
+						   phase == 0 ? PIM4328_MFR_READ_VINA
+							      : PIM4328_MFR_READ_VINB);
+		} else {
+			ret = -ENODATA;
+		}
+		break;
+	case PMBUS_READ_IIN:
+		if (phase != 0xff) {
+			ret = pmbus_read_word_data(client, page, phase,
+						   phase == 0 ? PIM4328_MFR_READ_IINA
+							      : PIM4328_MFR_READ_IINB);
+		} else {
+			ret = -ENODATA;
+		}
+		break;
+	default:
+		ret = -ENODATA;
+	}
+
+	return ret;
+}
+
+static int pim4328_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	int ret;
+
+	if (page > 0)
+		return -ENXIO;
+
+	switch (reg) {
+	case PMBUS_STATUS_BYTE:
+		ret = pim4328_read_word_data(client, page, 0xff, PMBUS_STATUS_WORD);
+		if (ret > 0)
+			ret &= 0xff;
+		break;
+	default:
+		ret = -ENODATA;
+	}
+
+	return ret;
+}
+
+static int pim4328_probe(struct i2c_client *client)
+{
+	int status;
+	u8 device_id[I2C_SMBUS_BLOCK_MAX + 1];
+	const struct i2c_device_id *mid;
+	struct pim4328_data *data;
+	struct pmbus_driver_info *info;
+	struct pmbus_platform_data *pdata;
+	struct device *dev = &client->dev;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_BYTE_DATA
+				     | I2C_FUNC_SMBUS_BLOCK_DATA))
+		return -ENODEV;
+
+	data = devm_kzalloc(&client->dev, sizeof(struct pim4328_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	status = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, device_id);
+	if (status < 0) {
+		dev_err(&client->dev, "Failed to read Manufacturer Model\n");
+		return status;
+	}
+	for (mid = pim4328_id; mid->name[0]; mid++) {
+		if (!strncasecmp(mid->name, device_id, strlen(mid->name)))
+			break;
+	}
+	if (!mid->name[0]) {
+		dev_err(&client->dev, "Unsupported device\n");
+		return -ENODEV;
+	}
+
+	if (strcmp(client->name, mid->name) != 0)
+		dev_notice(&client->dev,
+			   "Device mismatch: Configured %s, detected %s\n",
+			   client->name, mid->name);
+
+	data->id = mid->driver_data;
+
+	if (data->id == pim4328 || data->id == pim4820)
+		if (!i2c_check_functionality(client->adapter,
+					     I2C_FUNC_SMBUS_BLOCK_PROC_CALL))
+			return -ENODEV;
+
+	info = &data->info;
+	info->pages = 1;
+	info->read_byte_data = pim4328_read_byte_data;
+	info->read_word_data = pim4328_read_word_data;
+
+	switch (data->id) {
+	case pim4006:
+		info->phases[0] = 2;
+		info->func[0] = PMBUS_PHASE_VIRTUAL | PMBUS_HAVE_VIN
+			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
+		info->pfunc[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
+		info->pfunc[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
+		break;
+	case pim4328:
+		info->phases[0] = 2;
+		info->func[0] = PMBUS_PHASE_VIRTUAL
+			| PMBUS_HAVE_VCAP | PMBUS_HAVE_VIN
+			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
+		info->pfunc[0] = PMBUS_HAVE_VIN;
+		info->pfunc[1] = PMBUS_HAVE_VIN;
+		info->format[PSC_VOLTAGE_IN] = direct;
+		info->format[PSC_VOLTAGE_OUT] = direct;
+		info->format[PSC_TEMPERATURE] = direct;
+		info->format[PSC_CURRENT_OUT] = direct;
+		break;
+	case pim4820:
+		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_TEMP
+			| PMBUS_HAVE_IIN;
+		info->format[PSC_VOLTAGE_IN] = direct;
+		info->format[PSC_TEMPERATURE] = direct;
+		info->format[PSC_CURRENT_IN] = direct;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	if (info->func[0] & PMBUS_HAVE_VCAP &&
+	    info->format[PSC_VOLTAGE_OUT] == direct) {
+		status = pmbus_read_coefficients(client, info,
+						 PSC_VOLTAGE_OUT,
+						 PMBUS_READ_VCAP,
+						 true);
+		if (status < 0) {
+			dev_err(&client->dev,
+				"Failed to read coefficients for PMBUS_READ_VCAP\n");
+			return status;
+		}
+	}
+	if (info->func[0] & PMBUS_HAVE_VIN &&
+	    info->format[PSC_VOLTAGE_IN] == direct) {
+		status = pmbus_read_coefficients(client, info,
+						 PSC_VOLTAGE_IN,
+						 PMBUS_READ_VIN,
+						 true);
+		if (status < 0) {
+			dev_err(&client->dev,
+				"Failed to read coefficients for PMBUS_READ_VIN\n");
+			return status;
+		}
+	}
+	if (info->func[0] & PMBUS_HAVE_IIN &&
+	    info->format[PSC_CURRENT_IN] == direct) {
+		status = pmbus_read_coefficients(client, info,
+						 PSC_CURRENT_IN,
+						 PMBUS_READ_IIN,
+						 true);
+		if (status < 0) {
+			dev_err(&client->dev,
+				"Failed to read coefficients for PMBUS_READ_IIN\n");
+			return status;
+		}
+	}
+	if (info->func[0] & PMBUS_HAVE_IOUT &&
+	    info->format[PSC_CURRENT_OUT] == direct) {
+		status = pmbus_read_coefficients(client, info,
+						 PSC_CURRENT_OUT,
+						 PMBUS_READ_IOUT,
+						 true);
+		if (status < 0) {
+			dev_err(&client->dev,
+				"Failed to read coefficients for PMBUS_READ_IOUT\n");
+			return status;
+		}
+	}
+	if (info->func[0] & PMBUS_HAVE_TEMP &&
+	    info->format[PSC_TEMPERATURE] == direct) {
+		status = pmbus_read_coefficients(client, info,
+						 PSC_TEMPERATURE,
+						 PMBUS_READ_TEMPERATURE_1,
+						 true);
+		if (status < 0) {
+			dev_err(&client->dev,
+				"Failed to read coefficients for PMBUS_READ_TEMPERATURE_1\n");
+			return status;
+		}
+	}
+
+	pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
+			     GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->flags = PMBUS_NO_CAPABILITY | PMBUS_NO_WRITE_PROTECT;
+	dev->platform_data = pdata;
+
+	return pmbus_do_probe(client, info);
+}
+
+static struct i2c_driver pim4328_driver = {
+	.driver = {
+		   .name = "pim4328",
+		   },
+	.probe_new = pim4328_probe,
+	.id_table = pim4328_id,
+};
+
+module_i2c_driver(pim4328_driver);
+
+MODULE_AUTHOR("Erik Rosen <erik.rosen@metormote.com>");
+MODULE_DESCRIPTION("PMBus driver for PIM4006, PIM4328, PIM4820 power interface modules");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(PMBUS);
-- 
2.20.1

[PATCH 5/5] hwmon: (pmbus/pim4328) Add documentation for the pim4328 PMBus driver

[Erik Rosen]

Add documentation and index link for pim4328 PMBus driver.

Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
---
 Documentation/hwmon/index.rst   |   1 +
 Documentation/hwmon/pim4328.rst | 105 ++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   7 +++
 3 files changed, 113 insertions(+)
 create mode 100644 Documentation/hwmon/pim4328.rst

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 9ed60fa84cbe..719625f8f755 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -150,6 +150,7 @@ Hardware Monitoring Kernel Drivers
    pc87360
    pc87427
    pcf8591
+   pim4328
    pm6764tr
    pmbus
    powr1220
diff --git a/Documentation/hwmon/pim4328.rst b/Documentation/hwmon/pim4328.rst
new file mode 100644
index 000000000000..70c9e7a6882c
--- /dev/null
+++ b/Documentation/hwmon/pim4328.rst
@@ -0,0 +1,105 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver pim4328
+=====================
+
+Supported chips:
+
+  * Flex PIM4328
+
+    Prefix: 'pim4328', 'bmr455'
+
+    Addresses scanned: -
+
+    Datasheet:
+
+https://flexpowermodules.com/resources/fpm-techspec-pim4328
+
+  * Flex PIM4820
+
+    Prefixes: 'pim4820'
+
+    Addresses scanned: -
+
+    Datasheet: https://flexpowermodules.com/resources/fpm-techspec-pim4820
+
+  * Flex PIM4006, PIM4106, PIM4206, PIM4306, PIM4406
+
+    Prefixes: 'pim4006', 'pim4106', 'pim4206', 'pim4306', 'pim4406'
+
+    Addresses scanned: -
+
+    Datasheet: https://flexpowermodules.com/resources/fpm-techspec-pim4006
+
+Author: Erik Rosen <erik.rosen@metormote.com>
+
+
+Description
+-----------
+
+This driver supports hardware monitoring for Flex PIM4328 and
+compatible digital power interface modules.
+
+The driver is a client driver to the core PMBus driver. Please see
+Documentation/hwmon/pmbus.rst and Documentation.hwmon/pmbus-core 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.
+
+
+Platform data support
+---------------------
+
+The driver supports standard PMBus driver platform data.
+
+
+Sysfs entries
+-------------
+
+The following attributes are supported. All attributes are read-only.
+
+======================= ========================================================
+in1_label		"vin"
+in1_input		Measured input voltage.
+in1_alarm		Input voltage alarm.
+
+in2_label		"vin.0"
+in2_input		Measured input voltage on input A.
+
+			PIM4328 and PIM4X06
+
+in3_label		"vin.1"
+in3_input		Measured input voltage on input B.
+
+			PIM4328 and PIM4X06
+
+in4_label		"vcap"
+in4_input		Measured voltage on holdup capacitor.
+
+			PIM4328
+
+curr1_label		"iin.0"
+curr1_input		Measured input current on input A.
+
+			PIM4X06
+
+curr2_label		"iin.1"
+curr2_input		Measured input current on input B.
+
+			PIM4X06
+
+currX_label		"iout1"
+currX_input		Measured output current.
+currX_alarm		Output current alarm.
+
+			X is 1 for PIM4820, 3 otherwise.
+
+temp1_input		Measured temperature.
+temp1_alarm		High temperature alarm.
+======================= ========================================================
diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c120f..378a121d80f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14416,6 +14416,13 @@ K:	(?i)pidfd
 K:	(?i)clone3
 K:	\b(clone_args|kernel_clone_args)\b
 
+PIM4328 DRIVER
+M:	Daniel Nilsson <daniel.nilsson@flex.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/pim4328.rst
+F:	drivers/hwmon/pmbus/pim4328.c
+
 PIN CONTROL SUBSYSTEM
 M:	Linus Walleij <linus.walleij@linaro.org>
 L:	linux-gpio@vger.kernel.org
-- 
2.20.1

Re: [PATCH 1/5] hwmon: (pmbus/pim4328) Add function for reading direct mode coefficients

[Guenter Roeck]

On 5/19/21 1:10 PM, Erik Rosen wrote:
> Add the function pmbus_read_coefficients to pmbus_core to be able to
> read and decode the coefficients for the direct format for a certain
> command and read/write direction.
> 
> Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
> ---
>   drivers/hwmon/pmbus/pmbus.h      |  4 ++++
>   drivers/hwmon/pmbus/pmbus_core.c | 38 ++++++++++++++++++++++++++++++++
>   2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index 3968924f8533..a131b253ebf9 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -499,6 +499,10 @@ int pmbus_get_fan_rate_cached(struct i2c_client *client, int page, int id,
>   			      enum pmbus_fan_mode mode);
>   int pmbus_update_fan(struct i2c_client *client, int page, int id,
>   		     u8 config, u8 mask, u16 command);
> +int pmbus_read_coefficients(struct i2c_client *client,
> +			    struct pmbus_driver_info *info,
> +			    enum pmbus_sensor_classes sensor_class,
> +			    u8 command, bool for_reading);
>   struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
>   
>   #endif /* PMBUS_H */
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index bbd745178147..14d3d3352aac 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -301,6 +301,44 @@ int pmbus_update_fan(struct i2c_client *client, int page, int id,
>   }
>   EXPORT_SYMBOL_NS_GPL(pmbus_update_fan, PMBUS);
>   
> +/*
> + * Read the coefficients for direct mode.
> + */
> +int pmbus_read_coefficients(struct i2c_client *client,
> +			    struct pmbus_driver_info *info,
> +			    enum pmbus_sensor_classes sensor_class,
> +			    u8 command, bool for_reading)
> +{

The way the code is written, we never need to read coefficients for writing.
With that in mind, I don't think the 'for_reading' parameter is necessary.

Guenter

> +	int rv;
> +	union i2c_smbus_data data;
> +	s8 R;
> +	s16 m, b;
> +
> +	data.block[0] = 2;
> +	data.block[1] = command;
> +	data.block[2] = for_reading ? 0x01 : 0x00;
> +
> +	rv = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> +			    I2C_SMBUS_WRITE, PMBUS_COEFFICIENTS,
> +			    I2C_SMBUS_BLOCK_PROC_CALL, &data);
> +
> +	if (rv < 0)
> +		return rv;
> +
> +	if (data.block[0] != 5)
> +		return -EIO;
> +
> +	m = data.block[1] | (data.block[2] << 8);
> +	b = data.block[3] | (data.block[4] << 8);
> +	R = data.block[5];
> +	info->m[sensor_class] = m;
> +	info->b[sensor_class] = b;
> +	info->R[sensor_class] = R;
> +
> +	return rv;
> +}
> +EXPORT_SYMBOL_GPL(pmbus_read_coefficients);

EXPORT_SYMBOL_NS_GPL(pmbus_read_coefficients, PMBUS);

> +
>   int pmbus_read_word_data(struct i2c_client *client, int page, int phase, u8 reg)
>   {
>   	int rv;
> 

Re: [PATCH 4/5] hwmon: (pmbus/pim4328) Add PMBus driver for PIM4006, PIM4328 and PIM4820

[Guenter Roeck]

On 5/19/21 1:10 PM, Erik Rosen wrote:
> Add hardware monitoring support for Flex power interface modules PIM4006,
> PIM4328 and PIM4820.
> 
> Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
> ---
>   drivers/hwmon/pmbus/Kconfig   |   9 +
>   drivers/hwmon/pmbus/Makefile  |   1 +
>   drivers/hwmon/pmbus/pim4328.c | 310 ++++++++++++++++++++++++++++++++++
>   3 files changed, 320 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/pim4328.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 37a5c39784fa..001527c71269 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -257,6 +257,15 @@ config SENSORS_MP2975
>   	  This driver can also be built as a module. If so, the module will
>   	  be called mp2975.
>   
> +config SENSORS_PIM4328
> +	tristate "Flex PIM4328 and compatibles"
> +	help
> +	  If you say yes here you get hardware monitoring support for Flex
> +	  PIM4328, PIM4820 and PIM4006 Power Interface Modules.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called pim4328.
> +
>   config SENSORS_PM6764TR
>   	tristate "ST PM6764TR"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index f8dcc27cd56a..2a12397535ba 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
>   obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
>   obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
>   obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
> +obj-$(CONFIG_SENSORS_PIM4328)   += pim4328.o
> diff --git a/drivers/hwmon/pmbus/pim4328.c b/drivers/hwmon/pmbus/pim4328.c
> new file mode 100644
> index 000000000000..b9aa4f76f6cd
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pim4328.c
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for PIM4006, PIM4328 and PIM4820
> + *
> + * Copyright (c) 2021 Flextronics International Sweden AB
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/pmbus.h>

Alphabetic include file order, please.

> +#include "pmbus.h"
> +
> +enum chips { pim4006, pim4328, pim4820 };
> +
> +struct pim4328_data {
> +	enum chips id;
> +	struct pmbus_driver_info info;
> +};
> +
> +#define to_pim4328_data(x)  container_of(x, struct pim4328_data, info)
> +
> +/* PIM4006 and PIM4328 */
> +#define PIM4328_MFR_READ_VINA		0xd3
> +#define PIM4328_MFR_READ_VINB		0xd4
> +
> +/* PIM4006 */
> +#define PIM4328_MFR_READ_IINA		0xd6
> +#define PIM4328_MFR_READ_IINB		0xd7
> +#define PIM4328_MFR_FET_CHECKSTATUS     0xd9
> +
> +/* PIM4328 */
> +#define PIM4328_MFR_STATUS_BITS		0xd5
> +
> +/* PIM4820 */
> +#define PIM4328_MFR_READ_STATUS		0xd0
> +
> +static const struct i2c_device_id pim4328_id[] = {
> +	{"bmr455", pim4328},
> +	{"pim4006", pim4006},
> +	{"pim4106", pim4006},
> +	{"pim4206", pim4006},
> +	{"pim4306", pim4006},
> +	{"pim4328", pim4328},
> +	{"pim4406", pim4006},
> +	{"pim4820", pim4820},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, pim4328_id);
> +
> +static int pim4328_read_word_data(struct i2c_client *client, int page,
> +				  int phase, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct pim4328_data *data = to_pim4328_data(info);
> +	int ret, status;
> +
> +	if (page > 0)
> +		return -ENXIO;
> +
> +	switch (reg) {
> +	case PMBUS_STATUS_WORD:
> +		ret = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
> +		if (ret >= 0) {
> +			if (data->id == pim4006) {
> +				status = pmbus_read_word_data(client, page, 0xff,
> +							      PIM4328_MFR_FET_CHECKSTATUS);
> +				if (status > 0) {
> +					if (status & 0x0630) /* Input UV */
> +						ret |= 0x08;
> +				}
> +			} else if (data->id == pim4328) {
> +				status = pmbus_read_byte_data(client, page,
> +							      PIM4328_MFR_STATUS_BITS);
> +				if (status > 0) {
> +					if (status & 0x04) /* Input UV */
> +						ret |= 0x08;
> +					if (status & 0x40) /* Output UV */
> +						ret |= 0x80;
> +				}
> +			} else if (data->id == pim4820) {
> +				status = pmbus_read_byte_data(client, page,
> +							      PIM4328_MFR_READ_STATUS);
> +				if (status > 0) {
> +					if (status & 0x05) /* Input OV or OC */
> +						ret |= 0x2001;
> +					if (status & 0x1a) /* Input UV */
> +						ret |= 0x2008;
> +					if (status & 0x40) /* OT */
> +						ret |= 0x0004;
> +				}
> +			}
> +		}
> +		break;
> +	case PMBUS_READ_VIN:
> +		if (phase != 0xff) {
> +			ret = pmbus_read_word_data(client, page, phase,
> +						   phase == 0 ? PIM4328_MFR_READ_VINA
> +							      : PIM4328_MFR_READ_VINB);
> +		} else {
> +			ret = -ENODATA;
> +		}
> +		break;
> +	case PMBUS_READ_IIN:
> +		if (phase != 0xff) {
> +			ret = pmbus_read_word_data(client, page, phase,
> +						   phase == 0 ? PIM4328_MFR_READ_IINA
> +							      : PIM4328_MFR_READ_IINB);
> +		} else {
> +			ret = -ENODATA;
> +		}
> +		break;
> +	default:
> +		ret = -ENODATA;
> +	}
> +
> +	return ret;
> +}
> +
> +static int pim4328_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +	int ret;
> +
> +	if (page > 0)
> +		return -ENXIO;
> +
> +	switch (reg) {
> +	case PMBUS_STATUS_BYTE:
> +		ret = pim4328_read_word_data(client, page, 0xff, PMBUS_STATUS_WORD);
> +		if (ret > 0)
> +			ret &= 0xff;
> +		break;

Doesn't the core take care of that ?

> +	default:
> +		ret = -ENODATA;
> +	}
> +
> +	return ret;
> +}
> +
> +static int pim4328_probe(struct i2c_client *client)
> +{
> +	int status;
> +	u8 device_id[I2C_SMBUS_BLOCK_MAX + 1];
> +	const struct i2c_device_id *mid;
> +	struct pim4328_data *data;
> +	struct pmbus_driver_info *info;
> +	struct pmbus_platform_data *pdata;
> +	struct device *dev = &client->dev;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> +				     | I2C_FUNC_SMBUS_BLOCK_DATA))
> +		return -ENODEV;
> +
> +	data = devm_kzalloc(&client->dev, sizeof(struct pim4328_data),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	status = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, device_id);
> +	if (status < 0) {
> +		dev_err(&client->dev, "Failed to read Manufacturer Model\n");
> +		return status;
> +	}
> +	for (mid = pim4328_id; mid->name[0]; mid++) {
> +		if (!strncasecmp(mid->name, device_id, strlen(mid->name)))
> +			break;
> +	}
> +	if (!mid->name[0]) {
> +		dev_err(&client->dev, "Unsupported device\n");
> +		return -ENODEV;
> +	}
> +
> +	if (strcmp(client->name, mid->name) != 0)
> +		dev_notice(&client->dev,
> +			   "Device mismatch: Configured %s, detected %s\n",
> +			   client->name, mid->name);
> +
> +	data->id = mid->driver_data;
> +
> +	if (data->id == pim4328 || data->id == pim4820)
> +		if (!i2c_check_functionality(client->adapter,
> +					     I2C_FUNC_SMBUS_BLOCK_PROC_CALL))
> +			return -ENODEV;
> +
> +	info = &data->info;
> +	info->pages = 1;
> +	info->read_byte_data = pim4328_read_byte_data;
> +	info->read_word_data = pim4328_read_word_data;
> +
> +	switch (data->id) {
> +	case pim4006:
> +		info->phases[0] = 2;
> +		info->func[0] = PMBUS_PHASE_VIRTUAL | PMBUS_HAVE_VIN
> +			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
> +		info->pfunc[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> +		info->pfunc[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> +		break;
> +	case pim4328:
> +		info->phases[0] = 2;
> +		info->func[0] = PMBUS_PHASE_VIRTUAL
> +			| PMBUS_HAVE_VCAP | PMBUS_HAVE_VIN
> +			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
> +		info->pfunc[0] = PMBUS_HAVE_VIN;
> +		info->pfunc[1] = PMBUS_HAVE_VIN;
> +		info->format[PSC_VOLTAGE_IN] = direct;
> +		info->format[PSC_VOLTAGE_OUT] = direct;
> +		info->format[PSC_TEMPERATURE] = direct;
> +		info->format[PSC_CURRENT_OUT] = direct;
> +		break;
> +	case pim4820:
> +		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_TEMP
> +			| PMBUS_HAVE_IIN;
> +		info->format[PSC_VOLTAGE_IN] = direct;
> +		info->format[PSC_TEMPERATURE] = direct;
> +		info->format[PSC_CURRENT_IN] = direct;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	if (info->func[0] & PMBUS_HAVE_VCAP &&
> +	    info->format[PSC_VOLTAGE_OUT] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_VOLTAGE_OUT,
> +						 PMBUS_READ_VCAP,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_VCAP\n");
> +			return status;
> +		}
> +	}
> +	if (info->func[0] & PMBUS_HAVE_VIN &&
> +	    info->format[PSC_VOLTAGE_IN] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_VOLTAGE_IN,
> +						 PMBUS_READ_VIN,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_VIN\n");
> +			return status;
> +		}
> +	}
> +	if (info->func[0] & PMBUS_HAVE_IIN &&
> +	    info->format[PSC_CURRENT_IN] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_CURRENT_IN,
> +						 PMBUS_READ_IIN,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_IIN\n");
> +			return status;
> +		}
> +	}
> +	if (info->func[0] & PMBUS_HAVE_IOUT &&
> +	    info->format[PSC_CURRENT_OUT] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_CURRENT_OUT,
> +						 PMBUS_READ_IOUT,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_IOUT\n");
> +			return status;
> +		}
> +	}
> +	if (info->func[0] & PMBUS_HAVE_TEMP &&
> +	    info->format[PSC_TEMPERATURE] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_TEMPERATURE,
> +						 PMBUS_READ_TEMPERATURE_1,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_TEMPERATURE_1\n");
> +			return status;
> +		}
> +	}
> +
> +	pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
> +			     GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	pdata->flags = PMBUS_NO_CAPABILITY | PMBUS_NO_WRITE_PROTECT;
> +	dev->platform_data = pdata;
> +
> +	return pmbus_do_probe(client, info);
> +}
> +
> +static struct i2c_driver pim4328_driver = {
> +	.driver = {
> +		   .name = "pim4328",
> +		   },
> +	.probe_new = pim4328_probe,
> +	.id_table = pim4328_id,
> +};
> +
> +module_i2c_driver(pim4328_driver);
> +
> +MODULE_AUTHOR("Erik Rosen <erik.rosen@metormote.com>");
> +MODULE_DESCRIPTION("PMBus driver for PIM4006, PIM4328, PIM4820 power interface modules");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);
> 

Re: [PATCH 4/5] hwmon: (pmbus/pim4328) Add PMBus driver for PIM4006, PIM4328 and PIM4820

[Guenter Roeck]

On 5/19/21 1:10 PM, Erik Rosen wrote:
> Add hardware monitoring support for Flex power interface modules PIM4006,
> PIM4328 and PIM4820.
> 
> Signed-off-by: Erik Rosen <erik.rosen@metormote.com>
> ---
>   drivers/hwmon/pmbus/Kconfig   |   9 +
>   drivers/hwmon/pmbus/Makefile  |   1 +
>   drivers/hwmon/pmbus/pim4328.c | 310 ++++++++++++++++++++++++++++++++++
>   3 files changed, 320 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/pim4328.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 37a5c39784fa..001527c71269 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -257,6 +257,15 @@ config SENSORS_MP2975
>   	  This driver can also be built as a module. If so, the module will
>   	  be called mp2975.
>   
> +config SENSORS_PIM4328
> +	tristate "Flex PIM4328 and compatibles"
> +	help
> +	  If you say yes here you get hardware monitoring support for Flex
> +	  PIM4328, PIM4820 and PIM4006 Power Interface Modules.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called pim4328.
> +
>   config SENSORS_PM6764TR
>   	tristate "ST PM6764TR"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index f8dcc27cd56a..2a12397535ba 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
>   obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
>   obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
>   obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
> +obj-$(CONFIG_SENSORS_PIM4328)   += pim4328.o
> diff --git a/drivers/hwmon/pmbus/pim4328.c b/drivers/hwmon/pmbus/pim4328.c
> new file mode 100644
> index 000000000000..b9aa4f76f6cd
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pim4328.c
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for PIM4006, PIM4328 and PIM4820
> + *
> + * Copyright (c) 2021 Flextronics International Sweden AB
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/pmbus.h>
> +#include "pmbus.h"
> +
> +enum chips { pim4006, pim4328, pim4820 };
> +
> +struct pim4328_data {
> +	enum chips id;
> +	struct pmbus_driver_info info;
> +};
> +
> +#define to_pim4328_data(x)  container_of(x, struct pim4328_data, info)
> +
> +/* PIM4006 and PIM4328 */
> +#define PIM4328_MFR_READ_VINA		0xd3
> +#define PIM4328_MFR_READ_VINB		0xd4
> +
> +/* PIM4006 */
> +#define PIM4328_MFR_READ_IINA		0xd6
> +#define PIM4328_MFR_READ_IINB		0xd7
> +#define PIM4328_MFR_FET_CHECKSTATUS     0xd9
> +
> +/* PIM4328 */
> +#define PIM4328_MFR_STATUS_BITS		0xd5
> +
> +/* PIM4820 */
> +#define PIM4328_MFR_READ_STATUS		0xd0
> +
> +static const struct i2c_device_id pim4328_id[] = {
> +	{"bmr455", pim4328},
> +	{"pim4006", pim4006},
> +	{"pim4106", pim4006},
> +	{"pim4206", pim4006},
> +	{"pim4306", pim4006},
> +	{"pim4328", pim4328},
> +	{"pim4406", pim4006},
> +	{"pim4820", pim4820},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, pim4328_id);
> +
> +static int pim4328_read_word_data(struct i2c_client *client, int page,
> +				  int phase, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct pim4328_data *data = to_pim4328_data(info);
> +	int ret, status;
> +
> +	if (page > 0)
> +		return -ENXIO;
> +
> +	switch (reg) {
> +	case PMBUS_STATUS_WORD:
> +		ret = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
> +		if (ret >= 0) {
> +			if (data->id == pim4006) {
> +				status = pmbus_read_word_data(client, page, 0xff,
> +							      PIM4328_MFR_FET_CHECKSTATUS);
> +				if (status > 0) {
> +					if (status & 0x0630) /* Input UV */
> +						ret |= 0x08;
> +				}
> +			} else if (data->id == pim4328) {
> +				status = pmbus_read_byte_data(client, page,
> +							      PIM4328_MFR_STATUS_BITS);
> +				if (status > 0) {
> +					if (status & 0x04) /* Input UV */
> +						ret |= 0x08;
> +					if (status & 0x40) /* Output UV */
> +						ret |= 0x80;
> +				}
> +			} else if (data->id == pim4820) {
> +				status = pmbus_read_byte_data(client, page,
> +							      PIM4328_MFR_READ_STATUS);
> +				if (status > 0) {
> +					if (status & 0x05) /* Input OV or OC */
> +						ret |= 0x2001;
> +					if (status & 0x1a) /* Input UV */
> +						ret |= 0x2008;
> +					if (status & 0x40) /* OT */
> +						ret |= 0x0004;
> +				}
> +			}
> +		}
> +		break;
> +	case PMBUS_READ_VIN:
> +		if (phase != 0xff) {
> +			ret = pmbus_read_word_data(client, page, phase,
> +						   phase == 0 ? PIM4328_MFR_READ_VINA
> +							      : PIM4328_MFR_READ_VINB);
> +		} else {
> +			ret = -ENODATA;
> +		}
> +		break;
> +	case PMBUS_READ_IIN:
> +		if (phase != 0xff) {
> +			ret = pmbus_read_word_data(client, page, phase,
> +						   phase == 0 ? PIM4328_MFR_READ_IINA
> +							      : PIM4328_MFR_READ_IINB);
> +		} else {
> +			ret = -ENODATA;
> +		}
> +		break;
> +	default:
> +		ret = -ENODATA;
> +	}
> +
> +	return ret;
> +}
> +
> +static int pim4328_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +	int ret;
> +
> +	if (page > 0)
> +		return -ENXIO;
> +
> +	switch (reg) {
> +	case PMBUS_STATUS_BYTE:
> +		ret = pim4328_read_word_data(client, page, 0xff, PMBUS_STATUS_WORD);
> +		if (ret > 0)
> +			ret &= 0xff;
> +		break;
> +	default:
> +		ret = -ENODATA;
> +	}
> +
> +	return ret;
> +}
> +
> +static int pim4328_probe(struct i2c_client *client)
> +{
> +	int status;
> +	u8 device_id[I2C_SMBUS_BLOCK_MAX + 1];
> +	const struct i2c_device_id *mid;
> +	struct pim4328_data *data;
> +	struct pmbus_driver_info *info;
> +	struct pmbus_platform_data *pdata;
> +	struct device *dev = &client->dev;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> +				     | I2C_FUNC_SMBUS_BLOCK_DATA))
> +		return -ENODEV;
> +
> +	data = devm_kzalloc(&client->dev, sizeof(struct pim4328_data),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	status = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, device_id);
> +	if (status < 0) {
> +		dev_err(&client->dev, "Failed to read Manufacturer Model\n");
> +		return status;
> +	}
> +	for (mid = pim4328_id; mid->name[0]; mid++) {
> +		if (!strncasecmp(mid->name, device_id, strlen(mid->name)))
> +			break;
> +	}
> +	if (!mid->name[0]) {
> +		dev_err(&client->dev, "Unsupported device\n");
> +		return -ENODEV;
> +	}
> +
> +	if (strcmp(client->name, mid->name) != 0)
> +		dev_notice(&client->dev,
> +			   "Device mismatch: Configured %s, detected %s\n",
> +			   client->name, mid->name);
> +
> +	data->id = mid->driver_data;
> +
> +	if (data->id == pim4328 || data->id == pim4820)
> +		if (!i2c_check_functionality(client->adapter,
> +					     I2C_FUNC_SMBUS_BLOCK_PROC_CALL))
> +			return -ENODEV;
> +
> +	info = &data->info;
> +	info->pages = 1;
> +	info->read_byte_data = pim4328_read_byte_data;
> +	info->read_word_data = pim4328_read_word_data;
> +
> +	switch (data->id) {
> +	case pim4006:
> +		info->phases[0] = 2;
> +		info->func[0] = PMBUS_PHASE_VIRTUAL | PMBUS_HAVE_VIN
> +			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
> +		info->pfunc[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> +		info->pfunc[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> +		break;
> +	case pim4328:
> +		info->phases[0] = 2;
> +		info->func[0] = PMBUS_PHASE_VIRTUAL
> +			| PMBUS_HAVE_VCAP | PMBUS_HAVE_VIN
> +			| PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT;
> +		info->pfunc[0] = PMBUS_HAVE_VIN;
> +		info->pfunc[1] = PMBUS_HAVE_VIN;
> +		info->format[PSC_VOLTAGE_IN] = direct;
> +		info->format[PSC_VOLTAGE_OUT] = direct;
> +		info->format[PSC_TEMPERATURE] = direct;
> +		info->format[PSC_CURRENT_OUT] = direct;
> +		break;
> +	case pim4820:
> +		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_TEMP
> +			| PMBUS_HAVE_IIN;
> +		info->format[PSC_VOLTAGE_IN] = direct;
> +		info->format[PSC_TEMPERATURE] = direct;
> +		info->format[PSC_CURRENT_IN] = direct;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	if (info->func[0] & PMBUS_HAVE_VCAP &&
> +	    info->format[PSC_VOLTAGE_OUT] == direct) {
> +		status = pmbus_read_coefficients(client, info,
> +						 PSC_VOLTAGE_OUT,
> +						 PMBUS_READ_VCAP,
> +						 true);
> +		if (status < 0) {
> +			dev_err(&client->dev,
> +				"Failed to read coefficients for PMBUS_READ_VCAP\n");
> +			return status;
> +		}
> +	}

Is there reason to implement this all here, or could we simply add another flag
such as PMBUS_HAS_COEFFICIENTS_CMD and have the core read the coefficients as
needed ?

I'd rather have this implemented in the core instead of having to carry
similar code in all drivers supporting the coefficients command.

Thanks,
Guenter