Hi Dmitry,
On Mon, Jul 24, 2023 at 6:35 PM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Mon, Jul 24, 2023 at 03:06:50PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> > > callback
> > > On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > wrote:
> > > > Add i2c_device_get_match_data() callback to struct bus_type().
> > > >
> > > > While at it, introduced i2c_get_match_data_helper() to avoid code
> > > > duplication with i2c_get_match_data().
> > > >
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/i2c/i2c-core-base.c
> > > > +++ b/drivers/i2c/i2c-core-base.c
> > > > @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const
> > > > struct i2c_device_id *id, } EXPORT_SYMBOL_GPL(i2c_match_id);
> > > >
> > > > +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> > >
> > > static const void *
> >
> > I missed this.
> >
> > >
> > > > + const struct i2c_client
> > > > +*client) {
> > > > + const struct i2c_device_id *match;
> > > > +
> > > > + match = i2c_match_id(driver->id_table, client);
> > > > + if (!match)
> > > > + return NULL;
> > > > +
> > > > + return (const void *)match->driver_data;
> > >
> > > I guess your compiler didn't complain about the const/non-const
> > > conversion when returning because it inlined the function?
> >
> > It complained. Somehow, I didn't notice that warning before sending the patch.
> >
> > >
> > > > +}
> > > > +
> > > > +static const void *i2c_device_get_match_data(const struct device
> > > > +*dev) {
> > > > + const struct i2c_client *client = to_i2c_client(dev);
> >
> > Not sure, non-const i2c_verify_client(dev)to be used here??
>
> Good call, it actually should, as i2c bus contains instances of both
> i2c_client and i2c_adapter.
>
> Unfortunately i2c_verify_client() right now is a function, we might need
> to turn it into a macro to allow transparently handle const/non-const
> device argument... If this is too hard at the moment we could open-code
> i2c_verify_client() in i2c_device_get_match_data() and first check on
> the device type before doing to_i2c_client() conversion.
Tadah, we have _Generic()! See container_of_const():
https://elixir.bootlin.com/linux/latest/source/include/linux/container_of.h#L25
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
