[PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685

[PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685

[Chiang Brian]

The TPS53685 is a fully AMD SVI3 compliant step down controller with
trans-inductor voltage regulator(TLVR) topology support, dual channels,
built-in non-volatile memory (NVM), PMBus interface, and full compatible
with TI NexFET smart power stages.

Add support for it to the tps53679 driver.

Signed-off-by: Chiang Brian <chiang.brian@inventec.com>
---
v8 -> v9:
- Wrap commit message body at 75 columns
- Fix the alignment to match open parenthesis
- Link to v8: https://lore.kernel.org/all/20250602042454.184643-3-chiang.brian@inventec.com/

v7 -> v8:
- Convert the type of device_id in tps53679_identify_multiphase() from int to char *
- Run make.cross with ARCH i386
- Link to v7: https://lore.kernel.org/all/20250515081449.1433772-3-chiang.brian@inventec.com/

v6 -> v7:
- Modify the type of device_id from u16 to char *
- Run make.cross with ARCH nios2, powerpc, and riscv
- Link to v6: https://lore.kernel.org/all/20250424132538.2004510-2-chiang.brian@inventec.corp-partner.google.com/

v5 -> v6:
- Add information about tps53685 into tps53679.rst
- Add additional flags when identifing the chip as tps53685
- Adjust length once returned device id is terminated by null character
- Link to v5: https://lore.kernel.org/all/20250314033040.3190642-1-chiang.brian@inventec.com/

v4 -> v5: 
- document the compatible of tps53685 into dt-bindings
- add the buffer length as argument for %*ph
- Add Changelog
- Link to v4: https://lore.kernel.org/all/CAJCfHmW61d2jd_tYpNEqBG_Z58bEnVKAmsvhrEP1zXQoXqrUVw@mail.gmail.com/

v3 -> v4: 
- Add length comparison into the comparison of "id",or it may be true when the substring of "id" matches device id. 
- Restore `return 0;` in `tps53679_identify_chip()`
- Link to v3: https://lore.kernel.org/all/CAJCfHmVyaDPh0_ThPjhBP0zMO1oE1AR=4=Zsa0cMPXU3J4v6dw@mail.gmail.com/

v2 -> v3:
- Remove the length comparsion in the comparison of "id".
- Link to v2: https://lore.kernel.org/all/CAJCfHmUteFM+nUZWBWvmwFjALg1QUL5r+=syU1HmYTL1ewQWqA@mail.gmail.com/

v1 -> v2: 
- Modify subject and description to meet requirements
- Add "tps53685" into enum chips with numeric order
- Modify the content of marco "TPS53681_DEVICE_ID" from 0x81 to "\x81" 
- Add marco "TPS53685_DEVICE_ID" with content "TIShP"
- Modify the type of "id" from u16 to char* in `tps53679_identify_chip()`
- Modify the comparison of "id". It will be true if the string "id" matches device ID and compare with type char*,
- Add the length comparsion into the comparison of "id".
- Modify "len" as return code in `tps53679_identify_chip()`
- Output device error log with %*ph, instead of 0x%x\n" 
- Use existing tps53679_identify_multiphase() with argument "TPS53685_DEVICE_ID" in tps53685_identify() rather than creating one tps53685_identify_multiphase()
- Link to v1: https://lore.kernel.org/all/CAJCfHmVy3O4-nz2_PKF7TcXYr+HqTte1-bdUWLBmV7JOS7He1g@mail.gmail.com/

 Documentation/hwmon/tps53679.rst |  8 +++++++
 drivers/hwmon/pmbus/tps53679.c   | 37 ++++++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/tps53679.rst b/Documentation/hwmon/tps53679.rst
index 3b9561648c24..dd5e4a37375d 100644
--- a/Documentation/hwmon/tps53679.rst
+++ b/Documentation/hwmon/tps53679.rst
@@ -43,6 +43,14 @@ Supported chips:
 
     Datasheet: https://www.ti.com/lit/gpn/TPS53681
 
+  * Texas Instruments TPS53685
+
+    Prefix: 'tps53685'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.ti.com/lit/gpn/TPS53685
+
   * Texas Instruments TPS53688
 
     Prefix: 'tps53688'
diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index 63524dff5e75..47c28a8e25a5 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -16,7 +16,7 @@
 #include "pmbus.h"
 
 enum chips {
-	tps53647, tps53667, tps53676, tps53679, tps53681, tps53688
+	tps53647, tps53667, tps53676, tps53679, tps53681, tps53685, tps53688
 };
 
 #define TPS53647_PAGE_NUM		1
@@ -31,7 +31,8 @@ enum chips {
 #define TPS53679_PROT_VR13_5MV		0x07 /* VR13.0 mode, 5-mV DAC */
 #define TPS53679_PAGE_NUM		2
 
-#define TPS53681_DEVICE_ID		0x81
+#define TPS53681_DEVICE_ID     "\x81"
+#define TPS53685_DEVICE_ID     "TIShP"
 
 #define TPS53681_PMBUS_REVISION		0x33
 
@@ -86,10 +87,12 @@ static int tps53679_identify_phases(struct i2c_client *client,
 }
 
 static int tps53679_identify_chip(struct i2c_client *client,
-				  u8 revision, u16 id)
+				  u8 revision, char *id)
 {
 	u8 buf[I2C_SMBUS_BLOCK_MAX];
 	int ret;
+	int buf_len;
+	int id_len;
 
 	ret = pmbus_read_byte_data(client, 0, PMBUS_REVISION);
 	if (ret < 0)
@@ -102,8 +105,14 @@ static int tps53679_identify_chip(struct i2c_client *client,
 	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
 	if (ret < 0)
 		return ret;
-	if (ret != 1 || buf[0] != id) {
-		dev_err(&client->dev, "Unexpected device ID 0x%x\n", buf[0]);
+
+	/* Adjust length if null terminator if present */
+	buf_len = (buf[ret - 1] != '\x00' ? ret : ret - 1);
+
+	id_len = strlen(id);
+
+	if (buf_len != id_len || strncmp(id, buf, id_len)) {
+		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
 		return -ENODEV;
 	}
 	return 0;
@@ -117,7 +126,7 @@ static int tps53679_identify_chip(struct i2c_client *client,
  */
 static int tps53679_identify_multiphase(struct i2c_client *client,
 					struct pmbus_driver_info *info,
-					int pmbus_rev, int device_id)
+					int pmbus_rev, char *device_id)
 {
 	int ret;
 
@@ -138,6 +147,16 @@ static int tps53679_identify(struct i2c_client *client,
 	return tps53679_identify_mode(client, info);
 }
 
+static int tps53685_identify(struct i2c_client *client,
+			     struct pmbus_driver_info *info)
+{
+	info->func[1] |= PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
+			 PMBUS_HAVE_STATUS_INPUT;
+	info->format[PSC_VOLTAGE_OUT] = linear;
+	return tps53679_identify_chip(client, TPS53681_PMBUS_REVISION,
+					   TPS53685_DEVICE_ID);
+}
+
 static int tps53681_identify(struct i2c_client *client,
 			     struct pmbus_driver_info *info)
 {
@@ -263,6 +282,10 @@ static int tps53679_probe(struct i2c_client *client)
 		info->identify = tps53681_identify;
 		info->read_word_data = tps53681_read_word_data;
 		break;
+	case tps53685:
+	    info->pages = TPS53679_PAGE_NUM;
+	    info->identify = tps53685_identify;
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -277,6 +300,7 @@ static const struct i2c_device_id tps53679_id[] = {
 	{"tps53676", tps53676},
 	{"tps53679", tps53679},
 	{"tps53681", tps53681},
+	{"tps53685", tps53685},
 	{"tps53688", tps53688},
 	{}
 };
@@ -289,6 +313,7 @@ static const struct of_device_id __maybe_unused tps53679_of_match[] = {
 	{.compatible = "ti,tps53676", .data = (void *)tps53676},
 	{.compatible = "ti,tps53679", .data = (void *)tps53679},
 	{.compatible = "ti,tps53681", .data = (void *)tps53681},
+	{.compatible = "ti,tps53685", .data = (void *)tps53685},
 	{.compatible = "ti,tps53688", .data = (void *)tps53688},
 	{}
 };
-- 
2.25.1

Re: [PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685

[kernel test robot]

Hi Chiang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on groeck-staging/hwmon-next krzk-dt/for-next linus/master v6.16-rc1 next-20250612]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chiang-Brian/hwmon-pmbus-tps53679-Add-support-for-TPS53685/20250610-192612
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250610102556.236300-3-chiang.brian%40inventec.com
patch subject: [PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685
config: s390-randconfig-r073-20250611 (https://download.01.org/0day-ci/archive/20250613/202506130217.6OQ8zSf5-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506130217.6OQ8zSf5-lkp@intel.com/

smatch warnings:
drivers/hwmon/pmbus/tps53679.c:288 tps53679_probe() warn: inconsistent indenting

vim +288 drivers/hwmon/pmbus/tps53679.c

   249	
   250	static int tps53679_probe(struct i2c_client *client)
   251	{
   252		struct device *dev = &client->dev;
   253		struct pmbus_driver_info *info;
   254		enum chips chip_id;
   255	
   256		if (dev->of_node)
   257			chip_id = (uintptr_t)of_device_get_match_data(dev);
   258		else
   259			chip_id = i2c_match_id(tps53679_id, client)->driver_data;
   260	
   261		info = devm_kmemdup(dev, &tps53679_info, sizeof(*info), GFP_KERNEL);
   262		if (!info)
   263			return -ENOMEM;
   264	
   265		switch (chip_id) {
   266		case tps53647:
   267		case tps53667:
   268			info->pages = TPS53647_PAGE_NUM;
   269			info->identify = tps53679_identify;
   270			break;
   271		case tps53676:
   272			info->identify = tps53676_identify;
   273			break;
   274		case tps53679:
   275		case tps53688:
   276			info->pages = TPS53679_PAGE_NUM;
   277			info->identify = tps53679_identify;
   278			break;
   279		case tps53681:
   280			info->pages = TPS53679_PAGE_NUM;
   281			info->phases[0] = 6;
   282			info->identify = tps53681_identify;
   283			info->read_word_data = tps53681_read_word_data;
   284			break;
   285		case tps53685:
   286		    info->pages = TPS53679_PAGE_NUM;
   287		    info->identify = tps53685_identify;
 > 288			break;
   289		default:
   290			return -ENODEV;
   291		}
   292	
   293		return pmbus_do_probe(client, info);
   294	}
   295	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685

[Guenter Roeck]

On 6/10/25 03:25, Chiang Brian wrote:
...
> @@ -263,6 +282,10 @@ static int tps53679_probe(struct i2c_client *client)
>   		info->identify = tps53681_identify;
>   		info->read_word_data = tps53681_read_word_data;
>   		break;
> +	case tps53685:
> +	    info->pages = TPS53679_PAGE_NUM;
> +	    info->identify = tps53685_identify;
> +		break;

0-day rightfully complains about the bad alignment above.

If you want this patch to be applied for the next kernel release,
I would suggest to copy the hardware monitoring list on the first patch
of the series. Without it, I get

WARNING: DT compatible string "ti,tps53685" appears un-documented -- check ./Documentation/devicetree/bindings/
#296: FILE: drivers/hwmon/pmbus/tps53679.c:316:
+	{.compatible = "ti,tps53685", .data = (void *)tps53685},

Since I am not copied on the first patch of the series, that patch has to be applied
through a different maintainer tree, and I have to wait until that is the case.
Not that I understand the logic behind it, but I mentioned that before, so I assume
this is on purpose.

Guenter

Re: [PATCH v9 2/2] hwmon: (pmbus/tps53679) Add support for TPS53685

[Brian Chiang]

> On Mon, 16 Jun 2025 09:15:43 -0700 Guenter Roeck wrote:
> 
> On 6/10/25 03:25, Chiang Brian wrote:
> ...
> > @@ -263,6 +282,10 @@ static int tps53679_probe(struct i2c_client *client)
> >   		info->identify = tps53681_identify;
> >   		info->read_word_data = tps53681_read_word_data;
> >   		break;
> > +	case tps53685:
> > +	    info->pages = TPS53679_PAGE_NUM;
> > +	    info->identify = tps53685_identify;
> > +		break;
> 
> 0-day rightfully complains about the bad alignment above.
> 
> If you want this patch to be applied for the next kernel release,
> I would suggest to copy the hardware monitoring list on the first patch
> of the series. Without it, I get
> 
> WARNING: DT compatible string "ti,tps53685" appears un-documented -- check ./Documentation/devicetree/bindings/
> #296: FILE: drivers/hwmon/pmbus/tps53679.c:316:
> +	{.compatible = "ti,tps53685", .data = (void *)tps53685},
> 
> Since I am not copied on the first patch of the series, that patch has to be applied
> through a different maintainer tree, and I have to wait until that is the case.
> Not that I understand the logic behind it, but I mentioned that before, so I assume
> this is on purpose.

Thank you for the feedback. I'll address both issues in v10:

1. Fix the indentation alignment in the switch case
2. CC linux-hwmon@vger.kernel.org on all patches in the series

I apologize for the oversight, this wasn't intentional, but rather due to 
my automated CC generation only considering per-patch maintainers.

For v10, I'll ensure all the devicetree and hwmon maintainer lists are 
included on all patches so you can see the complete series.

Best Regards,
Brian Chiang