| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250817-refactor-add-i2c-tracepoints-v2-1-c0bad299e02e@gmail.com>
Date: Sun, 17 Aug 2025 10:55:14 +0300
From: Mohammad Gomaa <midomaxgomaa@...il.com>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, kenalba@...gle.com,
hbarnor@...omium.org, rayxu@...gle.com,
Mohammad Gomaa <midomaxgomaa@...il.com>
Subject: [PATCH WIP v2] i2c: add tracepoints to aid debugging in
i2c-core-base
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 <midomaxgomaa@...il.com>
---
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-d1d39bd6fb57@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 <midomaxgomaa@...il.com>
Powered by blists - more mailing lists