RE: [PATCH RFC/RFT] iio: imu: lsm6dsx: Use i2c_get_match_data()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux