mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add support for Infineon Digital Multi-phase Controllers XDPE1A2G5B/7B
@ 2026-02-19 10:56 ASHISH YADAV
  2026-02-19 10:56 ` [PATCH v2 1/3] dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B ASHISH YADAV
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: ASHISH YADAV @ 2026-02-19 10:56 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel, Ashish Yadav

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1406 bytes --]

From: Ashish Yadav <ashish.yadav@infineon.com>

Hi ,

These patches add support for Infineon Digital Multi-phase Controllers 
XDPE1A2G5B and XDPE1A2G7B.
Both controller supports Linear and VID Data format for VOUT using 
VOUT_MODE command.
The configuring both loops/pages of the device independently is not 
supported for VOUT_MODE.

In case of vid mode, NVIDIA PWM VID vrm_version is supported:
Vout = 5mV * (VID-1) + 195mV

With Best Regards
   Ashish Yadav
---
Changes in v2:
- Link to v1: 
   https://lore.kernel.org/all/20260202080355.53061-1-Ashish.Yadav@infineon.com/
- Address review comments for driver documentation, pages with different
  VOUT_MODE, Indentation.  
- Created separate patch for core changes.
---

Ashish Yadav (3):
  dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B
  hwmon: (pmbus/core) Add support for NVIDIA nvidia195mv mode
  hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers

 .../devicetree/bindings/trivial-devices.yaml  |   3 +
 drivers/hwmon/pmbus/Kconfig                   |   9 ++
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/pmbus.h                   |   2 +-
 drivers/hwmon/pmbus/pmbus_core.c              |   4 +
 drivers/hwmon/pmbus/xdpe1a2g7b.c              | 120 ++++++++++++++++++
 6 files changed, 138 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwmon/pmbus/xdpe1a2g7b.c

-- 
2.39.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B
  2026-02-19 10:56 [PATCH v2 0/3] Add support for Infineon Digital Multi-phase Controllers XDPE1A2G5B/7B ASHISH YADAV
@ 2026-02-19 10:56 ` ASHISH YADAV
  2026-02-19 10:56 ` =?y?q?=5BPATCH=20v2=202/3=5D=20hwmon=3A=20=28pmbus/core=29=20Add=20support=20for=C2=A0NVIDIA=C2=A0nvidia195mv=20mode?= ASHISH YADAV
  2026-02-19 10:56 ` [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers ASHISH YADAV
  2 siblings, 0 replies; 6+ messages in thread
From: ASHISH YADAV @ 2026-02-19 10:56 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel, Ashish Yadav

From: Ashish Yadav <ashish.yadav@infineon.com>

Add Infineon Digital Multi-phase XDPE1A2G5B and XDPE1A2G7B
Controllers to trivial devices.

Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index a482aeadcd44..343f1e62c373 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -157,6 +157,9 @@ properties:
           - infineon,xdpe15284
             # Infineon Multi-phase Digital VR Controller xdpe152c4
           - infineon,xdpe152c4
+            # Infineon Multi-phase Digital VR Controller xdpe1a2g7b
+          - infineon,xdpe1a2g5b
+          - infineon,xdpe1a2g7b
             # Injoinic IP5108 2.0A Power Bank IC with I2C
           - injoinic,ip5108
             # Injoinic IP5109 2.1A Power Bank IC with I2C
-- 
2.39.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* =?y?q?=5BPATCH=20v2=202/3=5D=20hwmon=3A=20=28pmbus/core=29=20Add=20support=20for=C2=A0NVIDIA=C2=A0nvidia195mv=20mode?=
  2026-02-19 10:56 [PATCH v2 0/3] Add support for Infineon Digital Multi-phase Controllers XDPE1A2G5B/7B ASHISH YADAV
  2026-02-19 10:56 ` [PATCH v2 1/3] dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B ASHISH YADAV
@ 2026-02-19 10:56 ` ASHISH YADAV
  2026-02-19 10:56 ` [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers ASHISH YADAV
  2 siblings, 0 replies; 6+ messages in thread
From: ASHISH YADAV @ 2026-02-19 10:56 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel, Ashish Yadav

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1602 bytes --]

From: Ashish Yadav <ashish.yadav@infineon.com>

Extend the PMBus core vrm_version handling to support NVIDIA nvidia195mv
VID mode. This adds a new VRM/VID encoding type and the corresponding 
voltage conversion logic so devices reporting nvidia195mv can have their
VOUT/VID values interpreted correctly by the hwmon PMBus core.

Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
---
 drivers/hwmon/pmbus/pmbus.h      | 2 +-
 drivers/hwmon/pmbus/pmbus_core.c | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index d2e9bfb5320f..3ddcb742d289 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -416,7 +416,7 @@ enum pmbus_sensor_classes {
 #define PMBUS_PAGE_VIRTUAL	BIT(31)	/* Page is virtual */
 
 enum pmbus_data_format { linear = 0, ieee754, direct, vid };
-enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
+enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv, nvidia195mv };
 
 /* PMBus revision identifiers */
 #define PMBUS_REV_10 0x00	/* PMBus revision 1.0 */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index be6d05def115..4d7634ee6148 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -885,6 +885,10 @@ static s64 pmbus_reg2data_vid(struct pmbus_data *data,
 		if (val >= 0x0 && val <= 0xd8)
 			rv = DIV_ROUND_CLOSEST(155000 - val * 625, 100);
 		break;
+	case nvidia195mv:
+		if (val >= 0x01)
+			rv = 195 + (val - 1) * 5;  /* VID step is 5mv */
+		break;
 	}
 	return rv;
 }
-- 
2.39.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers
  2026-02-19 10:56 [PATCH v2 0/3] Add support for Infineon Digital Multi-phase Controllers XDPE1A2G5B/7B ASHISH YADAV
  2026-02-19 10:56 ` [PATCH v2 1/3] dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B ASHISH YADAV
  2026-02-19 10:56 ` =?y?q?=5BPATCH=20v2=202/3=5D=20hwmon=3A=20=28pmbus/core=29=20Add=20support=20for=C2=A0NVIDIA=C2=A0nvidia195mv=20mode?= ASHISH YADAV
@ 2026-02-19 10:56 ` ASHISH YADAV
  2026-02-21 23:21   ` Guenter Roeck
  2 siblings, 1 reply; 6+ messages in thread
From: ASHISH YADAV @ 2026-02-19 10:56 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel, Ashish Yadav

From: Ashish Yadav <ashish.yadav@infineon.com>

Add the pmbus driver for Infineon Digital Multi-phase XDPE1A2G5B and
XDPE1A2G7B controllers.

Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
---
XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID Data
formats for VOUT_MODE.
The configuring both loops/pages of the device independently is not
supported for VOUT_MODE.
In case of vid mode, NVIDIA PWM VID vrm_version is supported:
Vout = 5mV * (VID-1) + 195mV
---
 drivers/hwmon/pmbus/Kconfig      |   9 +++
 drivers/hwmon/pmbus/Makefile     |   1 +
 drivers/hwmon/pmbus/xdpe1a2g7b.c | 120 +++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/xdpe1a2g7b.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index fc1273abe357..a4513fc6bc26 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -711,6 +711,15 @@ config SENSORS_XDPE152
 	  This driver can also be built as a module. If so, the module will
 	  be called xdpe152c4.
 
+config SENSORS_XDPE1A2G7B
+	tristate "Infineon XDPE1A2G7B"
+	help
+	  If you say yes here you get hardware monitoring support for Infineon
+	  XDPE1A2G5B and XDPE1A2G7B.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called xdpe1a2g7b.
+
 config SENSORS_XDPE122
 	tristate "Infineon XDPE122 family"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index d6c86924f887..d592d8c77bec 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
 obj-$(CONFIG_SENSORS_XDP710)	+= xdp710.o
 obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
 obj-$(CONFIG_SENSORS_XDPE152)	+= xdpe152c4.o
+obj-$(CONFIG_SENSORS_XDPE1A2G7B)	+= xdpe1a2g7b.o
 obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
 obj-$(CONFIG_SENSORS_PIM4328)	+= pim4328.o
 obj-$(CONFIG_SENSORS_CRPS)	+= crps.o
diff --git a/drivers/hwmon/pmbus/xdpe1a2g7b.c b/drivers/hwmon/pmbus/xdpe1a2g7b.c
new file mode 100644
index 000000000000..e10bafeb0984
--- /dev/null
+++ b/drivers/hwmon/pmbus/xdpe1a2g7b.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for Infineon Multi-phase Digital XDPE1A2G5B
+ * and XDPE1A2G7B Controllers
+ *
+ * Copyright (c) 2026 Infineon Technologies. All rights reserved.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+#define XDPE1A2G7B_PAGE_NUM 2
+#define XDPE1A2G7B_NVIDIA_195MV 0x1E /* NVIDIA mode 1.95mV, VID step is 5mV */
+
+static int xdpe1a2g7b_identify(struct i2c_client *client,
+			       struct pmbus_driver_info *info)
+{
+	u8 vout_params;
+	int vout_mode;
+
+	/*
+	 * XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID data
+	 * formats via VOUT_MODE. Note that the device pages/loops are not fully
+	 * independent: configuration is shared, so programming each page/loop
+	 * separately is not supported for VOUT_MODE.
+	 */
+	vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
+	if (vout_mode >= 0 && vout_mode != 0xff) {
+		switch (vout_mode >> 5) {
+		case 0:
+			info->format[PSC_VOLTAGE_OUT] = linear;
+			return 0;
+		case 1:
+			info->format[PSC_VOLTAGE_OUT] = vid;
+			vout_params = vout_mode & GENMASK(4, 0);
+			/* Check for VID Code Type */
+			switch (vout_params) {
+			case XDPE1A2G7B_NVIDIA_195MV:
+				/* VID vrm_version for PAGE0 and PAGE1 */
+				info->vrm_version[0] = nvidia195mv;
+				info->vrm_version[1] = nvidia195mv;
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		default:
+			return -ENODEV;
+		}
+	} else {
+		return vout_mode < 0 ? vout_mode : -ENODEV;
+	}
+
+	return 0;
+}
+
+static struct pmbus_driver_info xdpe1a2g7b_info = {
+	.pages = XDPE1A2G7B_PAGE_NUM,
+	.identify = xdpe1a2g7b_identify,
+	.format[PSC_VOLTAGE_IN] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.format[PSC_CURRENT_IN] = linear,
+	.format[PSC_CURRENT_OUT] = linear,
+	.format[PSC_POWER] = linear,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
+		   PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_STATUS_TEMP |
+		   PMBUS_HAVE_POUT | PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT,
+	.func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
+		   PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_INPUT,
+};
+
+static int xdpe1a2g7b_probe(struct i2c_client *client)
+{
+	struct pmbus_driver_info *info;
+
+	info = devm_kmemdup(&client->dev, &xdpe1a2g7b_info, sizeof(*info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	return pmbus_do_probe(client, info);
+}
+
+static const struct i2c_device_id xdpe1a2g7b_id[] = {
+	{ "xdpe1a2g5b" },
+	{ "xdpe1a2g7b" },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, xdpe1a2g7b_id);
+
+static const struct of_device_id __maybe_unused xdpe1a2g7b_of_match[] = {
+	{ .compatible = "infineon,xdpe1a2g5b" },
+	{ .compatible = "infineon,xdpe1a2g7b" },
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, xdpe1a2g7b_of_match);
+
+static struct i2c_driver xdpe1a2g7b_driver = {
+	.driver = {
+		.name = "xdpe1a2g7b",
+		.of_match_table = of_match_ptr(xdpe1a2g7b_of_match),
+	},
+	.probe = xdpe1a2g7b_probe,
+	.id_table = xdpe1a2g7b_id,
+};
+
+module_i2c_driver(xdpe1a2g7b_driver);
+
+MODULE_AUTHOR("Ashish Yadav <ashish.yadav@infineon.com>");
+MODULE_DESCRIPTION("PMBus driver for Infineon XDPE1A2G5B/7B");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");
-- 
2.39.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers
  2026-02-19 10:56 ` [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers ASHISH YADAV
@ 2026-02-21 23:21   ` Guenter Roeck
  2026-02-23  4:19     ` ashish yadav
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2026-02-21 23:21 UTC (permalink / raw)
  To: ASHISH YADAV, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-hwmon, devicetree, linux-kernel, Ashish Yadav

On 2/19/26 02:56, ASHISH YADAV wrote:
> From: Ashish Yadav <ashish.yadav@infineon.com>
> 
> Add the pmbus driver for Infineon Digital Multi-phase XDPE1A2G5B and
> XDPE1A2G7B controllers.
> 
> Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
> ---
> XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID Data
> formats for VOUT_MODE.
> The configuring both loops/pages of the device independently is not
> supported for VOUT_MODE.
> In case of vid mode, NVIDIA PWM VID vrm_version is supported:
> Vout = 5mV * (VID-1) + 195mV
> ---
>   drivers/hwmon/pmbus/Kconfig      |   9 +++
>   drivers/hwmon/pmbus/Makefile     |   1 +
>   drivers/hwmon/pmbus/xdpe1a2g7b.c | 120 +++++++++++++++++++++++++++++++
>   3 files changed, 130 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/xdpe1a2g7b.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index fc1273abe357..a4513fc6bc26 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -711,6 +711,15 @@ config SENSORS_XDPE152
>   	  This driver can also be built as a module. If so, the module will
>   	  be called xdpe152c4.
>   
> +config SENSORS_XDPE1A2G7B
> +	tristate "Infineon XDPE1A2G7B"
> +	help
> +	  If you say yes here you get hardware monitoring support for Infineon
> +	  XDPE1A2G5B and XDPE1A2G7B.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called xdpe1a2g7b.
> +
>   config SENSORS_XDPE122
>   	tristate "Infineon XDPE122 family"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d6c86924f887..d592d8c77bec 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
>   obj-$(CONFIG_SENSORS_XDP710)	+= xdp710.o
>   obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
>   obj-$(CONFIG_SENSORS_XDPE152)	+= xdpe152c4.o
> +obj-$(CONFIG_SENSORS_XDPE1A2G7B)	+= xdpe1a2g7b.o
>   obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
>   obj-$(CONFIG_SENSORS_PIM4328)	+= pim4328.o
>   obj-$(CONFIG_SENSORS_CRPS)	+= crps.o
> diff --git a/drivers/hwmon/pmbus/xdpe1a2g7b.c b/drivers/hwmon/pmbus/xdpe1a2g7b.c
> new file mode 100644
> index 000000000000..e10bafeb0984
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/xdpe1a2g7b.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon Multi-phase Digital XDPE1A2G5B
> + * and XDPE1A2G7B Controllers
> + *
> + * Copyright (c) 2026 Infineon Technologies. All rights reserved.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define XDPE1A2G7B_PAGE_NUM 2
> +#define XDPE1A2G7B_NVIDIA_195MV 0x1E /* NVIDIA mode 1.95mV, VID step is 5mV */
> +
> +static int xdpe1a2g7b_identify(struct i2c_client *client,
> +			       struct pmbus_driver_info *info)
> +{
> +	u8 vout_params;
> +	int vout_mode;
> +
> +	/*
> +	 * XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID data
> +	 * formats via VOUT_MODE. Note that the device pages/loops are not fully
> +	 * independent: configuration is shared, so programming each page/loop
> +	 * separately is not supported for VOUT_MODE.
> +	 */
> +	vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
> +	if (vout_mode >= 0 && vout_mode != 0xff) {

"vout_mode != 0xff only makes sense in generic code, where it is possible
that the command is not properly implemented. It does not make sense
in a device specific driver: Either the command is implemented and returns
a useful value, or it is not implemented. So this code should simply be

	if (vout_mode < 0)
		return vout_mode;

	switch (vout_mode >> 5) {
	...

If the controller is really so unstable that it returns 0xff once
in a while, a respective comment would be needed in the code. However,
even then, I would suggest something like

	if (vout_mode < 0)
		return vout_mode;

	/* Explain instability */
	if (vout_mode == 0xff)
		return -ENODEV;

	switch (vout_mode >> 5) {
	...

i.e., error handling first.

If the concern is that this isn't really one of the chips supported by the driver,
I would suggest to implement chip detection code either in the identify
function or better in the probe function. If that is the case, the return value
from PMBUS_VOUT_MODE isn't useful anyway.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers
  2026-02-21 23:21   ` Guenter Roeck
@ 2026-02-23  4:19     ` ashish yadav
  0 siblings, 0 replies; 6+ messages in thread
From: ashish yadav @ 2026-02-23  4:19 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-hwmon,
	devicetree, linux-kernel, Ashish Yadav

Hi Guenter,

Thanks for your time and feedback.
Please find my response inline.

With Best Regards,
    Ashish Yadav


On Sun, Feb 22, 2026 at 4:52 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 2/19/26 02:56, ASHISH YADAV wrote:
> > From: Ashish Yadav <ashish.yadav@infineon.com>
> >
> > Add the pmbus driver for Infineon Digital Multi-phase XDPE1A2G5B and
> > XDPE1A2G7B controllers.
> >
> > Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
> > ---
> > XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID Data
> > formats for VOUT_MODE.
> > The configuring both loops/pages of the device independently is not
> > supported for VOUT_MODE.
> > In case of vid mode, NVIDIA PWM VID vrm_version is supported:
> > Vout = 5mV * (VID-1) + 195mV
> > ---
> >   drivers/hwmon/pmbus/Kconfig      |   9 +++
> >   drivers/hwmon/pmbus/Makefile     |   1 +
> >   drivers/hwmon/pmbus/xdpe1a2g7b.c | 120 +++++++++++++++++++++++++++++++
> >   3 files changed, 130 insertions(+)
> >   create mode 100644 drivers/hwmon/pmbus/xdpe1a2g7b.c
> >
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index fc1273abe357..a4513fc6bc26 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -711,6 +711,15 @@ config SENSORS_XDPE152
> >         This driver can also be built as a module. If so, the module will
> >         be called xdpe152c4.
> >
> > +config SENSORS_XDPE1A2G7B
> > +     tristate "Infineon XDPE1A2G7B"
> > +     help
> > +       If you say yes here you get hardware monitoring support for Infineon
> > +       XDPE1A2G5B and XDPE1A2G7B.
> > +
> > +       This driver can also be built as a module. If so, the module will
> > +       be called xdpe1a2g7b.
> > +
> >   config SENSORS_XDPE122
> >       tristate "Infineon XDPE122 family"
> >       help
> > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> > index d6c86924f887..d592d8c77bec 100644
> > --- a/drivers/hwmon/pmbus/Makefile
> > +++ b/drivers/hwmon/pmbus/Makefile
> > @@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_UCD9200)       += ucd9200.o
> >   obj-$(CONFIG_SENSORS_XDP710)        += xdp710.o
> >   obj-$(CONFIG_SENSORS_XDPE122)       += xdpe12284.o
> >   obj-$(CONFIG_SENSORS_XDPE152)       += xdpe152c4.o
> > +obj-$(CONFIG_SENSORS_XDPE1A2G7B)     += xdpe1a2g7b.o
> >   obj-$(CONFIG_SENSORS_ZL6100)        += zl6100.o
> >   obj-$(CONFIG_SENSORS_PIM4328)       += pim4328.o
> >   obj-$(CONFIG_SENSORS_CRPS)  += crps.o
> > diff --git a/drivers/hwmon/pmbus/xdpe1a2g7b.c b/drivers/hwmon/pmbus/xdpe1a2g7b.c
> > new file mode 100644
> > index 000000000000..e10bafeb0984
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus/xdpe1a2g7b.c
> > @@ -0,0 +1,120 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Hardware monitoring driver for Infineon Multi-phase Digital XDPE1A2G5B
> > + * and XDPE1A2G7B Controllers
> > + *
> > + * Copyright (c) 2026 Infineon Technologies. All rights reserved.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include "pmbus.h"
> > +
> > +#define XDPE1A2G7B_PAGE_NUM 2
> > +#define XDPE1A2G7B_NVIDIA_195MV 0x1E /* NVIDIA mode 1.95mV, VID step is 5mV */
> > +
> > +static int xdpe1a2g7b_identify(struct i2c_client *client,
> > +                            struct pmbus_driver_info *info)
> > +{
> > +     u8 vout_params;
> > +     int vout_mode;
> > +
> > +     /*
> > +      * XDPE1A2G5B and XDPE1A2G7B support both Linear and NVIDIA PWM VID data
> > +      * formats via VOUT_MODE. Note that the device pages/loops are not fully
> > +      * independent: configuration is shared, so programming each page/loop
> > +      * separately is not supported for VOUT_MODE.
> > +      */
> > +     vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
> > +     if (vout_mode >= 0 && vout_mode != 0xff) {
>
> "vout_mode != 0xff only makes sense in generic code, where it is possible
> that the command is not properly implemented. It does not make sense
> in a device specific driver: Either the command is implemented and returns
> a useful value, or it is not implemented. So this code should simply be
>
>         if (vout_mode < 0)
>                 return vout_mode;
>
>         switch (vout_mode >> 5) {
>         ...
>
> If the controller is really so unstable that it returns 0xff once
> in a while, a respective comment would be needed in the code. However,
> even then, I would suggest something like
>
>         if (vout_mode < 0)
>                 return vout_mode;
>
>         /* Explain instability */
>         if (vout_mode == 0xff)
>                 return -ENODEV;
>
>         switch (vout_mode >> 5) {
>         ...
>
> i.e., error handling first.
  The chip and command are stable, so I will proceed with making the
necessary changes as suggested by you.

> If the concern is that this isn't really one of the chips supported by the driver,
> I would suggest to implement chip detection code either in the identify
> function or better in the probe function. If that is the case, the return value
> from PMBUS_VOUT_MODE isn't useful anyway.
>
> Thanks,
> Guenter
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-23  4:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-19 10:56 [PATCH v2 0/3] Add support for Infineon Digital Multi-phase Controllers XDPE1A2G5B/7B ASHISH YADAV
2026-02-19 10:56 ` [PATCH v2 1/3] dt-bindings: trivial-devices: Add support for XDPE1A2G5B/7B ASHISH YADAV
2026-02-19 10:56 ` =?y?q?=5BPATCH=20v2=202/3=5D=20hwmon=3A=20=28pmbus/core=29=20Add=20support=20for=C2=A0NVIDIA=C2=A0nvidia195mv=20mode?= ASHISH YADAV
2026-02-19 10:56 ` [PATCH v2 3/3] hwmon:(pmbus/xdpe1a2g7b) Add support for xdpe1a2g5b/7b controllers ASHISH YADAV
2026-02-21 23:21   ` Guenter Roeck
2026-02-23  4:19     ` ashish yadav

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®