Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v2 2/2] i2c: Add i2c_device_get_match_data()
> callback
>
> On Wed, Jul 26, 2023 at 10:56:41AM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 26, 2023 at 07:44:17PM +0300, Andy Shevchenko wrote:
> > > On Wed, Jul 26, 2023 at 02:08:04PM +0100, Biju Das wrote:
>
> ...
>
> > > > +static const void *i2c_device_get_match_data(const struct device
> > > > +*dev) {
> > > > + const struct i2c_client *client = (dev->type ==
> &i2c_client_type) ?
> > > > + to_i2c_client(dev) : NULL;
> > >
> > > There is an API i2c_verify_client() or something like this, I don't
> > > remember by heart.
> >
> > It's been discussed in a separate thread. i2c_verify_client() needs a
> > non-const pointer. It would be nice to clean up i2c_verify_client() to
> > accept both variants, but that can be done later.
>
> Then this code needs a TODO comment:
>
> /* TODO: use i2c_verify_client() when it accepts const pointer */
Agreed.
>
>
> > > > + if (!dev->driver)
> > > > + return NULL;
> > > > +
> > > > + return i2c_get_match_data_helper(to_i2c_driver(dev->driver),
> > > > +client); }
>
> ...
>
> > > Side question, what is the idea for i2c_of_match_device()? Shouldn't
> > > you also take it into consideration?
> >
> > Good call. I think we need to add something like
> >
> > if (!data && driver->driver.of_match_table) {
> > match =
> > i2c_of_match_device_sysfs(driver->driver.of_match_table, client);
> > if (match)
> > data = match->data;
> > }
> >
> > to i2c_device_get_match_data().
>
> Haven't checked myself, by I trust your suggestion. Let's see it in v3
> then.
OK, will send V3 to provide feedback.
Cheers,
Biju
