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@xxxxxx>
> ---
>  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
>