On Sun, 1 Sep 2024 13:04:59 +0800
jason liu <jasonliu10041728@xxxxxxxxx> wrote:
> Hi, I would like to know your current opinion on this patch.
>
> Let me explain the reason for proposing this patch.
>
> First, through __spi_register_driver, we know that when registering an SPI
> driver, if an id_table is present, it matches the id_table; otherwise, it
> matches the SPI device's driver_name.
> Then, in inv_icm_42600spi.c, driver name is "inv-icm42600-spi", but the
> compatible is compatible = "invensense,icm42600".
>
> so I think, it's necessary to add an id_table in the inv_icm42600 driver.
The quest Jean-Baptiste was raising was whether it would match on the
of_match_id table instead if it was present. __spi_register_driver
does that so I think perhaps what you are actually referring to is
the comment in there.
/*
* For Really Good Reasons we use spi: modaliases not of:
* modaliases for DT so module autoloading won't work if we
* don't have a spi_device_id as well as a compatible string.
*/
The actually matching is done in spi_match_device.
Anyhow, the comment suggests strongly that we do still need the
spi_device_id table. I'm not 100% sure on the i2c equivalent, but
I'm fine with adding both. Please fix up the formatting as requested
below and send a v2.
>
>
> Jonathan Cameron <jic23@xxxxxxxxxx> 于2024年8月26日周一 18:27写道：
>
> > On Mon, 26 Aug 2024 08:22:11 +0000
> > Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@xxxxxxx> wrote:
> >
> > > Hello,
> > >
> > > I was believing that id tables weren't required anymore when using of
> > tables.
> > >
> > > Jonathan,
> > > can you help on this subject?
> > >
> > > If we have to add id tables, then we need to add all supported chips
> > (missing here icm42686 and icm42688).
> >
> > There were some oddities around autoloading for some busses a while
> > back but I can't find the reference.
> >
> > +CC Mark + Wolfram for input.
> > Do we currently need i2c_device_id and spi_device_id tables for
> > autoprobing on DT only platforms?
> >
> > A few minor comments inline.
> >
> >
> >
> >
> > >
> > > Thanks,
> > > JB
> > >
> > > ________________________________________
> > > From: Jason Liu <jasonliu10041728@xxxxxxxxx>
> > > Sent: Sunday, August 25, 2024 08:39
> > > To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@xxxxxxx>
> > > Cc: jic23@xxxxxxxxxx <jic23@xxxxxxxxxx>; lars@xxxxxxxxxx <
> > lars@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx <linux-iio@xxxxxxxxxxxxxxx>;
> > linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; Jason Liu <
> > jasonliu10041728@xxxxxxxxx>
> > > Subject: [PATCH] iio/inv_icm42600: add inv_icm42600 id_table
> > >
> > > This Message Is From an Untrusted Sender
> > > You have not previously corresponded with this sender.
> > >
> > > Add the id_table of inv_icm42600, so the device can probe correctly.
> > >
> > > Signed-off-by: Jason Liu <jasonliu10041728@xxxxxxxxx>
> > > ---
> > > drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c | 15 +++++++++++++++
> > > drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c | 15 +++++++++++++++
> > > 2 files changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> > b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> > > index ebb31b385881..8cc550b8cfc3 100644
> > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> > > @@ -71,6 +71,20 @@ static int inv_icm42600_probe(struct i2c_client
> > *client)
> > > inv_icm42600_i2c_bus_setup);
> > > }
> > >
> > > +/*
> > > + * device id table is used to identify what device can be
> > > + * supported by this driver
> > > + */
> > > +static const struct i2c_device_id inv_icm42600_id[] = {
> > > + {"icm42600", INV_CHIP_ICM42600},
> > Spaces after { and before }
> > > + {"icm42602", INV_CHIP_ICM42602},
> > > + {"icm42605", INV_CHIP_ICM42605},
> > > + {"icm42622", INV_CHIP_ICM42622},
> > > + {"icm42631", INV_CHIP_ICM42631},
> > > + {}
> > { }
> >
> > I'm trying to standardize this in IIO.
> >
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, inv_icm42600_id);
> > > +
> > > static const struct of_device_id inv_icm42600_of_matches[] = {
> > > {
> > > .compatible = "invensense,icm42600",
> > > @@ -104,6 +118,7 @@ static struct i2c_driver inv_icm42600_driver = {
> > > .of_match_table = inv_icm42600_of_matches,
> > > .pm = pm_ptr(&inv_icm42600_pm_ops),
> > > },
> > > + .id_table = inv_icm42600_id,
> > > .probe = inv_icm42600_probe,
> > > };
> > > module_i2c_driver(inv_icm42600_driver);
> > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
> > b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
> > > index eae5ff7a3cc1..5fe078ddc8a1 100644
> > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
> > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
> > > @@ -67,6 +67,20 @@ static int inv_icm42600_probe(struct spi_device *spi)
> > > inv_icm42600_spi_bus_setup);
> > > }
> > >
> > > +/*
> > > + * device id table is used to identify what device can be
> > > + * supported by this driver
> > > + */
> > > +static const struct spi_device_id inv_icm42600_id[] = {
> > > + {"icm42600", INV_CHIP_ICM42600},
> > > + {"icm42602", INV_CHIP_ICM42602},
> > > + {"icm42605", INV_CHIP_ICM42605},
> > > + {"icm42622", INV_CHIP_ICM42622},
> > > + {"icm42631", INV_CHIP_ICM42631},
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(spi, inv_icm42600_id);
> > > +
> > > static const struct of_device_id inv_icm42600_of_matches[] = {
> > > {
> > > .compatible = "invensense,icm42600",
> > > @@ -100,6 +114,7 @@ static struct spi_driver inv_icm42600_driver = {
> > > .of_match_table = inv_icm42600_of_matches,
> > > .pm = pm_ptr(&inv_icm42600_pm_ops),
> > > },
> > > + .id_table = inv_icm42600_id,
> > > .probe = inv_icm42600_probe,
> > > };
> > > module_spi_driver(inv_icm42600_driver);
> >
> >
