* RE: [PATCH v3] i2c: Add i2c_get_match_data() [not found] <20230606130519.382304-1-biju.das.jz@bp.renesas.com> @ 2023-06-07 7:21 ` Biju Das 2023-06-07 9:28 ` Geert Uytterhoeven 2023-06-07 10:17 ` Wolfram Sang 0 siblings, 2 replies; 6+ messages in thread From: Biju Das @ 2023-06-07 7:21 UTC (permalink / raw) To: Wolfram Sang, Alessandro Zummo, Alexandre Belloni, Geert Uytterhoeven Cc: linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org Hi Wolfram/Geert, Do we need to enhance the logic to use device_get_match_data to support OF/ACPI/I2C match like below [1]. Or Are we happy with the current one? + Mark, Linux-clk If I am correct, the new enhancement[1] will add I2C match support [2] drivers. Currently this driver support only OF/ACPI match-data even though driver has I2C matching table. [2] https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-renesas-pcie.c#L282 The current patch will be useful for these drivers as well for I2C match support as these drivers have I2C matching table. [3] https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-versaclock5.c#L956 [4] https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-versaclock7.c#L1111 [1] +const void *i2c_get_match_data(const struct i2c_client *client) { + struct device_driver *drv = client->dev.driver; + struct i2c_driver *driver = to_i2c_driver(drv); + const struct i2c_device_id *match; + const void *data; + + data = device_get_match_data(&client->dev); + if (!data) { + match = i2c_match_id(driver->id_table, client); + if (!match) + return NULL; + + data = (const void *)match->driver_data; + } + > + return data; > +} > +EXPORT_SYMBOL(i2c_get_match_data); Cheers, Biju > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: Tuesday, June 6, 2023 2:05 PM > To: Wolfram Sang <wsa@kernel.org>; Alessandro Zummo > <a.zummo@towertech.it>; Alexandre Belloni > <alexandre.belloni@bootlin.com> > Cc: Biju Das <biju.das.jz@bp.renesas.com>; linux-i2c@vger.kernel.org; > linux-rtc@vger.kernel.org; Geert Uytterhoeven <geert+renesas@glider.be>; > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux- > renesas-soc@vger.kernel.org > Subject: [PATCH v3] i2c: Add i2c_get_match_data() > > Add i2c_get_match_data() to get match data for both I2C and DT-based > matching, so that we can optimize the driver code that uses both. > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > v2->v3: > * Added support for getting match data for both I2C and DT-based > matching. > * Added Rb tag from Geert and retained the Rb tag as change is trivial. > v1->v2: > * Dropped parameter const struct i2c_device_id *id and the helper > function. > > eg: The RTC pcf85063/isl1208 driver code can be optimized with this > patch. > - if (client->dev.of_node) { > - config = of_device_get_match_data(&client->dev); > - if (!config) > - return -ENODEV; > - } else { > - enum pcf85063_type type = > - i2c_match_id(pcf85063_ids, client)->driver_data; > - if (type >= PCF85063_LAST_ID) > - return -ENODEV; > - config = &pcf85063_cfg[type]; > - } > + config = i2c_get_match_data(client); > + if (!config) > + return -ENODEV; > --- > drivers/i2c/i2c-core-base.c | 21 +++++++++++++++++++++ > include/linux/i2c.h | 2 ++ > 2 files changed, 23 insertions(+) > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > index ae3af738b03f..15a49f4ba668 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -114,6 +114,27 @@ const struct i2c_device_id *i2c_match_id(const > struct i2c_device_id *id, } EXPORT_SYMBOL_GPL(i2c_match_id); > > +const void *i2c_get_match_data(const struct i2c_client *client) { > + struct device_driver *drv = client->dev.driver; > + struct i2c_driver *driver = to_i2c_driver(drv); > + const struct i2c_device_id *match; > + const void *data; > + > + if (client->dev.of_node) { > + data = of_device_get_match_data(&client->dev); > + } else { > + match = i2c_match_id(driver->id_table, client); > + if (!match) > + return NULL; > + > + data = (const void *)match->driver_data; > + } > + > + return data; > +} > +EXPORT_SYMBOL(i2c_get_match_data); > + > static int i2c_device_match(struct device *dev, struct device_driver > *drv) { > struct i2c_client *client = i2c_verify_client(dev); > diff --git a/include/linux/i2c.h b/include/linux/i2c.h index > 13a1ce38cb0c..3430cc2b05a6 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -367,6 +367,8 @@ struct i2c_adapter *i2c_verify_adapter(struct device > *dev); const struct i2c_device_id *i2c_match_id(const struct > i2c_device_id *id, > const struct i2c_client *client); > > +const void *i2c_get_match_data(const struct i2c_client *client); > + > static inline struct i2c_client *kobj_to_i2c_client(struct kobject > *kobj) { > struct device * const dev = kobj_to_dev(kobj); > -- > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: Add i2c_get_match_data() 2023-06-07 7:21 ` [PATCH v3] i2c: Add i2c_get_match_data() Biju Das @ 2023-06-07 9:28 ` Geert Uytterhoeven 2023-06-07 9:36 ` Biju Das 2023-06-07 10:20 ` Wolfram Sang 2023-06-07 10:17 ` Wolfram Sang 1 sibling, 2 replies; 6+ messages in thread From: Geert Uytterhoeven @ 2023-06-07 9:28 UTC (permalink / raw) To: Biju Das Cc: Wolfram Sang, Alessandro Zummo, Alexandre Belloni, linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org Hi Biju, On Wed, Jun 7, 2023 at 9:21 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Do we need to enhance the logic to use device_get_match_data > to support OF/ACPI/I2C match like below [1]. Why not? > > Or > > Are we happy with the current one? I don't mind. > +const void *i2c_get_match_data(const struct i2c_client *client) { > + struct device_driver *drv = client->dev.driver; > + struct i2c_driver *driver = to_i2c_driver(drv); > + const struct i2c_device_id *match; > + const void *data; > + > + data = device_get_match_data(&client->dev); if (data) return data; > + if (!data) { > + match = i2c_match_id(driver->id_table, client); > + if (!match) > + return NULL; > + > + data = (const void *)match->driver_data; > + } > + > > + return data; > > +} > > +EXPORT_SYMBOL(i2c_get_match_data); > > From: Biju Das <biju.das.jz@bp.renesas.com> > > --- a/drivers/i2c/i2c-core-base.c > > +++ b/drivers/i2c/i2c-core-base.c > > @@ -114,6 +114,27 @@ const struct i2c_device_id *i2c_match_id(const > > struct i2c_device_id *id, } EXPORT_SYMBOL_GPL(i2c_match_id); > > > > +const void *i2c_get_match_data(const struct i2c_client *client) { > > + struct device_driver *drv = client->dev.driver; > > + struct i2c_driver *driver = to_i2c_driver(drv); > > + const struct i2c_device_id *match; > > + const void *data; > > + > > + if (client->dev.of_node) { > > + data = of_device_get_match_data(&client->dev); return of_device_get_match_data(&client->dev); > > + } else { > > + match = i2c_match_id(driver->id_table, client); > > + if (!match) > > + return NULL; > > + > > + data = (const void *)match->driver_data; return ... > > + } > > + > > + return data; > > +} > > +EXPORT_SYMBOL(i2c_get_match_data); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] i2c: Add i2c_get_match_data() 2023-06-07 9:28 ` Geert Uytterhoeven @ 2023-06-07 9:36 ` Biju Das 2023-06-07 10:20 ` Wolfram Sang 1 sibling, 0 replies; 6+ messages in thread From: Biju Das @ 2023-06-07 9:36 UTC (permalink / raw) To: Geert Uytterhoeven, Wolfram Sang Cc: Alessandro Zummo, Alexandre Belloni, linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v3] i2c: Add i2c_get_match_data() > > Hi Biju, > > On Wed, Jun 7, 2023 at 9:21 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: Wolfram, What is your preference in supporting OF/ACPI/I2C match-data as in [A]? Or Just support OF/i2C match-data as in [B]? > > Do we need to enhance the logic to use device_get_match_data to > > support OF/ACPI/I2C match like below [1]. > > Why not? > > > > > Or > > > > Are we happy with the current one? > > I don't mind. > [A] > > +const void *i2c_get_match_data(const struct i2c_client *client) { > > + struct device_driver *drv = client->dev.driver; > > + struct i2c_driver *driver = to_i2c_driver(drv); > > + const struct i2c_device_id *match; > > + const void *data; > > + > > + data = device_get_match_data(&client->dev); > > if (data) > return data; > > > + if (!data) { > > + match = i2c_match_id(driver->id_table, client); > > + if (!match) > > + return NULL; > > + > > + data = (const void *)match->driver_data; > > + } > > + > > > + return data; > > > +} > > > +EXPORT_SYMBOL(i2c_get_match_data); > > > > From: Biju Das <biju.das.jz@bp.renesas.com> > > > > --- a/drivers/i2c/i2c-core-base.c > > > +++ b/drivers/i2c/i2c-core-base.c > > > @@ -114,6 +114,27 @@ const struct i2c_device_id *i2c_match_id(const > > > struct i2c_device_id *id, } EXPORT_SYMBOL_GPL(i2c_match_id); > > > [B] > > > +const void *i2c_get_match_data(const struct i2c_client *client) { > > > + struct device_driver *drv = client->dev.driver; > > > + struct i2c_driver *driver = to_i2c_driver(drv); > > > + const struct i2c_device_id *match; > > > + const void *data; > > > + > > > + if (client->dev.of_node) { > > > + data = of_device_get_match_data(&client->dev); > > return of_device_get_match_data(&client->dev); > > > > + } else { > > > + match = i2c_match_id(driver->id_table, client); > > > + if (!match) > > > + return NULL; > > > + > > > + data = (const void *)match->driver_data; > > return ... > > > > + } > > > + > > > + return data; > > > +} > > > +EXPORT_SYMBOL(i2c_get_match_data); > Cheers, Biju ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: Add i2c_get_match_data() 2023-06-07 9:28 ` Geert Uytterhoeven 2023-06-07 9:36 ` Biju Das @ 2023-06-07 10:20 ` Wolfram Sang 2023-06-07 16:47 ` Biju Das 1 sibling, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2023-06-07 10:20 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Biju Das, Alessandro Zummo, Alexandre Belloni, linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 540 bytes --] > > +const void *i2c_get_match_data(const struct i2c_client *client) { > > + struct device_driver *drv = client->dev.driver; > > + struct i2c_driver *driver = to_i2c_driver(drv); > > + const struct i2c_device_id *match; > > + const void *data; > > + > > + data = device_get_match_data(&client->dev); > > if (data) > return data; I like Biju's version a tad more. Except for errors, and especially within small functions, I think single exit points are easier to understand. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] i2c: Add i2c_get_match_data() 2023-06-07 10:20 ` Wolfram Sang @ 2023-06-07 16:47 ` Biju Das 0 siblings, 0 replies; 6+ messages in thread From: Biju Das @ 2023-06-07 16:47 UTC (permalink / raw) To: Wolfram Sang, Geert Uytterhoeven Cc: Alessandro Zummo, Alexandre Belloni, linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org Hi Wolfram, > Subject: Re: [PATCH v3] i2c: Add i2c_get_match_data() > > > > > +const void *i2c_get_match_data(const struct i2c_client *client) { > > > + struct device_driver *drv = client->dev.driver; > > > + struct i2c_driver *driver = to_i2c_driver(drv); > > > + const struct i2c_device_id *match; > > > + const void *data; > > > + > > > + data = device_get_match_data(&client->dev); > > > > if (data) > > return data; > > I like Biju's version a tad more. Except for errors, and especially > within small functions, I think single exit points are easier to > understand. OK, will send V4. Cheers, Biju ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: Add i2c_get_match_data() 2023-06-07 7:21 ` [PATCH v3] i2c: Add i2c_get_match_data() Biju Das 2023-06-07 9:28 ` Geert Uytterhoeven @ 2023-06-07 10:17 ` Wolfram Sang 1 sibling, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2023-06-07 10:17 UTC (permalink / raw) To: Biju Das Cc: Alessandro Zummo, Alexandre Belloni, Geert Uytterhoeven, linux-i2c@vger.kernel.org, linux-rtc@vger.kernel.org, Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org, Marek Vasut, linux-clk@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 397 bytes --] Hi Biju, > Do we need to enhance the logic to use device_get_match_data > to support OF/ACPI/I2C match like below [1]. Sure, the more the better. With your original patch, I thought we can add it incrementally when someone needs it and can test it. But using device_get_match_data() we already have a well-tested helper we can use right away. So, let's do that IMO. Happy hacking, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-07 16:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230606130519.382304-1-biju.das.jz@bp.renesas.com>
2023-06-07 7:21 ` [PATCH v3] i2c: Add i2c_get_match_data() Biju Das
2023-06-07 9:28 ` Geert Uytterhoeven
2023-06-07 9:36 ` Biju Das
2023-06-07 10:20 ` Wolfram Sang
2023-06-07 16:47 ` Biju Das
2023-06-07 10:17 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox