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