Add tracepoints to i2c-core-base.c file to help trace and debug I2C device probe failures.
The new trace points are: - i2c_device_probe_debug: records non-failure routines e.g. IRQ 0. - i2c_device_probe_complete: records failed & successful probbes with appropriate trace message. To support operation of these tracepoints an enum was added that stores log message for debug and failure. Signed-off-by: Mohammad Gomaa <[email protected]> --- Hello, This patch adds tracepoints to i2c-core-base to aid with debugging I2C probing failrues. The motivation for this comes from my work in Google Summer of Code (GSoC) 2025: "ChromeOS Platform Input Device Quality Monitoring" https://summerofcode.withgoogle.com/programs/2025/projects/uCdIgK7K This is my first submission to the Linux kernel, so any feedback is welcome. --- Changes in v2: - Refactored i2c_device_probe_failed to i2c_device_probe_complete to support both successful and failed probes. - Fixed formatting for TRACE_EVENT(). - Used enum instead of string variable for "reason". - Link to v1: https://lore.kernel.org/r/20250806-refactor-add-i2c-tracepoints-v1-1-d1d39bd6f...@gmail.com --- drivers/i2c/i2c-core-base.c | 63 ++++++++++++++++++++++++-------- include/trace/events/i2c.h | 88 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 14 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index ecca8c006b020379fb53293b20ab09ba25314609..7ca22759d26e85ee51bea60f414ed014e9bcec40 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -495,6 +495,8 @@ static int i2c_device_probe(struct device *dev) struct i2c_driver *driver; bool do_power_on; int status; + int err_reason; + bool has_id_table, has_acpi_match, has_of_match; if (!client) return 0; @@ -509,38 +511,54 @@ static int i2c_device_probe(struct device *dev) /* Keep adapter active when Host Notify is required */ pm_runtime_get_sync(&client->adapter->dev); irq = i2c_smbus_host_notify_to_irq(client); + err_reason = I2C_TRACE_REASON_HOST_NOTIFY; } else if (is_of_node(fwnode)) { irq = fwnode_irq_get_byname(fwnode, "irq"); if (irq == -EINVAL || irq == -ENODATA) irq = fwnode_irq_get(fwnode, 0); + err_reason = I2C_TRACE_REASON_FROM_OF; } else if (is_acpi_device_node(fwnode)) { bool wake_capable; irq = i2c_acpi_get_irq(client, &wake_capable); if (irq > 0 && wake_capable) client->flags |= I2C_CLIENT_WAKE; + err_reason = I2C_TRACE_REASON_FROM_ACPI; } if (irq == -EPROBE_DEFER) { status = dev_err_probe(dev, irq, "can't get irq\n"); + err_reason = I2C_TRACE_REASON_EPROBE_DEFER_IRQ; goto put_sync_adapter; } - if (irq < 0) + if (irq < 0) { + trace_i2c_device_probe_debug(dev, err_reason); irq = 0; + } client->irq = irq; } driver = to_i2c_driver(dev->driver); + has_id_table = driver->id_table; + has_acpi_match = acpi_driver_match_device(dev, dev->driver); + has_of_match = i2c_of_match_device(dev->driver->of_match_table, client); + + if (!has_id_table) + trace_i2c_device_probe_debug(dev, I2C_TRACE_REASON_NO_I2C_ID_TABLE); + if (!has_acpi_match) + trace_i2c_device_probe_debug(dev, I2C_TRACE_REASON_ACPI_ID_MISMATCH); + if (!has_of_match) + trace_i2c_device_probe_debug(dev, I2C_TRACE_REASON_OF_ID_MISMATCH); + /* * An I2C ID table is not mandatory, if and only if, a suitable OF * or ACPI ID table is supplied for the probing device. */ - if (!driver->id_table && - !acpi_driver_match_device(dev, dev->driver) && - !i2c_of_match_device(dev->driver->of_match_table, client)) { + if (!has_id_table && !has_acpi_match && !has_of_match) { status = -ENODEV; + err_reason = I2C_TRACE_REASON_NO_ID_MATCH; goto put_sync_adapter; } @@ -550,47 +568,60 @@ static int i2c_device_probe(struct device *dev) wakeirq = fwnode_irq_get_byname(fwnode, "wakeup"); if (wakeirq == -EPROBE_DEFER) { status = dev_err_probe(dev, wakeirq, "can't get wakeirq\n"); + err_reason = I2C_TRACE_REASON_EPROBE_DEFER_WAKEIRQ; goto put_sync_adapter; } device_init_wakeup(&client->dev, true); - if (wakeirq > 0 && wakeirq != client->irq) + if (wakeirq > 0 && wakeirq != client->irq) { status = dev_pm_set_dedicated_wake_irq(dev, wakeirq); - else if (client->irq > 0) + err_reason = I2C_TRACE_REASON_SET_DED_WAKE_FAILED; + } else if (client->irq > 0) { status = dev_pm_set_wake_irq(dev, client->irq); - else + err_reason = I2C_TRACE_REASON_SET_WAKE_FAILED; + } else { status = 0; + err_reason = I2C_TRACE_REASON_NO_IRQ; + } - if (status) + if (status) { dev_warn(&client->dev, "failed to set up wakeup irq\n"); + trace_i2c_device_probe_debug(&client->dev, err_reason); + } } dev_dbg(dev, "probe\n"); status = of_clk_set_defaults(to_of_node(fwnode), false); - if (status < 0) + if (status < 0) { + err_reason = I2C_TRACE_REASON_SET_DEF_CLOCKS; goto err_clear_wakeup_irq; - + } do_power_on = !i2c_acpi_waive_d0_probe(dev); status = dev_pm_domain_attach(&client->dev, do_power_on ? PD_FLAG_ATTACH_POWER_ON : 0); - if (status) + if (status) { + err_reason = I2C_TRACE_REASON_ATTACH_PM_DOMAIN; goto err_clear_wakeup_irq; - + } client->devres_group_id = devres_open_group(&client->dev, NULL, GFP_KERNEL); if (!client->devres_group_id) { status = -ENOMEM; + err_reason = I2C_TRACE_REASON_OPEN_DEVRES_GROUP; goto err_detach_pm_domain; } client->debugfs = debugfs_create_dir(dev_name(&client->dev), client->adapter->debugfs); - if (driver->probe) + if (driver->probe) { status = driver->probe(client); - else + err_reason = I2C_TRACE_REASON_PROBE_FAILED; + } else { status = -EINVAL; + err_reason = I2C_TRACE_REASON_NO_PROBE; + } /* * Note that we are not closing the devres group opened above so @@ -603,6 +634,8 @@ static int i2c_device_probe(struct device *dev) if (status) goto err_release_driver_resources; + trace_i2c_device_probe_complete(&client->dev, status, err_reason); + return 0; err_release_driver_resources: @@ -617,6 +650,8 @@ static int i2c_device_probe(struct device *dev) if (client->flags & I2C_CLIENT_HOST_NOTIFY) pm_runtime_put_sync(&client->adapter->dev); + trace_i2c_device_probe_complete(&client->dev, status, err_reason); + return status; } diff --git a/include/trace/events/i2c.h b/include/trace/events/i2c.h index 142a23c6593c611de9abc2a89a146b95550b23cd..b00650ceba0a96a7cc538991ce5d5a45ea553715 100644 --- a/include/trace/events/i2c.h +++ b/include/trace/events/i2c.h @@ -16,6 +16,94 @@ /* * drivers/i2c/i2c-core-base.c */ +#ifndef I2C_TRACE_REASON_ENUM_DEFINED +#define I2C_TRACE_REASON_ENUM_DEFINED + +#define I2C_TRACE_REASON \ +EM(HOST_NOTIFY, "IRQ 0: could not get irq from Host Notify") \ +EM(FROM_OF, "IRQ 0: could not get irq from OF") \ +EM(FROM_ACPI, "IRQ 0: could not get irq from ACPI") \ +EM(EPROBE_DEFER_IRQ, "IRQ 0: could not get IRQ due to EPROBE_DEFER") \ +EM(NO_I2C_ID_TABLE, "no I2C ID table") \ +EM(ACPI_ID_MISMATCH, "ACPI ID table mismatch") \ +EM(OF_ID_MISMATCH, "OF ID table mismatch") \ +EM(NO_ID_MATCH, "no I2C ID table and no ACPI/OF match") \ +EM(EPROBE_DEFER_WAKEIRQ, "get wake IRQ due to EPROBE_DEFER") \ +EM(SET_DED_WAKE_FAILED, "failed to set dedicated wakeup IRQ") \ +EM(SET_WAKE_FAILED, "failed to set wakeup IRQ") \ +EM(NO_IRQ, "no IRQ") \ +EM(SET_DEF_CLOCKS, "set default clocks") \ +EM(ATTACH_PM_DOMAIN, "attach PM domain") \ +EM(OPEN_DEVRES_GROUP, "open devres group") \ +EM(PROBE_FAILED, "specific driver probe failed") \ +EMe(NO_PROBE, "no probe defined") + +#undef EM +#undef EMe +#define EM(a, b) I2C_TRACE_REASON_##a, +#define EMe(a, b) I2C_TRACE_REASON_##a +enum { + I2C_TRACE_REASON +}; + +#undef EM +#undef EMe +#define EM(a, b) TRACE_DEFINE_ENUM(I2C_TRACE_REASON_##a); +#define EMe(a, b) TRACE_DEFINE_ENUM(I2C_TRACE_REASON_##a); +I2C_TRACE_REASON + +#undef EM +#undef EMe +#define EM(a, b) { I2C_TRACE_REASON_##a, b }, +#define EMe(a, b) { I2C_TRACE_REASON_##a, b } + +#endif /* I2C_TRACE_REASON_ENUM_DEFINED */ + +TRACE_EVENT(i2c_device_probe_debug, + + TP_PROTO(struct device *dev, int err_reason), + + TP_ARGS(dev, err_reason), + + TP_STRUCT__entry( + __string( devname, dev_name(dev) ) + __field( int, err_reason ) + ), + + TP_fast_assign( + __assign_str(devname); + __entry->err_reason = err_reason; + ), + + TP_printk("device=%s: %s", + __get_str(devname), + __print_symbolic(__entry->err_reason, I2C_TRACE_REASON)) +); + +TRACE_EVENT(i2c_device_probe_complete, + + TP_PROTO(struct device *dev, int status, int err_reason), + + TP_ARGS(dev, status, err_reason), + + TP_STRUCT__entry( + __string( dev_name, dev_name(dev) ) + __field( int, status ) + __field( int, err_reason ) + ), + + TP_fast_assign( + __assign_str(dev_name); + __entry->status = status; + __entry->err_reason = err_reason; + ), + + TP_printk("%s probe %s: %s", + __get_str(dev_name), + __entry->status ? "failed" : "succeeded", + __entry->status ? __print_symbolic(__entry->err_reason, I2C_TRACE_REASON) : "") +); + extern int i2c_transfer_trace_reg(void); extern void i2c_transfer_trace_unreg(void); --- base-commit: 7e161a991ea71e6ec526abc8f40c6852ebe3d946 change-id: 20250806-refactor-add-i2c-tracepoints-b6e2b92d4cd9 Best regards, -- Mohammad Gomaa <[email protected]>
