[PATCH v2 0/2] Add pmbus support for Infineon IRPS5401 (v2)

[PATCH v2 0/2] Add pmbus support for Infineon IRPS5401 (v2)

[Robert Hancock]

This patch set adds support for the Infineon IRPS5401 PMIC to pmbus.
This chip has 5 outputs and includes separate VIN, IIN and PIN parameters
on multiple pages, which is something that the pmbus core did not
previously support properly.

Changes since v1:
-Changed to treat parameters as paged if they are on multiple pages, rather
than treating VIN, IIN and PIN as paged unconditionally
-Create separate driver for IRPS5401 to avoid potential impact on existing
devices from changing auto-probing

Robert Hancock (2):
  hwmon: (pmbus) Treat parameters as paged if on multiple pages
  hwmon: (pmbus) Add Infineon IRPS5401 driver

 drivers/hwmon/pmbus/Kconfig      |  9 ++++++
 drivers/hwmon/pmbus/Makefile     |  1 +
 drivers/hwmon/pmbus/irps5401.c   | 68 ++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus_core.c | 33 ++++++++++++++++---
 4 files changed, 107 insertions(+), 4 deletions(-)
 create mode 100644 drivers/hwmon/pmbus/irps5401.c

-- 
1.8.3.1

[PATCH v2 1/2] hwmon: (pmbus) Treat parameters as paged if on multiple pages

[Robert Hancock]

Some chips have attributes which exist on more than one page but the
attribute is not presently marked as paged. This causes the attributes
to be generated with the same label, which makes it impossible for
userspace to tell them apart.

Marking all such attributes as paged would result in the page suffix
being added regardless of whether they were present on more than one
page or not, which might break existing setups. Therefore, we add a
second check which treats the attribute as paged, even if not marked as
such, if it is present on multiple pages.

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 drivers/hwmon/pmbus/pmbus_core.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index ef7ee90..49dcb84 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1217,7 +1217,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
 				      const struct pmbus_driver_info *info,
 				      const char *name,
 				      int index, int page,
-				      const struct pmbus_sensor_attr *attr)
+				      const struct pmbus_sensor_attr *attr,
+				      bool paged)
 {
 	struct pmbus_sensor *base;
 	bool upper = !!(attr->gbit & 0xff00);	/* need to check STATUS_WORD */
@@ -1225,7 +1226,7 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
 
 	if (attr->label) {
 		ret = pmbus_add_label(data, name, index, attr->label,
-				      attr->paged ? page + 1 : 0);
+				      paged ? page + 1 : 0);
 		if (ret)
 			return ret;
 	}
@@ -1258,6 +1259,29 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
 	return 0;
 }
 
+static bool pmbus_sensor_is_paged(const struct pmbus_driver_info *info,
+				  const struct pmbus_sensor_attr *attr)
+{
+	int p;
+
+	if (attr->paged)
+		return true;
+
+	/* Some attributes may be present on more than one page despite
+	 * not being marked with the paged attribute. If that is the case,
+	 * then treat the sensor as being paged and add the page suffix to the
+	 * attribute name.
+	 * We don't just add the paged attribute to all such attributes, in
+	 * order to maintain the un-suffixed labels in the case where the
+	 * attribute is only on page 0.
+	 */
+	for (p = 1; p < info->pages; p++) {
+		if (info->func[p] & attr->func)
+			return true;
+	}
+	return false;
+}
+
 static int pmbus_add_sensor_attrs(struct i2c_client *client,
 				  struct pmbus_data *data,
 				  const char *name,
@@ -1271,14 +1295,15 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client,
 	index = 1;
 	for (i = 0; i < nattrs; i++) {
 		int page, pages;
+		bool paged = pmbus_sensor_is_paged(info, attrs);
 
-		pages = attrs->paged ? info->pages : 1;
+		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,
-							 attrs);
+							 attrs, paged);
 			if (ret)
 				return ret;
 			index++;
-- 
1.8.3.1

[PATCH v2 2/2] hwmon: (pmbus) Add Infineon IRPS5401 driver

[Robert Hancock]

Add a driver to support the Infineon IRPS5401 PMIC. This chip has 5
pages corresponding to 4 switching outputs and one linear (LDO) output.
The switching and LDO outputs have slightly different supported
parameters.

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 drivers/hwmon/pmbus/Kconfig    |  9 ++++++
 drivers/hwmon/pmbus/Makefile   |  1 +
 drivers/hwmon/pmbus/irps5401.c | 68 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/irps5401.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 30751eb..8eb5bb4 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -64,6 +64,15 @@ config SENSORS_IR38064
 	  This driver can also be built as a module. If so, the module will
 	  be called ir38064.
 
+config SENSORS_IRPS5401
+	tristate "Infineon IRPS5401"
+	help
+	  If you say yes here you get hardware monitoring support for the
+	  Infineon IRPS5401 controller.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called irps5401.
+
 config SENSORS_ISL68137
 	tristate "Intersil ISL68137"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 2219b93..e4a7dd0 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
 obj-$(CONFIG_SENSORS_IBM_CFFPS)	+= ibm-cffps.o
 obj-$(CONFIG_SENSORS_IR35221)	+= ir35221.o
 obj-$(CONFIG_SENSORS_IR38064)	+= ir38064.o
+obj-$(CONFIG_SENSORS_IRPS5401)	+= irps5401.o
 obj-$(CONFIG_SENSORS_ISL68137)	+= isl68137.o
 obj-$(CONFIG_SENSORS_LM25066)	+= lm25066.o
 obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
diff --git a/drivers/hwmon/pmbus/irps5401.c b/drivers/hwmon/pmbus/irps5401.c
new file mode 100644
index 0000000..825e9fa
--- /dev/null
+++ b/drivers/hwmon/pmbus/irps5401.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for the Infineon IRPS5401M PMIC.
+ *
+ * Copyright (c) 2019 SED Systems, a division of Calian Ltd.
+ *
+ * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
+ * this driver does not currently support them.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+#define IRPS5401_SW_FUNC (PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | \
+			  PMBUS_HAVE_STATUS_INPUT | \
+			  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \
+			  PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \
+			  PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \
+			  PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
+
+#define IRPS5401_LDO_FUNC (PMBUS_HAVE_VIN | \
+			   PMBUS_HAVE_STATUS_INPUT | \
+			   PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \
+			   PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \
+			   PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \
+			   PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
+
+static struct pmbus_driver_info irps5401_info = {
+	.pages = 5,
+	.func[0] = IRPS5401_SW_FUNC,
+	.func[1] = IRPS5401_SW_FUNC,
+	.func[2] = IRPS5401_SW_FUNC,
+	.func[3] = IRPS5401_SW_FUNC,
+	.func[4] = IRPS5401_LDO_FUNC,
+};
+
+static int irps5401_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	return pmbus_do_probe(client, id, &irps5401_info);
+}
+
+static const struct i2c_device_id irps5401_id[] = {
+	{"irps5401", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, irps5401_id);
+
+static struct i2c_driver irps5401_driver = {
+	.driver = {
+		   .name = "irps5401",
+		   },
+	.probe = irps5401_probe,
+	.remove = pmbus_do_remove,
+	.id_table = irps5401_id,
+};
+
+module_i2c_driver(irps5401_driver);
+
+MODULE_AUTHOR("Robert Hancock");
+MODULE_DESCRIPTION("PMBus driver for Infineon IRPS5401");
+MODULE_LICENSE("GPL");
+
-- 
1.8.3.1

Re: [PATCH v2 1/2] hwmon: (pmbus) Treat parameters as paged if on multiple pages

[Guenter Roeck]

On Wed, Jun 05, 2019 at 01:49:00PM -0600, Robert Hancock wrote:
> Some chips have attributes which exist on more than one page but the
> attribute is not presently marked as paged. This causes the attributes
> to be generated with the same label, which makes it impossible for
> userspace to tell them apart.
> 
> Marking all such attributes as paged would result in the page suffix
> being added regardless of whether they were present on more than one
> page or not, which might break existing setups. Therefore, we add a
> second check which treats the attribute as paged, even if not marked as
> such, if it is present on multiple pages.
> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/pmbus/pmbus_core.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index ef7ee90..49dcb84 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -1217,7 +1217,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
>  				      const struct pmbus_driver_info *info,
>  				      const char *name,
>  				      int index, int page,
> -				      const struct pmbus_sensor_attr *attr)
> +				      const struct pmbus_sensor_attr *attr,
> +				      bool paged)
>  {
>  	struct pmbus_sensor *base;
>  	bool upper = !!(attr->gbit & 0xff00);	/* need to check STATUS_WORD */
> @@ -1225,7 +1226,7 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
>  
>  	if (attr->label) {
>  		ret = pmbus_add_label(data, name, index, attr->label,
> -				      attr->paged ? page + 1 : 0);
> +				      paged ? page + 1 : 0);
>  		if (ret)
>  			return ret;
>  	}
> @@ -1258,6 +1259,29 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
>  	return 0;
>  }
>  
> +static bool pmbus_sensor_is_paged(const struct pmbus_driver_info *info,
> +				  const struct pmbus_sensor_attr *attr)
> +{
> +	int p;
> +
> +	if (attr->paged)
> +		return true;
> +
> +	/* Some attributes may be present on more than one page despite
> +	 * not being marked with the paged attribute. If that is the case,
> +	 * then treat the sensor as being paged and add the page suffix to the
> +	 * attribute name.
> +	 * We don't just add the paged attribute to all such attributes, in
> +	 * order to maintain the un-suffixed labels in the case where the
> +	 * attribute is only on page 0.
> +	 */
> +	for (p = 1; p < info->pages; p++) {
> +		if (info->func[p] & attr->func)
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static int pmbus_add_sensor_attrs(struct i2c_client *client,
>  				  struct pmbus_data *data,
>  				  const char *name,
> @@ -1271,14 +1295,15 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client,
>  	index = 1;
>  	for (i = 0; i < nattrs; i++) {
>  		int page, pages;
> +		bool paged = pmbus_sensor_is_paged(info, attrs);
>  
> -		pages = attrs->paged ? info->pages : 1;
> +		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,
> -							 attrs);
> +							 attrs, paged);
>  			if (ret)
>  				return ret;
>  			index++;

Re: [PATCH v2 2/2] hwmon: (pmbus) Add Infineon IRPS5401 driver

[Guenter Roeck]

On Wed, Jun 05, 2019 at 01:49:01PM -0600, Robert Hancock wrote:
> Add a driver to support the Infineon IRPS5401 PMIC. This chip has 5
> pages corresponding to 4 switching outputs and one linear (LDO) output.
> The switching and LDO outputs have slightly different supported
> parameters.
> 

Applied (removed empty line at end of file).

Thanks,
Guenter

> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
> ---
>  drivers/hwmon/pmbus/Kconfig    |  9 ++++++
>  drivers/hwmon/pmbus/Makefile   |  1 +
>  drivers/hwmon/pmbus/irps5401.c | 68 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
>  create mode 100644 drivers/hwmon/pmbus/irps5401.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb..8eb5bb4 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -64,6 +64,15 @@ config SENSORS_IR38064
>  	  This driver can also be built as a module. If so, the module will
>  	  be called ir38064.
>  
> +config SENSORS_IRPS5401
> +	tristate "Infineon IRPS5401"
> +	help
> +	  If you say yes here you get hardware monitoring support for the
> +	  Infineon IRPS5401 controller.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called irps5401.
> +
>  config SENSORS_ISL68137
>  	tristate "Intersil ISL68137"
>  	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b93..e4a7dd0 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
>  obj-$(CONFIG_SENSORS_IBM_CFFPS)	+= ibm-cffps.o
>  obj-$(CONFIG_SENSORS_IR35221)	+= ir35221.o
>  obj-$(CONFIG_SENSORS_IR38064)	+= ir38064.o
> +obj-$(CONFIG_SENSORS_IRPS5401)	+= irps5401.o
>  obj-$(CONFIG_SENSORS_ISL68137)	+= isl68137.o
>  obj-$(CONFIG_SENSORS_LM25066)	+= lm25066.o
>  obj-$(CONFIG_SENSORS_LTC2978)	+= ltc2978.o
> diff --git a/drivers/hwmon/pmbus/irps5401.c b/drivers/hwmon/pmbus/irps5401.c
> new file mode 100644
> index 0000000..825e9fa
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/irps5401.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for the Infineon IRPS5401M PMIC.
> + *
> + * Copyright (c) 2019 SED Systems, a division of Calian Ltd.
> + *
> + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
> + * this driver does not currently support them.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define IRPS5401_SW_FUNC (PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | \
> +			  PMBUS_HAVE_STATUS_INPUT | \
> +			  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \
> +			  PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \
> +			  PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \
> +			  PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
> +
> +#define IRPS5401_LDO_FUNC (PMBUS_HAVE_VIN | \
> +			   PMBUS_HAVE_STATUS_INPUT | \
> +			   PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \
> +			   PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \
> +			   PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | \
> +			   PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
> +
> +static struct pmbus_driver_info irps5401_info = {
> +	.pages = 5,
> +	.func[0] = IRPS5401_SW_FUNC,
> +	.func[1] = IRPS5401_SW_FUNC,
> +	.func[2] = IRPS5401_SW_FUNC,
> +	.func[3] = IRPS5401_SW_FUNC,
> +	.func[4] = IRPS5401_LDO_FUNC,
> +};
> +
> +static int irps5401_probe(struct i2c_client *client,
> +			  const struct i2c_device_id *id)
> +{
> +	return pmbus_do_probe(client, id, &irps5401_info);
> +}
> +
> +static const struct i2c_device_id irps5401_id[] = {
> +	{"irps5401", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, irps5401_id);
> +
> +static struct i2c_driver irps5401_driver = {
> +	.driver = {
> +		   .name = "irps5401",
> +		   },
> +	.probe = irps5401_probe,
> +	.remove = pmbus_do_remove,
> +	.id_table = irps5401_id,
> +};
> +
> +module_i2c_driver(irps5401_driver);
> +
> +MODULE_AUTHOR("Robert Hancock");
> +MODULE_DESCRIPTION("PMBus driver for Infineon IRPS5401");
> +MODULE_LICENSE("GPL");
> +