Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH RFC/RFT] iio: imu: lsm6dsx: Use i2c_get_match_data()
>
> Hi Biju,
>
> On Sat, Aug 12, 2023 at 10:32 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > Replace device_get_match_data() and id lookup for retrieving match
> > data by i2c_get_match_data() by converting enum->pointer for data in
> > the match table.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > index 911444ec57c0..a2def435c9c2 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> > @@ -16,6 +16,30 @@
> >
> > #include "st_lsm6dsx.h"
> >
> > +static const int lsm6ds3 = ST_LSM6DS3_ID; static const int lsm6ds3h =
> > +ST_LSM6DS3H_ID;
>
> As these are single values, not structures with multiple members, I see
> not much value in adding all these variables, which increases kernel
> size...
OK.
>
> > @@ -23,12 +47,10 @@ static const struct regmap_config
> > st_lsm6dsx_i2c_regmap_config = {
> >
> > static int st_lsm6dsx_i2c_probe(struct i2c_client *client) {
> > - int hw_id;
> > + const int *hw_id;
> > struct regmap *regmap;
> >
> > - hw_id = (kernel_ulong_t)device_get_match_data(&client->dev);
> > - if (!hw_id)
> > - hw_id = i2c_client_get_device_id(client)->driver_data;
> > + hw_id = i2c_get_match_data(client);
> > if (!hw_id)
> > return -EINVAL;
>
> So just
>
> - hw_id = (kernel_ulong_t)device_get_match_data(&client->dev);
> - if (!hw_id)
> - hw_id = i2c_client_get_device_id(client)->driver_data;
> + hw_id = (kernel_ulong_t)i2c_get_match_data(client);
>
> and be done with it?
Agreed for this case. This will fit here.
There is a discussion related to extending device_match_data() to
buses [1]
[1] https://lore.kernel.org/all/20230807204505.5f3f245e@jic23-huawei/
Some maintainer's want legacy enums in ID table and pointer in OF table. There won't be uniform data across OF/ACPI/ID tables.
So i2c_get_match_data() and device_match_data() will fail in those cases.
Once case is here [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iio/magnetometer/ak8975.c?h=next-20230809#n936
Here name = id->name; which is making it not backward compatible.
By looking at the data in OF/I2C table (pointer vs enum) in the table, it needs to return NULL to make it backward compatible.
Cheers,
Biju
