Re: [PATCH WIP v2] i2c: add tracepoints to aid debugging in i2c-core-base

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Hi Mohammad,

> We started by adding a bunch of `dev_err` and `dev_debug` then we moved
> to traces when we saw this Kernel Patch (note: it didn't get merged):
> https://patchwork.kernel.org/project/linux-input/patch/C1C54D7FA3DF3958+20250710073148.3994900-1-wangyuli@uniontech.com/

Tracepoints can make sense and we already have a few of them in the I2C
subsystem. Using them to propagate a new set of subsystem specific error
codes is not where tracepoints really shine IMHO.

> My point is: we can always circle back to using `dev_err` and `dev_debug`,
> while focusing on making the least amount of changes.

Good.

> Few examples of where we think adding an extra log line would be beneficial:
> - https://elixir.bootlin.com/linux/v6.6.94/source/drivers/i2c/i2c-core-base.c#L539-L544
> (log that no driver were matched)

If you enable debug printouts, it will be printed by the driver core.
Check the -ENODEV case of this function:
https://elixir.bootlin.com/linux/v6.6.94/source/drivers/base/dd.c#L572

At runtime, you can also check '/sys/bus/i2c/drivers/' if there are
symlinks to devices.

> - https://elixir.bootlin.com/linux/v6.6.94/source/drivers/i2c/i2c-core-base.c#L614
> (log that probing failed)

That really should be in the logs, because the same function as above
prints that with pr_warn (dev_err in recent kernels). No?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]