[PATCH 00/11] Remove use of i2c_match_id in PMBUS

[PATCH 00/11] Remove use of i2c_match_id in PMBUS

[Andrew Davis]

Hello all,

Much like we already have done for HWMON top level drivers,
we are removing the i2c_match_id() function from all hwmon/pmbus/
drivers now. Using i2c_get_match_data() can simplify code and has
some other benefits described in the patches.

I don't have most of this hardware, so testing is very welcome :)

Thanks,
Andrew

Andrew Davis (11):
  hwmon: (pmbus/bel-pfe) Remove use of i2c_match_id()
  hwmon: (pmbus/ibm-cffps) Remove use of i2c_match_id()
  hwmon: (pmbus/isl68137) Remove use of i2c_match_id()
  hwmon: (pmbus/max20730) Remove use of i2c_match_id()
  hwmon: (pmbus/max34440) Remove use of i2c_match_id()
  hwmon: (pmbus) Remove use of i2c_match_id()
  hwmon: (pmbus/q54sj108a2) Remove use of i2c_match_id()
  hwmon: (pmbus/tps53679) Remove use of i2c_match_id()
  hwmon: (pmbus/fsp-3y) Remove use of i2c_match_id()
  hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()
  hwmon: (pmbus/max16601) Remove use of i2c_match_id()

 drivers/hwmon/pmbus/bel-pfe.c    |  5 +----
 drivers/hwmon/pmbus/fsp-3y.c     | 13 ++++++-------
 drivers/hwmon/pmbus/ibm-cffps.c  | 12 +-----------
 drivers/hwmon/pmbus/isl68137.c   |  4 +---
 drivers/hwmon/pmbus/ltc2978.c    |  2 +-
 drivers/hwmon/pmbus/max16601.c   |  2 +-
 drivers/hwmon/pmbus/max20730.c   |  5 +----
 drivers/hwmon/pmbus/max34440.c   |  4 +---
 drivers/hwmon/pmbus/pmbus.c      |  4 +---
 drivers/hwmon/pmbus/q54sj108a2.c |  5 +----
 drivers/hwmon/pmbus/tps53679.c   |  5 +----
 11 files changed, 16 insertions(+), 45 deletions(-)

-- 
2.39.2

[PATCH 01/11] hwmon: (pmbus/bel-pfe) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/bel-pfe.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
index ddf9d9a2958c4..6499556f735b4 100644
--- a/drivers/hwmon/pmbus/bel-pfe.c
+++ b/drivers/hwmon/pmbus/bel-pfe.c
@@ -88,13 +88,10 @@ static struct pmbus_driver_info pfe_driver_info[] = {
 	},
 };
 
-static const struct i2c_device_id pfe_device_id[];
-
 static int pfe_pmbus_probe(struct i2c_client *client)
 {
-	int model;
+	int model = (uintptr_t)i2c_get_match_data(client);
 
-	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
 	client->dev.platform_data = &pfe_plat_data;
 
 	/*
-- 
2.39.2

[PATCH 02/11] hwmon: (pmbus/ibm-cffps) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/ibm-cffps.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index d05ef7a968a96..884b17de3b623 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -482,19 +482,9 @@ MODULE_DEVICE_TABLE(i2c, ibm_cffps_id);
 static int ibm_cffps_probe(struct i2c_client *client)
 {
 	int i, rc;
-	enum versions vs = cffps_unknown;
+	enum versions vs = (uintptr_t)i2c_get_match_data(client);
 	struct dentry *debugfs;
 	struct ibm_cffps *psu;
-	const void *md = of_device_get_match_data(&client->dev);
-	const struct i2c_device_id *id;
-
-	if (md) {
-		vs = (uintptr_t)md;
-	} else {
-		id = i2c_match_id(ibm_cffps_id, client);
-		if (id)
-			vs = (enum versions)id->driver_data;
-	}
 
 	if (vs == cffps_unknown) {
 		u16 ccin_revision = 0;
-- 
2.39.2

[PATCH 03/11] hwmon: (pmbus/isl68137) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/isl68137.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c
index 97b61836f53a4..78cff97128541 100644
--- a/drivers/hwmon/pmbus/isl68137.c
+++ b/drivers/hwmon/pmbus/isl68137.c
@@ -90,8 +90,6 @@ struct isl68137_data {
 
 #define to_isl68137_data(x)	container_of(x, struct isl68137_data, info)
 
-static const struct i2c_device_id raa_dmpvr_id[];
-
 static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client,
 					     int page,
 					     char *buf)
@@ -375,7 +373,7 @@ static int isl68137_probe(struct i2c_client *client)
 	memcpy(&data->info, &raa_dmpvr_info, sizeof(data->info));
 	info = &data->info;
 
-	switch (i2c_match_id(raa_dmpvr_id, client)->driver_data) {
+	switch ((uintptr_t)i2c_get_match_data(client)) {
 	case raa_dmpvr1_2rail:
 		info->pages = 2;
 		info->R[PSC_VOLTAGE_IN] = 3;
-- 
2.39.2

[PATCH 04/11] hwmon: (pmbus/max20730) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/max20730.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
index 95869d198ecf0..fe03164788dfa 100644
--- a/drivers/hwmon/pmbus/max20730.c
+++ b/drivers/hwmon/pmbus/max20730.c
@@ -715,10 +715,7 @@ static int max20730_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
-	if (client->dev.of_node)
-		chip_id = (uintptr_t)of_device_get_match_data(dev);
-	else
-		chip_id = i2c_match_id(max20730_id, client)->driver_data;
+	chip_id = (uintptr_t)i2c_get_match_data(client);
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
-- 
2.39.2

[PATCH 05/11] hwmon: (pmbus/max34440) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/max34440.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
index 8ea4e68d4e9de..cc96bb22f8f5a 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -71,8 +71,6 @@ struct max34440_data {
 
 #define to_max34440_data(x)  container_of(x, struct max34440_data, info)
 
-static const struct i2c_device_id max34440_id[];
-
 static int max34440_read_word_data(struct i2c_client *client, int page,
 				   int phase, int reg)
 {
@@ -628,7 +626,7 @@ static int max34440_probe(struct i2c_client *client)
 			    GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	data->id = i2c_match_id(max34440_id, client)->driver_data;
+	data->id = (uintptr_t)i2c_get_match_data(client);
 	data->info = max34440_info[data->id];
 	data->iout_oc_fault_limit = MAX34440_IOUT_OC_FAULT_LIMIT;
 	data->iout_oc_warn_limit = MAX34440_IOUT_OC_WARN_LIMIT;
-- 
2.39.2

[PATCH 06/11] hwmon: (pmbus) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/pmbus.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index 920cd5408141a..d1844c7a51eef 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -20,8 +20,6 @@ struct pmbus_device_info {
 	u32 flags;
 };
 
-static const struct i2c_device_id pmbus_id[];
-
 /*
  * Find sensor groups and status registers on each page.
  */
@@ -174,7 +172,7 @@ static int pmbus_probe(struct i2c_client *client)
 	if (!info)
 		return -ENOMEM;
 
-	device_info = (struct pmbus_device_info *)i2c_match_id(pmbus_id, client)->driver_data;
+	device_info = (struct pmbus_device_info *)i2c_get_match_data(client);
 	if (device_info->flags) {
 		pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
 				     GFP_KERNEL);
-- 
2.39.2

[PATCH 07/11] hwmon: (pmbus/q54sj108a2) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/q54sj108a2.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c
index fc030ca34480c..88c25290ffce1 100644
--- a/drivers/hwmon/pmbus/q54sj108a2.c
+++ b/drivers/hwmon/pmbus/q54sj108a2.c
@@ -291,10 +291,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
 				     I2C_FUNC_SMBUS_BLOCK_DATA))
 		return -ENODEV;
 
-	if (client->dev.of_node)
-		chip_id = (enum chips)(unsigned long)of_device_get_match_data(dev);
-	else
-		chip_id = i2c_match_id(q54sj108a2_id, client)->driver_data;
+	chip_id = (enum chips)(uintptr_t)i2c_get_match_data(client);
 
 	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
 	if (ret < 0) {
-- 
2.39.2

[PATCH 08/11] hwmon: (pmbus/tps53679) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/tps53679.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index ca2bfa25eb04c..df2726659a4ed 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -253,10 +253,7 @@ static int tps53679_probe(struct i2c_client *client)
 	struct pmbus_driver_info *info;
 	enum chips chip_id;
 
-	if (dev->of_node)
-		chip_id = (uintptr_t)of_device_get_match_data(dev);
-	else
-		chip_id = i2c_match_id(tps53679_id, client)->driver_data;
+	chip_id = (uintptr_t)i2c_get_match_data(client);
 
 	info = devm_kmemdup(dev, &tps53679_info, sizeof(*info), GFP_KERNEL);
 	if (!info)
-- 
2.39.2

[PATCH 09/11] hwmon: (pmbus/fsp-3y) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/fsp-3y.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
index a4dc09e2ef75a..cad4d23300036 100644
--- a/drivers/hwmon/pmbus/fsp-3y.c
+++ b/drivers/hwmon/pmbus/fsp-3y.c
@@ -222,12 +222,6 @@ static int fsp3y_detect(struct i2c_client *client)
 	return -ENODEV;
 }
 
-static const struct i2c_device_id fsp3y_id[] = {
-	{"ym2151e", ym2151e},
-	{"yh5151e", yh5151e},
-	{ }
-};
-
 static int fsp3y_probe(struct i2c_client *client)
 {
 	struct fsp3y_data *data;
@@ -242,7 +236,7 @@ static int fsp3y_probe(struct i2c_client *client)
 	if (data->chip < 0)
 		return data->chip;
 
-	id = i2c_match_id(fsp3y_id, client);
+	id = i2c_client_get_device_id(client);
 	if (data->chip != id->driver_data)
 		dev_warn(&client->dev, "Device mismatch: Configured %s (%d), detected %d\n",
 			 id->name, (int)id->driver_data, data->chip);
@@ -276,6 +270,11 @@ static int fsp3y_probe(struct i2c_client *client)
 	return pmbus_do_probe(client, &data->info);
 }
 
+static const struct i2c_device_id fsp3y_id[] = {
+	{"ym2151e", ym2151e},
+	{"yh5151e", yh5151e},
+	{ }
+};
 MODULE_DEVICE_TABLE(i2c, fsp3y_id);
 
 static struct i2c_driver fsp3y_driver = {
-- 
2.39.2

[PATCH 10/11] hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/ltc2978.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
index 8f5be520a15db..d69a5e675e80e 100644
--- a/drivers/hwmon/pmbus/ltc2978.c
+++ b/drivers/hwmon/pmbus/ltc2978.c
@@ -733,7 +733,7 @@ static int ltc2978_probe(struct i2c_client *client)
 		return chip_id;
 
 	data->id = chip_id;
-	id = i2c_match_id(ltc2978_id, client);
+	id = i2c_client_get_device_id(client);
 	if (data->id != id->driver_data)
 		dev_warn(&client->dev,
 			 "Device mismatch: Configured %s (%d), detected %d\n",
-- 
2.39.2

[PATCH 11/11] hwmon: (pmbus/max16601) Remove use of i2c_match_id()

[Andrew Davis]

The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This is often used to then retrieve the
matching driver_data. This can be done in one step with the helper
i2c_get_match_data().

This helper has a couple other benefits:
 * It doesn't need the i2c_device_id passed in so we do not need
   to have that forward declared, allowing us to remove those or
   move the i2c_device_id table down to its more natural spot
   with the other module info.
 * It also checks for device match data, which allows for OF and
   ACPI based probing. That means we do not have to manually check
   those first and can remove those checks.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/max16601.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
index d696e506aafba..36dc13424d929 100644
--- a/drivers/hwmon/pmbus/max16601.c
+++ b/drivers/hwmon/pmbus/max16601.c
@@ -318,7 +318,7 @@ static int max16601_probe(struct i2c_client *client)
 	if (chip_id < 0)
 		return chip_id;
 
-	id = i2c_match_id(max16601_id, client);
+	id = i2c_client_get_device_id(client);
 	if (chip_id != id->driver_data)
 		dev_warn(&client->dev,
 			 "Device mismatch: Configured %s (%d), detected %d\n",
-- 
2.39.2

Re: [PATCH 01/11] hwmon: (pmbus/bel-pfe) Remove use of i2c_match_id()

[Guenter Roeck]

On Thu, Mar 05, 2026 at 01:56:32PM -0600, Andrew Davis wrote:
> The function i2c_match_id() is used to fetch the matching ID from
> the i2c_device_id table. This is often used to then retrieve the
> matching driver_data. This can be done in one step with the helper
> i2c_get_match_data().
> 
> This helper has a couple other benefits:
>  * It doesn't need the i2c_device_id passed in so we do not need
>    to have that forward declared, allowing us to remove those or
>    move the i2c_device_id table down to its more natural spot
>    with the other module info.
>  * It also checks for device match data, which allows for OF and
>    ACPI based probing. That means we do not have to manually check
>    those first and can remove those checks.

The above seem to be boilerplates, not matching the actual patches.

AI review says:

  This isn't a bug, but this description is inaccurate for this specific driver.
  Looking at the code before this commit, there were no manual checks for OF or
  ACPI match data (like of_match_device or acpi_match_device) in
  pfe_pmbus_probe(). The driver only ever relied on i2c_match_id().
  Could the commit message be updated to avoid claiming it removes manual
  OF/ACPI checks from this specific driver?

Similar feedback applies to most of the patches in this series. Please resend
the series and drop inaccurate information from the commit descriptions.

Thanks,
Guenter

> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/hwmon/pmbus/bel-pfe.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
> index ddf9d9a2958c4..6499556f735b4 100644
> --- a/drivers/hwmon/pmbus/bel-pfe.c
> +++ b/drivers/hwmon/pmbus/bel-pfe.c
> @@ -88,13 +88,10 @@ static struct pmbus_driver_info pfe_driver_info[] = {
>  	},
>  };
>  
> -static const struct i2c_device_id pfe_device_id[];
> -
>  static int pfe_pmbus_probe(struct i2c_client *client)
>  {
> -	int model;
> +	int model = (uintptr_t)i2c_get_match_data(client);
>  
> -	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
>  	client->dev.platform_data = &pfe_plat_data;
>  
>  	/*
> -- 
> 2.39.2
> 

Re: [PATCH 02/11] hwmon: (pmbus/ibm-cffps) Remove use of i2c_match_id()

[Guenter Roeck]

On Thu, Mar 05, 2026 at 01:56:33PM -0600, Andrew Davis wrote:
> The function i2c_match_id() is used to fetch the matching ID from
> the i2c_device_id table. This is often used to then retrieve the
> matching driver_data. This can be done in one step with the helper
> i2c_get_match_data().
> 
> This helper has a couple other benefits:
>  * It doesn't need the i2c_device_id passed in so we do not need
>    to have that forward declared, allowing us to remove those or
>    move the i2c_device_id table down to its more natural spot
>    with the other module info.
>  * It also checks for device match data, which allows for OF and
>    ACPI based probing. That means we do not have to manually check
>    those first and can remove those checks.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/hwmon/pmbus/ibm-cffps.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
> index d05ef7a968a96..884b17de3b623 100644
> --- a/drivers/hwmon/pmbus/ibm-cffps.c
> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
> @@ -482,19 +482,9 @@ MODULE_DEVICE_TABLE(i2c, ibm_cffps_id);
>  static int ibm_cffps_probe(struct i2c_client *client)
>  {
>  	int i, rc;
> -	enum versions vs = cffps_unknown;
> +	enum versions vs = (uintptr_t)i2c_get_match_data(client);

AI feedback:

  Does this inadvertently change the fallback behavior for unmatched devices?

  In the old code, `vs` was initialized to `cffps_unknown` (which is 2). If a
  device was instantiated manually via sysfs without a matching I2C name, both
  `md` and `id` would be NULL, and `vs` remained `cffps_unknown`. This
  correctly triggered the dynamic version detection logic using
  `CFFPS_CCIN_CMD`.

  With the new code, if `i2c_get_match_data()` doesn't find a match, it returns
  `NULL`. Since `NULL` casts to `0` (`cffps1`), the driver will now default to
  `cffps1` and skip the dynamic version detection entirely.

  Additionally, because `cffps1` is `0`, `i2c_get_match_data()` cannot distinguish
  between an explicit `cffps1` match (where `.data` is `0`) and no match at
  all, since both result in returning `NULL`.

  To safely support fallback, `cffps1` could be assigned a non-zero value in
  `enum versions` (e.g., `enum versions { cffps1 = 1, ... }`).

Guenter

>  	struct dentry *debugfs;
>  	struct ibm_cffps *psu;
> -	const void *md = of_device_get_match_data(&client->dev);
> -	const struct i2c_device_id *id;
> -
> -	if (md) {
> -		vs = (uintptr_t)md;
> -	} else {
> -		id = i2c_match_id(ibm_cffps_id, client);
> -		if (id)
> -			vs = (enum versions)id->driver_data;
> -	}
>  
>  	if (vs == cffps_unknown) {
>  		u16 ccin_revision = 0;
> -- 
> 2.39.2
> 

Re: [PATCH 11/11] hwmon: (pmbus/max16601) Remove use of i2c_match_id()

[Guenter Roeck]

On Thu, Mar 05, 2026 at 01:56:42PM -0600, Andrew Davis wrote:
> The function i2c_match_id() is used to fetch the matching ID from
> the i2c_device_id table. This is often used to then retrieve the
> matching driver_data. This can be done in one step with the helper
> i2c_get_match_data().

... but the code below doesn;t actually call i2c_get_match_data().

> 
> This helper has a couple other benefits:
>  * It doesn't need the i2c_device_id passed in so we do not need
>    to have that forward declared, allowing us to remove those or
>    move the i2c_device_id table down to its more natural spot
>    with the other module info.

... but it doesn't do that.

>  * It also checks for device match data, which allows for OF and
>    ACPI based probing. That means we do not have to manually check
>    those first and can remove those checks.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/hwmon/pmbus/max16601.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> index d696e506aafba..36dc13424d929 100644
> --- a/drivers/hwmon/pmbus/max16601.c
> +++ b/drivers/hwmon/pmbus/max16601.c
> @@ -318,7 +318,7 @@ static int max16601_probe(struct i2c_client *client)
>  	if (chip_id < 0)
>  		return chip_id;
>  
> -	id = i2c_match_id(max16601_id, client);
> +	id = i2c_client_get_device_id(client);
>  	if (chip_id != id->driver_data)
>  		dev_warn(&client->dev,
>  			 "Device mismatch: Configured %s (%d), detected %d\n",
> -- 
> 2.39.2
> 

Re: [PATCH 08/11] hwmon: (pmbus/tps53679) Remove use of i2c_match_id()

[Guenter Roeck]

On Thu, Mar 05, 2026 at 01:56:39PM -0600, Andrew Davis wrote:
> The function i2c_match_id() is used to fetch the matching ID from
> the i2c_device_id table. This is often used to then retrieve the
> matching driver_data. This can be done in one step with the helper
> i2c_get_match_data().
> 
> This helper has a couple other benefits:
>  * It doesn't need the i2c_device_id passed in so we do not need
>    to have that forward declared, allowing us to remove those or
>    move the i2c_device_id table down to its more natural spot
>    with the other module info.

The patch does not actually remove the forward declaration or move 
the i2c_device_id table. At the very least this is misleading.
The same applies to other patches of the series as well.

>  * It also checks for device match data, which allows for OF and
>    ACPI based probing. That means we do not have to manually check
>    those first and can remove those checks.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/hwmon/pmbus/tps53679.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
> index ca2bfa25eb04c..df2726659a4ed 100644
> --- a/drivers/hwmon/pmbus/tps53679.c
> +++ b/drivers/hwmon/pmbus/tps53679.c
> @@ -253,10 +253,7 @@ static int tps53679_probe(struct i2c_client *client)
>  	struct pmbus_driver_info *info;
>  	enum chips chip_id;
>  
> -	if (dev->of_node)
> -		chip_id = (uintptr_t)of_device_get_match_data(dev);
> -	else
> -		chip_id = i2c_match_id(tps53679_id, client)->driver_data;
> +	chip_id = (uintptr_t)i2c_get_match_data(client);
>  
>  	info = devm_kmemdup(dev, &tps53679_info, sizeof(*info), GFP_KERNEL);
>  	if (!info)

Re: [PATCH 01/11] hwmon: (pmbus/bel-pfe) Remove use of i2c_match_id()

[Andrew Davis]

On 3/6/26 10:16 AM, Guenter Roeck wrote:
> On Thu, Mar 05, 2026 at 01:56:32PM -0600, Andrew Davis wrote:
>> The function i2c_match_id() is used to fetch the matching ID from
>> the i2c_device_id table. This is often used to then retrieve the
>> matching driver_data. This can be done in one step with the helper
>> i2c_get_match_data().
>>
>> This helper has a couple other benefits:
>>   * It doesn't need the i2c_device_id passed in so we do not need
>>     to have that forward declared, allowing us to remove those or
>>     move the i2c_device_id table down to its more natural spot
>>     with the other module info.
>>   * It also checks for device match data, which allows for OF and
>>     ACPI based probing. That means we do not have to manually check
>>     those first and can remove those checks.
> 
> The above seem to be boilerplates, not matching the actual patches.
> 
> AI review says:
> 
>    This isn't a bug, but this description is inaccurate for this specific driver.
>    Looking at the code before this commit, there were no manual checks for OF or
>    ACPI match data (like of_match_device or acpi_match_device) in
>    pfe_pmbus_probe(). The driver only ever relied on i2c_match_id().
>    Could the commit message be updated to avoid claiming it removes manual
>    OF/ACPI checks from this specific driver?
> 
> Similar feedback applies to most of the patches in this series. Please resend
> the series and drop inaccurate information from the commit descriptions.
> 

Yes, this one superset commit message was used for all the patches.
I'll go though and manually trim out the parts that are not relevant
for each given patch.

Thanks,
Andrew

> Thanks,
> Guenter
> 
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/hwmon/pmbus/bel-pfe.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
>> index ddf9d9a2958c4..6499556f735b4 100644
>> --- a/drivers/hwmon/pmbus/bel-pfe.c
>> +++ b/drivers/hwmon/pmbus/bel-pfe.c
>> @@ -88,13 +88,10 @@ static struct pmbus_driver_info pfe_driver_info[] = {
>>   	},
>>   };
>>   
>> -static const struct i2c_device_id pfe_device_id[];
>> -
>>   static int pfe_pmbus_probe(struct i2c_client *client)
>>   {
>> -	int model;
>> +	int model = (uintptr_t)i2c_get_match_data(client);
>>   
>> -	model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
>>   	client->dev.platform_data = &pfe_plat_data;
>>   
>>   	/*
>> -- 
>> 2.39.2
>>

Re: [PATCH 02/11] hwmon: (pmbus/ibm-cffps) Remove use of i2c_match_id()

[Andrew Davis]

On 3/6/26 10:19 AM, Guenter Roeck wrote:
> On Thu, Mar 05, 2026 at 01:56:33PM -0600, Andrew Davis wrote:
>> The function i2c_match_id() is used to fetch the matching ID from
>> the i2c_device_id table. This is often used to then retrieve the
>> matching driver_data. This can be done in one step with the helper
>> i2c_get_match_data().
>>
>> This helper has a couple other benefits:
>>   * It doesn't need the i2c_device_id passed in so we do not need
>>     to have that forward declared, allowing us to remove those or
>>     move the i2c_device_id table down to its more natural spot
>>     with the other module info.
>>   * It also checks for device match data, which allows for OF and
>>     ACPI based probing. That means we do not have to manually check
>>     those first and can remove those checks.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/hwmon/pmbus/ibm-cffps.c | 12 +-----------
>>   1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
>> index d05ef7a968a96..884b17de3b623 100644
>> --- a/drivers/hwmon/pmbus/ibm-cffps.c
>> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
>> @@ -482,19 +482,9 @@ MODULE_DEVICE_TABLE(i2c, ibm_cffps_id);
>>   static int ibm_cffps_probe(struct i2c_client *client)
>>   {
>>   	int i, rc;
>> -	enum versions vs = cffps_unknown;
>> +	enum versions vs = (uintptr_t)i2c_get_match_data(client);
> 
> AI feedback:
> 
>    Does this inadvertently change the fallback behavior for unmatched devices?
> 
>    In the old code, `vs` was initialized to `cffps_unknown` (which is 2). If a
>    device was instantiated manually via sysfs without a matching I2C name, both
>    `md` and `id` would be NULL, and `vs` remained `cffps_unknown`. This
>    correctly triggered the dynamic version detection logic using
>    `CFFPS_CCIN_CMD`.
> 
>    With the new code, if `i2c_get_match_data()` doesn't find a match, it returns
>    `NULL`. Since `NULL` casts to `0` (`cffps1`), the driver will now default to
>    `cffps1` and skip the dynamic version detection entirely.
> 
>    Additionally, because `cffps1` is `0`, `i2c_get_match_data()` cannot distinguish
>    between an explicit `cffps1` match (where `.data` is `0`) and no match at
>    all, since both result in returning `NULL`.
> 
>    To safely support fallback, `cffps1` could be assigned a non-zero value in
>    `enum versions` (e.g., `enum versions { cffps1 = 1, ... }`).
> 

This does seem valid. I will make `cffps_unknown` the 0th enum so existing
behavior is preserved.

Andrew

> Guenter
> 
>>   	struct dentry *debugfs;
>>   	struct ibm_cffps *psu;
>> -	const void *md = of_device_get_match_data(&client->dev);
>> -	const struct i2c_device_id *id;
>> -
>> -	if (md) {
>> -		vs = (uintptr_t)md;
>> -	} else {
>> -		id = i2c_match_id(ibm_cffps_id, client);
>> -		if (id)
>> -			vs = (enum versions)id->driver_data;
>> -	}
>>   
>>   	if (vs == cffps_unknown) {
>>   		u16 ccin_revision = 0;
>> -- 
>> 2.39.2
>>