From: Uwe Kleine-König <[email protected]>
.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in the probe function.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 39f92eb1e698..8412b25eac86 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1665,9 +1665,9 @@ MODULE_ALIAS("spi:sc16is7xx");
#endif
#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
-static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
- const struct i2c_device_id *id)
+static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
{
+ const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
const struct sc16is7xx_devtype *devtype;
struct regmap *regmap;
@@ -1708,7 +1708,7 @@ static struct i2c_driver sc16is7xx_i2c_uart_driver = {
.name = SC16IS7XX_NAME,
.of_match_table = sc16is7xx_dt_ids,
},
- .probe = sc16is7xx_i2c_probe,
+ .probe_new = sc16is7xx_i2c_probe,
.remove = sc16is7xx_i2c_remove,
.id_table = sc16is7xx_i2c_id_table,
};
--
2.38.1
On 18. 11. 22, 23:45, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <[email protected]>
>
> .probe_new() doesn't get the i2c_device_id * parameter, so determine
> that explicitly in the probe function.
I wonder why -- is this a new approach to probe functions? Or is only
i2c affected? And why? Could you point to the commit introducing and
describing the change in the i2c core?
> Signed-off-by: Uwe Kleine-König <[email protected]>
> ---
> drivers/tty/serial/sc16is7xx.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 39f92eb1e698..8412b25eac86 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1665,9 +1665,9 @@ MODULE_ALIAS("spi:sc16is7xx");
> #endif
>
> #ifdef CONFIG_SERIAL_SC16IS7XX_I2C
> -static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
> - const struct i2c_device_id *id)
> +static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
> {
> + const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
> const struct sc16is7xx_devtype *devtype;
> struct regmap *regmap;
>
> @@ -1708,7 +1708,7 @@ static struct i2c_driver sc16is7xx_i2c_uart_driver = {
> .name = SC16IS7XX_NAME,
> .of_match_table = sc16is7xx_dt_ids,
> },
> - .probe = sc16is7xx_i2c_probe,
> + .probe_new = sc16is7xx_i2c_probe,
> .remove = sc16is7xx_i2c_remove,
> .id_table = sc16is7xx_i2c_id_table,
> };
thanks,
--
js
Hello Jiri,
On Mon, Nov 21, 2022 at 07:03:41AM +0100, Jiri Slaby wrote:
> On 18. 11. 22, 23:45, Uwe Kleine-K?nig wrote:
> > From: Uwe Kleine-K?nig <[email protected]>
> >
> > .probe_new() doesn't get the i2c_device_id * parameter, so determine
> > that explicitly in the probe function.
>
> I wonder why -- is this a new approach to probe functions? Or is only i2c
> affected? And why? Could you point to the commit introducing and describing
> the change in the i2c core?
I didn't sent the cover letter to all recipents of the individual
patches, so flow of information is a bit rough. Sorry about that.
You can find it at
https://lore.kernel.org/lkml/[email protected]/,
it should answer your question.
The short version is: The i2c framework does a more or less expensive
lookup for each call to .probe() to provide the id parameter. A relevant
part of the drivers however doesn't use this parameter, so the idea is
to let the drivers who actually need it, determine it themselves.
Statistics for the current state of this series in my tree:
Among the 602 converted drivers, 404 don't make use of the parameter.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hi,
On 21. 11. 22, 8:07, Uwe Kleine-König wrote:
> Hello Jiri,
>
> On Mon, Nov 21, 2022 at 07:03:41AM +0100, Jiri Slaby wrote:
>> On 18. 11. 22, 23:45, Uwe Kleine-König wrote:
>>> From: Uwe Kleine-König <[email protected]>
>>>
>>> .probe_new() doesn't get the i2c_device_id * parameter, so determine
>>> that explicitly in the probe function.
>>
>> I wonder why -- is this a new approach to probe functions? Or is only i2c
>> affected? And why? Could you point to the commit introducing and describing
>> the change in the i2c core?
>
> I didn't sent the cover letter to all recipents of the individual
> patches, so flow of information is a bit rough. Sorry about that.
>
> You can find it at
> https://lore.kernel.org/lkml/[email protected]/,
> it should answer your question.
Yes, I looked up that beforehand, but was no more clever after reading it.
> The short version is: The i2c framework does a more or less expensive
> lookup for each call to .probe() to provide the id parameter. A relevant
> part of the drivers however doesn't use this parameter, so the idea is
> to let the drivers who actually need it, determine it themselves.
>
> Statistics for the current state of this series in my tree:
> Among the 602 converted drivers, 404 don't make use of the parameter.
So doesn't it make sense to provide both probe with no id and "probe_id"
then? 200 is quite a few (a third to be precise).
BTW is this a performance issue? I.e. does it slow down the boot?
thanks,
--
js
suse labs
Hello Jiri,
On Wed, Nov 23, 2022 at 07:36:52AM +0100, Jiri Slaby wrote:
> On 21. 11. 22, 8:07, Uwe Kleine-K?nig wrote:
> > On Mon, Nov 21, 2022 at 07:03:41AM +0100, Jiri Slaby wrote:
> > > On 18. 11. 22, 23:45, Uwe Kleine-K?nig wrote:
> > > > From: Uwe Kleine-K?nig <[email protected]>
> > > >
> > > > .probe_new() doesn't get the i2c_device_id * parameter, so determine
> > > > that explicitly in the probe function.
> > >
> > > I wonder why -- is this a new approach to probe functions? Or is only i2c
> > > affected? And why? Could you point to the commit introducing and describing
> > > the change in the i2c core?
> >
> > I didn't sent the cover letter to all recipents of the individual
> > patches, so flow of information is a bit rough. Sorry about that.
> >
> > You can find it at
> > https://lore.kernel.org/lkml/[email protected]/,
> > it should answer your question.
>
> Yes, I looked up that beforehand, but was no more clever after reading it.
>
> > The short version is: The i2c framework does a more or less expensive
> > lookup for each call to .probe() to provide the id parameter. A relevant
> > part of the drivers however doesn't use this parameter, so the idea is
> > to let the drivers who actually need it, determine it themselves.
> >
> > Statistics for the current state of this series in my tree:
> > Among the 602 converted drivers, 404 don't make use of the parameter.
>
> So doesn't it make sense to provide both probe with no id and "probe_id"
> then? 200 is quite a few (a third to be precise).
Having the probe callback with the id parameter is only temporary. As
soon as all drivers are converted, the variant with the id parameter
will go away.
> BTW is this a performance issue? I.e. does it slow down the boot?
I don't know the start motivation for Lee (who triggered the conversion
in b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back
type")).
Looking at the git history, he created 1e98dcd77970 ("mfd: 88pm860x:
Move over to new I2C device .probe() call") converting a driver that
doesn't benefit immensely. The lookup is more expensive for drivers with
big .id_table, the converted driver has only one entry.
I think in the end is a mixture between:
- A big part of the drivers doesn't benefit from the lookup.
- For most other busses the probe function only gets a device parameter
and no id (spi, platform, i3c). There are counter examples though:
amba, usb. Didn't check further.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Wed, Nov 23, 2022 at 09:09:12AM +0100, Uwe Kleine-K?nig wrote:
> On Wed, Nov 23, 2022 at 07:36:52AM +0100, Jiri Slaby wrote:
> > On 21. 11. 22, 8:07, Uwe Kleine-K?nig wrote:
> > > On Mon, Nov 21, 2022 at 07:03:41AM +0100, Jiri Slaby wrote:
> > > > On 18. 11. 22, 23:45, Uwe Kleine-K?nig wrote:
> > > > > From: Uwe Kleine-K?nig <[email protected]>
> > > > >
> > > > > .probe_new() doesn't get the i2c_device_id * parameter, so determine
> > > > > that explicitly in the probe function.
> > > >
> > > > I wonder why -- is this a new approach to probe functions? Or is only i2c
> > > > affected? And why? Could you point to the commit introducing and describing
> > > > the change in the i2c core?
> > >
> > > I didn't sent the cover letter to all recipents of the individual
> > > patches, so flow of information is a bit rough. Sorry about that.
> > >
> > > You can find it at
> > > https://lore.kernel.org/lkml/[email protected]/,
> > > it should answer your question.
> >
> > Yes, I looked up that beforehand, but was no more clever after reading it.
> >
> > > The short version is: The i2c framework does a more or less expensive
> > > lookup for each call to .probe() to provide the id parameter. A relevant
> > > part of the drivers however doesn't use this parameter, so the idea is
> > > to let the drivers who actually need it, determine it themselves.
> > >
> > > Statistics for the current state of this series in my tree:
> > > Among the 602 converted drivers, 404 don't make use of the parameter.
> >
> > So doesn't it make sense to provide both probe with no id and "probe_id"
> > then? 200 is quite a few (a third to be precise).
>
> Having the probe callback with the id parameter is only temporary. As
> soon as all drivers are converted, the variant with the id parameter
> will go away.
>
> > BTW is this a performance issue? I.e. does it slow down the boot?
>
> I don't know the start motivation for Lee (who triggered the conversion
> in b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back
> type")).
> Looking at the git history, he created 1e98dcd77970 ("mfd: 88pm860x:
> Move over to new I2C device .probe() call") converting a driver that
> doesn't benefit immensely. The lookup is more expensive for drivers with
> big .id_table, the converted driver has only one entry.
>
> I think in the end is a mixture between:
>
> - A big part of the drivers doesn't benefit from the lookup.
> - For most other busses the probe function only gets a device parameter
> and no id (spi, platform, i3c). There are counter examples though:
> amba, usb. Didn't check further.
The discussion somehow ended here without a real result.
As of today's next master there are only 9 drivers left using .probe().
So I'd like to stop this discussion and ask to apply the conversion for
the sc16is7xx driver to be able to complete the conversion.
My plan is to drop the .probe callback as it is today after the next
merge window. So I ask the serial maintainers to either take the patch
under discussion for the next merge window or accept that the conversion
is done together with the patch that drops .probe() that probably will
go in via the i2c tree.
Best regards and thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, Jan 27, 2023 at 11:10:25AM +0100, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Wed, Nov 23, 2022 at 09:09:12AM +0100, Uwe Kleine-K?nig wrote:
> > On Wed, Nov 23, 2022 at 07:36:52AM +0100, Jiri Slaby wrote:
> > > On 21. 11. 22, 8:07, Uwe Kleine-K?nig wrote:
> > > > On Mon, Nov 21, 2022 at 07:03:41AM +0100, Jiri Slaby wrote:
> > > > > On 18. 11. 22, 23:45, Uwe Kleine-K?nig wrote:
> > > > > > From: Uwe Kleine-K?nig <[email protected]>
> > > > > >
> > > > > > .probe_new() doesn't get the i2c_device_id * parameter, so determine
> > > > > > that explicitly in the probe function.
> > > > >
> > > > > I wonder why -- is this a new approach to probe functions? Or is only i2c
> > > > > affected? And why? Could you point to the commit introducing and describing
> > > > > the change in the i2c core?
> > > >
> > > > I didn't sent the cover letter to all recipents of the individual
> > > > patches, so flow of information is a bit rough. Sorry about that.
> > > >
> > > > You can find it at
> > > > https://lore.kernel.org/lkml/[email protected]/,
> > > > it should answer your question.
> > >
> > > Yes, I looked up that beforehand, but was no more clever after reading it.
> > >
> > > > The short version is: The i2c framework does a more or less expensive
> > > > lookup for each call to .probe() to provide the id parameter. A relevant
> > > > part of the drivers however doesn't use this parameter, so the idea is
> > > > to let the drivers who actually need it, determine it themselves.
> > > >
> > > > Statistics for the current state of this series in my tree:
> > > > Among the 602 converted drivers, 404 don't make use of the parameter.
> > >
> > > So doesn't it make sense to provide both probe with no id and "probe_id"
> > > then? 200 is quite a few (a third to be precise).
> >
> > Having the probe callback with the id parameter is only temporary. As
> > soon as all drivers are converted, the variant with the id parameter
> > will go away.
> >
> > > BTW is this a performance issue? I.e. does it slow down the boot?
> >
> > I don't know the start motivation for Lee (who triggered the conversion
> > in b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back
> > type")).
> > Looking at the git history, he created 1e98dcd77970 ("mfd: 88pm860x:
> > Move over to new I2C device .probe() call") converting a driver that
> > doesn't benefit immensely. The lookup is more expensive for drivers with
> > big .id_table, the converted driver has only one entry.
> >
> > I think in the end is a mixture between:
> >
> > - A big part of the drivers doesn't benefit from the lookup.
> > - For most other busses the probe function only gets a device parameter
> > and no id (spi, platform, i3c). There are counter examples though:
> > amba, usb. Didn't check further.
>
> The discussion somehow ended here without a real result.
>
> As of today's next master there are only 9 drivers left using .probe().
> So I'd like to stop this discussion and ask to apply the conversion for
> the sc16is7xx driver to be able to complete the conversion.
>
> My plan is to drop the .probe callback as it is today after the next
> merge window. So I ask the serial maintainers to either take the patch
> under discussion for the next merge window or accept that the conversion
> is done together with the patch that drops .probe() that probably will
> go in via the i2c tree.
I don't see the patch anymore, so I have no objection for it going
through the i2c tree.
thanks,
greg k-h
Hello Greg,
On Fri, Jan 27, 2023 at 12:45:52PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Jan 27, 2023 at 11:10:25AM +0100, Uwe Kleine-K?nig wrote:
> > On Wed, Nov 23, 2022 at 09:09:12AM +0100, Uwe Kleine-K?nig wrote:
> > > On Wed, Nov 23, 2022 at 07:36:52AM +0100, Jiri Slaby wrote:
> > > > BTW is this a performance issue? I.e. does it slow down the boot?
> > >
> > > I don't know the start motivation for Lee (who triggered the conversion
> > > in b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back
> > > type")).
> > > Looking at the git history, he created 1e98dcd77970 ("mfd: 88pm860x:
> > > Move over to new I2C device .probe() call") converting a driver that
> > > doesn't benefit immensely. The lookup is more expensive for drivers with
> > > big .id_table, the converted driver has only one entry.
> > >
> > > I think in the end is a mixture between:
> > >
> > > - A big part of the drivers doesn't benefit from the lookup.
> > > - For most other busses the probe function only gets a device parameter
> > > and no id (spi, platform, i3c). There are counter examples though:
> > > amba, usb. Didn't check further.
> >
> > The discussion somehow ended here without a real result.
> >
> > As of today's next master there are only 9 drivers left using .probe().
> > So I'd like to stop this discussion and ask to apply the conversion for
> > the sc16is7xx driver to be able to complete the conversion.
> >
> > My plan is to drop the .probe callback as it is today after the next
> > merge window. So I ask the serial maintainers to either take the patch
> > under discussion for the next merge window or accept that the conversion
> > is done together with the patch that drops .probe() that probably will
> > go in via the i2c tree.
>
> I don't see the patch anymore,
If you want to take a look:
b4 am [email protected]
or
https://lore.kernel.org/lkml/[email protected]
> so I have no objection for it going through the i2c tree.
Can I interpret that as an Ack? :-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, Jan 27, 2023 at 02:17:51PM +0100, Uwe Kleine-K?nig wrote:
> Hello Greg,
>
> On Fri, Jan 27, 2023 at 12:45:52PM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Jan 27, 2023 at 11:10:25AM +0100, Uwe Kleine-K?nig wrote:
> > > On Wed, Nov 23, 2022 at 09:09:12AM +0100, Uwe Kleine-K?nig wrote:
> > > > On Wed, Nov 23, 2022 at 07:36:52AM +0100, Jiri Slaby wrote:
> > > > > BTW is this a performance issue? I.e. does it slow down the boot?
> > > >
> > > > I don't know the start motivation for Lee (who triggered the conversion
> > > > in b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back
> > > > type")).
> > > > Looking at the git history, he created 1e98dcd77970 ("mfd: 88pm860x:
> > > > Move over to new I2C device .probe() call") converting a driver that
> > > > doesn't benefit immensely. The lookup is more expensive for drivers with
> > > > big .id_table, the converted driver has only one entry.
> > > >
> > > > I think in the end is a mixture between:
> > > >
> > > > - A big part of the drivers doesn't benefit from the lookup.
> > > > - For most other busses the probe function only gets a device parameter
> > > > and no id (spi, platform, i3c). There are counter examples though:
> > > > amba, usb. Didn't check further.
> > >
> > > The discussion somehow ended here without a real result.
> > >
> > > As of today's next master there are only 9 drivers left using .probe().
> > > So I'd like to stop this discussion and ask to apply the conversion for
> > > the sc16is7xx driver to be able to complete the conversion.
> > >
> > > My plan is to drop the .probe callback as it is today after the next
> > > merge window. So I ask the serial maintainers to either take the patch
> > > under discussion for the next merge window or accept that the conversion
> > > is done together with the patch that drops .probe() that probably will
> > > go in via the i2c tree.
> >
> > I don't see the patch anymore,
>
> If you want to take a look:
>
> b4 am [email protected]
>
> or
>
> https://lore.kernel.org/lkml/[email protected]
>
> > so I have no objection for it going through the i2c tree.
>
> Can I interpret that as an Ack? :-)
Even better:
Reviewed-by: Greg Kroah-Hartman <[email protected]>