* [RFC Patch v5 0/1] i2c: core: Adapter and client stats as sysfs attributes
@ 2022-05-17 10:05 ` Sui Chen
0 siblings, 0 replies; 6+ messages in thread
From: Sui Chen @ 2022-05-17 10:05 UTC (permalink / raw)
To: linux-kernel, linux-i2c, wsa, openbmc, tali.perry1
Cc: joel, andrew, benjaminfair, krellan, Sui Chen
This change adds statistics to the i2c_adapter structure as Wolfram
previously suggested
(https://lore.kernel.org/linux-i2c/YgEYEk355t8C4J1x@shikoro/).
It also adds relevant statistics to the per-address i2c_clients where
applicable.
The list of statistics are:
- bus_errors
- nacks
- recovery_successes / recovery_failures (only applicable to
i2c_adapter)
- timeouts
- messages (only applicable to i2c_client)
- transfers (only applicable to i2c_adapter)
The statistics are located in /sys/class/i2c-adapter/i2c-x/stats and
/sys/class/i2c-adapter/i2c-x/x-xxxx/stats respectively.
Since the counting is done in __i2c_transfer, where the number of
messages transferred is not known upon error, the error counters are
attributed to all unique addresses that appear in the message list
passed into __i2c_transfer.
Currently an rbtree is used to find the i2c_client located at a certain
address. Would be happy to know if there is a better way of doing this.
Thanks!
Sui Chen (1):
i2c debug counters as sysfs attributes
drivers/i2c/i2c-core-base.c | 240 +++++++++++++++++++++++++++++++++++-
drivers/i2c/i2c-dev.c | 94 ++++++++++++++
include/linux/i2c.h | 41 ++++++
3 files changed, 374 insertions(+), 1 deletion(-)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC Patch v5 0/1] i2c: core: Adapter and client stats as sysfs attributes
@ 2022-05-17 10:05 ` Sui Chen
0 siblings, 0 replies; 6+ messages in thread
From: Sui Chen @ 2022-05-17 10:05 UTC (permalink / raw)
To: linux-kernel, linux-i2c, wsa, openbmc, tali.perry1
Cc: andrew, krellan, Sui Chen, joel, benjaminfair
This change adds statistics to the i2c_adapter structure as Wolfram
previously suggested
(https://lore.kernel.org/linux-i2c/YgEYEk355t8C4J1x@shikoro/).
It also adds relevant statistics to the per-address i2c_clients where
applicable.
The list of statistics are:
- bus_errors
- nacks
- recovery_successes / recovery_failures (only applicable to
i2c_adapter)
- timeouts
- messages (only applicable to i2c_client)
- transfers (only applicable to i2c_adapter)
The statistics are located in /sys/class/i2c-adapter/i2c-x/stats and
/sys/class/i2c-adapter/i2c-x/x-xxxx/stats respectively.
Since the counting is done in __i2c_transfer, where the number of
messages transferred is not known upon error, the error counters are
attributed to all unique addresses that appear in the message list
passed into __i2c_transfer.
Currently an rbtree is used to find the i2c_client located at a certain
address. Would be happy to know if there is a better way of doing this.
Thanks!
Sui Chen (1):
i2c debug counters as sysfs attributes
drivers/i2c/i2c-core-base.c | 240 +++++++++++++++++++++++++++++++++++-
drivers/i2c/i2c-dev.c | 94 ++++++++++++++
include/linux/i2c.h | 41 ++++++
3 files changed, 374 insertions(+), 1 deletion(-)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC Patch v5 1/1] i2c debug counters as sysfs attributes
2022-05-17 10:05 ` Sui Chen
@ 2022-05-17 10:05 ` Sui Chen
-1 siblings, 0 replies; 6+ messages in thread
From: Sui Chen @ 2022-05-17 10:05 UTC (permalink / raw)
To: linux-kernel, linux-i2c, wsa, openbmc, tali.perry1
Cc: joel, andrew, benjaminfair, krellan, Sui Chen
This change renames the I2C debug counters as suggested, and makes them
available to i2c_adapter's and i2c_client's:
- bus_errors
- transfers (only applicable to i2c_adapter)
- messages (only applicable to i2c_client)
- nacks
- timeouts
- recovery_successes (only applicable to i2c_adapter)
- recovery_failures (only applicable to i2c_adapter)
The above stats are located in the sysfs path "stats". The kobj for
"stats" for a i2c-device or an i2c-adapter are added as children of
the device's and adapter's kobj.
Did some quick tests with QEMU using a program that saves/replays I2C
trace by reading hwmon sensors (the program is located at
https://gerrit.openbmc-project.xyz/c/openbmc/openbmc-tools/+/52527)
(normal read)
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks}\
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks} && \
sleep 1 && ./i2c_bmk_bmc 0
46
0
92
0
/sys/class/hwmon/hwmon0/temp1_input: 0
[FindTraceEntries] t0=111.000000 t1=113.000000
Found 4 interesting I2C trace entries:
i2c_write: i2c-1 #0 a=05c f=0000 l=1 [00]
i2c_read: i2c-1 #1 a=05c f=0001 l=2
i2c_reply: i2c-1 #1 a=05c f=0001 l=2 [00-00]
i2c_result: i2c-1 n=2 ret=2
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks} \
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks}
47 <-- 1 more transfer on i2c-1
0
94 <-- 2 more messages sent to 1-005c
0
(deliberately nack)
root@gsj:~# sleep 1 && ./i2c_bmk_bmc 0
(injects 1 NACK using QEMU)
/sys/class/hwmon/hwmon0/temp1_input: 0
[FindTraceEntries] t0=116.000000 t1=118.000000
Found 3 interesting I2C trace entries:
i2c_write: i2c-1 #0 a=05c f=0000 l=1 [00]
i2c_read: i2c-1 #1 a=05c f=0001 l=2
i2c_result: i2c-1 n=2 ret=-6
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks} \
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks}
47
1 <-- 1 more NACK on i2c-1
94
1 <-- 1 more NACK attributed to 1-005c
Signed-off-by: Sui Chen <suichen@google.com>
---
drivers/i2c/i2c-core-base.c | 240 +++++++++++++++++++++++++++++++++++-
drivers/i2c/i2c-dev.c | 94 ++++++++++++++
include/linux/i2c.h | 41 ++++++
3 files changed, 374 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 84f12bf90644..4e7bd849f127 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -270,11 +270,22 @@ EXPORT_SYMBOL_GPL(i2c_generic_scl_recovery);
int i2c_recover_bus(struct i2c_adapter *adap)
{
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+
if (!adap->bus_recovery_info)
return -EBUSY;
dev_dbg(&adap->dev, "Trying i2c bus recovery\n");
- return adap->bus_recovery_info->recover_bus(adap);
+ int ret = adap->bus_recovery_info->recover_bus(adap);
+
+ if (ret == 0)
+ ++(adap->stats->recovery_successes);
+ else
+ ++(adap->stats->recovery_failures);
+
+ return ret;
+
}
EXPORT_SYMBOL_GPL(i2c_recover_bus);
@@ -993,6 +1004,78 @@ int i2c_dev_irq_from_resources(const struct resource *resources,
return 0;
}
+ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->messages);
+}
+
+struct kobj_attribute i2c_client_stats_messages_attr = {
+ .attr = { .name = "messages", .mode = 0444 },
+ .show = i2c_client_stats_messages_show,
+};
+
+ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->bus_errors);
+}
+
+struct kobj_attribute i2c_client_stats_bus_errors_attr = {
+ .attr = { .name = "bus_errors", .mode = 0444 },
+ .show = i2c_client_stats_bus_errors_show,
+};
+
+ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->nacks);
+}
+
+struct kobj_attribute i2c_client_stats_nacks_attr = {
+ .attr = { .name = "nacks", .mode = 0444 },
+ .show = i2c_client_stats_nacks_show,
+};
+
+ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->timeouts);
+}
+
+struct kobj_attribute i2c_client_stats_timeouts_attr = {
+ .attr = { .name = "timeouts", .mode = 0444 },
+ .show = i2c_client_stats_timeouts_show,
+};
+
+static struct attribute *i2c_client_stats_attrs[] = {
+ &i2c_client_stats_messages_attr.attr,
+ &i2c_client_stats_bus_errors_attr.attr,
+ &i2c_client_stats_nacks_attr.attr,
+ &i2c_client_stats_timeouts_attr.attr,
+ NULL
+};
+
+static struct kobj_type i2c_client_stats_ktype = {
+ .sysfs_ops = &kobj_sysfs_ops,
+ .default_attrs = i2c_client_stats_attrs,
+};
+
+/**
+ * Functions for maintaining the adapter -> client stats lookup structure
+ */
+static struct i2c_client_stats *__get_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr);
+static void __insert_i2c_client_stats(struct i2c_adapter_stats *stats,
+ struct i2c_client_stats *client_stats,
+ int i2c_addr);
+static void __remove_i2c_client_stats(struct i2c_adapter_stats *stats, int i2c_addr);
+static int __count_i2c_client_stats(struct i2c_adapter_stats *stats);
+
/**
* i2c_new_client_device - instantiate an i2c device
* @adap: the adapter managing the device
@@ -1014,6 +1097,7 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
{
struct i2c_client *client;
int status;
+ struct i2c_client_stats *client_stats;
client = kzalloc(sizeof *client, GFP_KERNEL);
if (!client)
@@ -1069,6 +1153,20 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n",
client->name, dev_name(&client->dev));
+ client_stats = kzalloc(sizeof(*client_stats), GFP_KERNEL);
+ client->stats = client_stats;
+ client_stats->address = info->addr;
+
+ kobject_init_and_add(&client_stats->kobj, &i2c_client_stats_ktype,
+ &client->dev.kobj, "stats");
+
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+
+ __insert_i2c_client_stats(adap->stats, client_stats, info->addr);
+ dev_info(&adap->dev, "Added i2c_client_stats, adapter has %d client stats now",
+ __count_i2c_client_stats(adap->stats));
+
return client;
out_remove_swnode:
@@ -1103,6 +1201,12 @@ void i2c_unregister_device(struct i2c_client *client)
if (ACPI_COMPANION(&client->dev))
acpi_device_clear_enumerated(ACPI_COMPANION(&client->dev));
device_remove_software_node(&client->dev);
+
+ __remove_i2c_client_stats(client->adapter->stats, client->addr);
+ dev_info(&client->adapter->dev, "Removed i2c_client_stats, adapter has %d client stats now",
+ __count_i2c_client_stats(client->adapter->stats));
+ kfree(client->stats);
+
device_unregister(&client->dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -2176,6 +2280,8 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
unsigned long orig_jiffies;
int ret, try;
+ int i, j, addr;
+ struct i2c_client_stats *cs;
if (WARN_ON(!msgs || num < 1))
return -EINVAL;
@@ -2223,6 +2329,50 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
trace_i2c_result(adap, num, ret);
}
+ // Per-adapter I2C stats
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+ if (ret < 0) {
+ if (ret == -ENXIO)
+ ++(adap->stats->nacks);
+ else if (ret == -ETIMEDOUT)
+ ++(adap->stats->timeouts);
+ else
+ ++(adap->stats->bus_errors);
+ } else if (num > 0) {
+ ++(adap->stats->transfers);
+ }
+
+ // Per-address I2C stats
+ // If no errors, incerement the message count per client
+ if (ret >= 0) {
+ for (i = 0; i < ret; ++i) {
+ addr = msgs[i].addr;
+ cs = __get_i2c_client_stats(adap->stats, addr);
+ if (!cs) // For muxed devices, their stats are at the leaf level
+ continue;
+ if (num > 0)
+ ++(cs->messages);
+ }
+ } else { // Otherwise, attribute the error to each of the distinct addresses
+ for (i = 0; i < num; ++i) {
+ for (j = 0; j < i; ++j) { // de-duplicate
+ if (msgs[i].addr == msgs[j].addr)
+ goto done;
+ }
+ cs = __get_i2c_client_stats(adap->stats, msgs[i].addr);
+ if (!cs)
+ continue;
+ if (ret == -ENXIO)
+ ++(cs->nacks);
+ else if (ret == -ETIMEDOUT)
+ ++(cs->timeouts);
+ else
+ ++(cs->bus_errors);
+done:
+ {}
+ }
+ }
return ret;
}
EXPORT_SYMBOL(__i2c_transfer);
@@ -2619,6 +2769,94 @@ void i2c_put_dma_safe_msg_buf(u8 *buf, struct i2c_msg *msg, bool xferred)
}
EXPORT_SYMBOL_GPL(i2c_put_dma_safe_msg_buf);
+static struct i2c_client_stats *__get_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr)
+{
+ int addr;
+ struct rb_node *n, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ n = stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (n) {
+ parent = n;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr)
+ n = n->rb_left;
+ else if (addr < i2c_addr)
+ n = n->rb_right;
+ else
+ return cs_node->stats;
+ }
+ return NULL;
+}
+
+static void __insert_i2c_client_stats(struct i2c_adapter_stats *stats,
+ struct i2c_client_stats *client_stats, int i2c_addr)
+{
+ char buf[32];
+ int ret, addr;
+ struct rb_node **link, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ ret = 1;
+ link = &stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (*link) {
+ parent = *link;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr)
+ link = &(*link)->rb_left;
+ else if (addr < i2c_addr)
+ link = &(*link)->rb_right;
+ else
+ return; // Already exists
+ }
+
+ cs_node = kzalloc(sizeof(*cs_node), GFP_KERNEL);
+
+ cs_node->stats = client_stats;
+ rb_link_node(&cs_node->rb_node, parent, link);
+ rb_insert_color(&cs_node->rb_node, &stats->i2c_client_stats_root);
+}
+
+static void __remove_i2c_client_stats(struct i2c_adapter_stats *stats, int i2c_addr)
+{
+ int addr;
+ struct rb_node *n, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ n = stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (n) {
+ parent = n;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr) {
+ n = n->rb_left;
+ } else if (addr < i2c_addr) {
+ n = n->rb_right;
+ } else {
+ rb_erase(&cs_node->rb_node, &stats->i2c_client_stats_root);
+ return;
+ }
+ }
+}
+
+static int __count_i2c_client_stats(struct i2c_adapter_stats *stats)
+{
+ struct rb_node *n = stats->i2c_client_stats_root.rb_node;
+ int ret = 0;
+
+ while (n) {
+ ++ret;
+ n = rb_next(n);
+ }
+ return ret;
+}
+
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
MODULE_DESCRIPTION("I2C-Bus main module");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 77f576e51652..50141dd42796 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -767,6 +767,100 @@ static void __exit i2c_dev_exit(void)
unregister_chrdev_region(MKDEV(I2C_MAJOR, 0), I2C_MINORS);
}
+static struct i2c_adapter *kobj_to_adapter(struct device *pdev)
+{
+ struct kobject *dev_kobj;
+ struct device *dev;
+
+ dev_kobj = ((struct kobject *)pdev)->parent;
+ dev = container_of(dev_kobj, struct device, kobj);
+ return to_i2c_adapter(dev);
+}
+
+ssize_t bus_errors_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->bus_errors);
+}
+DEVICE_ATTR_RO(bus_errors);
+
+ssize_t transfers_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->transfers);
+}
+DEVICE_ATTR_RO(transfers);
+
+ssize_t nacks_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->nacks);
+}
+DEVICE_ATTR_RO(nacks);
+
+ssize_t recovery_successes_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->recovery_successes);
+}
+DEVICE_ATTR_RO(recovery_successes);
+
+ssize_t recovery_failures_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->recovery_failures);
+}
+DEVICE_ATTR_RO(recovery_failures);
+
+ssize_t timeouts_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->timeouts);
+}
+DEVICE_ATTR_RO(timeouts);
+
+/**
+ * i2c_adapter_create_stats_directory - creates folder for I2C statistics.
+ * @adapter: the i2c_adapter to create the stats directory for.
+ *
+ * Return: 0 if successful, 1 if failed.
+ */
+int i2c_adapter_create_stats_directory(struct i2c_adapter *adapter)
+{
+ struct i2c_adapter_stats *stats;
+ int ret = 1;
+
+ stats = kzalloc(sizeof(*stats), GFP_KERNEL);
+ if (!stats) {
+ adapter->stats = NULL;
+ return ret;
+ }
+ adapter->stats = stats;
+ adapter->stats->kobj = kobject_create_and_add("stats", &adapter->dev.kobj);
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_transfers.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for tx_complete_cnt\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_bus_errors.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for bus_errors\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_nacks.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for nacks\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_recovery_successes.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for recovery_successes\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_recovery_failures.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for recovery_failures\n");
+
+ return ret;
+}
+
MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
MODULE_DESCRIPTION("I2C /dev entries driver");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 3eb60a2e9e61..4547c4c782b6 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -20,6 +20,7 @@
#include <linux/irqdomain.h> /* for Host Notify IRQ */
#include <linux/of.h> /* for struct device_node */
#include <linux/swab.h> /* for swab16 */
+#include <linux/rbtree.h>
#include <uapi/linux/i2c.h>
extern struct bus_type i2c_bus_type;
@@ -297,6 +298,20 @@ struct i2c_driver {
};
#define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
+int i2c_adapter_create_stats_directory(struct i2c_adapter *adapter);
+
+void i2c_adapter_stats_register_counter(struct i2c_adapter *adapter,
+ const char *counter_name, void *data_source);
+
+struct i2c_client_stats {
+ struct kobject kobj;
+ int address;
+ u64 messages;
+ u64 bus_errors;
+ u64 nacks;
+ u64 timeouts;
+};
+
/**
* struct i2c_client - represent an I2C slave device
* @flags: see I2C_CLIENT_* for possible flags
@@ -342,6 +357,7 @@ struct i2c_client {
i2c_slave_cb_t slave_cb; /* callback for slave mode */
#endif
void *devres_group_id; /* ID of probe devres group */
+ struct i2c_client_stats *stats; /* i2c client stats */
};
#define to_i2c_client(d) container_of(d, struct i2c_client, dev)
@@ -684,6 +700,30 @@ struct i2c_adapter_quirks {
u16 max_comb_2nd_msg_len;
};
+/**
+ * I2C adapter statistics
+ */
+struct i2c_adapter_stats {
+ struct kobject *kobj;
+
+ u64 transfers;
+ u64 bus_errors;
+ u64 nacks;
+ u64 recovery_successes;
+ u64 recovery_failures;
+ u64 timeouts;
+
+ struct rb_root i2c_client_stats_root;
+};
+
+struct i2c_client_stats_rbnode {
+ struct i2c_client_stats *stats;
+ struct rb_node rb_node;
+};
+
+struct i2c_client_stats *__get_or_create_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr);
+
/* enforce max_num_msgs = 2 and use max_comb_*_len for length checks */
#define I2C_AQ_COMB BIT(0)
/* first combined message must be write */
@@ -738,6 +778,7 @@ struct i2c_adapter {
struct irq_domain *host_notify_domain;
struct regulator *bus_regulator;
+ struct i2c_adapter_stats *stats;
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC Patch v5 1/1] i2c debug counters as sysfs attributes
@ 2022-05-17 10:05 ` Sui Chen
0 siblings, 0 replies; 6+ messages in thread
From: Sui Chen @ 2022-05-17 10:05 UTC (permalink / raw)
To: linux-kernel, linux-i2c, wsa, openbmc, tali.perry1
Cc: andrew, krellan, Sui Chen, joel, benjaminfair
This change renames the I2C debug counters as suggested, and makes them
available to i2c_adapter's and i2c_client's:
- bus_errors
- transfers (only applicable to i2c_adapter)
- messages (only applicable to i2c_client)
- nacks
- timeouts
- recovery_successes (only applicable to i2c_adapter)
- recovery_failures (only applicable to i2c_adapter)
The above stats are located in the sysfs path "stats". The kobj for
"stats" for a i2c-device or an i2c-adapter are added as children of
the device's and adapter's kobj.
Did some quick tests with QEMU using a program that saves/replays I2C
trace by reading hwmon sensors (the program is located at
https://gerrit.openbmc-project.xyz/c/openbmc/openbmc-tools/+/52527)
(normal read)
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks}\
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks} && \
sleep 1 && ./i2c_bmk_bmc 0
46
0
92
0
/sys/class/hwmon/hwmon0/temp1_input: 0
[FindTraceEntries] t0=111.000000 t1=113.000000
Found 4 interesting I2C trace entries:
i2c_write: i2c-1 #0 a=05c f=0000 l=1 [00]
i2c_read: i2c-1 #1 a=05c f=0001 l=2
i2c_reply: i2c-1 #1 a=05c f=0001 l=2 [00-00]
i2c_result: i2c-1 n=2 ret=2
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks} \
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks}
47 <-- 1 more transfer on i2c-1
0
94 <-- 2 more messages sent to 1-005c
0
(deliberately nack)
root@gsj:~# sleep 1 && ./i2c_bmk_bmc 0
(injects 1 NACK using QEMU)
/sys/class/hwmon/hwmon0/temp1_input: 0
[FindTraceEntries] t0=116.000000 t1=118.000000
Found 3 interesting I2C trace entries:
i2c_write: i2c-1 #0 a=05c f=0000 l=1 [00]
i2c_read: i2c-1 #1 a=05c f=0001 l=2
i2c_result: i2c-1 n=2 ret=-6
root@gsj:~# cat /sys/class/i2c-adapter/i2c-1/stats/{transfers,nacks} \
/sys/class/i2c-adapter/i2c-1/1-005c/stats/{messages,nacks}
47
1 <-- 1 more NACK on i2c-1
94
1 <-- 1 more NACK attributed to 1-005c
Signed-off-by: Sui Chen <suichen@google.com>
---
drivers/i2c/i2c-core-base.c | 240 +++++++++++++++++++++++++++++++++++-
drivers/i2c/i2c-dev.c | 94 ++++++++++++++
include/linux/i2c.h | 41 ++++++
3 files changed, 374 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 84f12bf90644..4e7bd849f127 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -270,11 +270,22 @@ EXPORT_SYMBOL_GPL(i2c_generic_scl_recovery);
int i2c_recover_bus(struct i2c_adapter *adap)
{
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+
if (!adap->bus_recovery_info)
return -EBUSY;
dev_dbg(&adap->dev, "Trying i2c bus recovery\n");
- return adap->bus_recovery_info->recover_bus(adap);
+ int ret = adap->bus_recovery_info->recover_bus(adap);
+
+ if (ret == 0)
+ ++(adap->stats->recovery_successes);
+ else
+ ++(adap->stats->recovery_failures);
+
+ return ret;
+
}
EXPORT_SYMBOL_GPL(i2c_recover_bus);
@@ -993,6 +1004,78 @@ int i2c_dev_irq_from_resources(const struct resource *resources,
return 0;
}
+ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->messages);
+}
+
+struct kobj_attribute i2c_client_stats_messages_attr = {
+ .attr = { .name = "messages", .mode = 0444 },
+ .show = i2c_client_stats_messages_show,
+};
+
+ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->bus_errors);
+}
+
+struct kobj_attribute i2c_client_stats_bus_errors_attr = {
+ .attr = { .name = "bus_errors", .mode = 0444 },
+ .show = i2c_client_stats_bus_errors_show,
+};
+
+ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->nacks);
+}
+
+struct kobj_attribute i2c_client_stats_nacks_attr = {
+ .attr = { .name = "nacks", .mode = 0444 },
+ .show = i2c_client_stats_nacks_show,
+};
+
+ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%llu\n",
+ container_of(kobj, struct i2c_client_stats, kobj)->timeouts);
+}
+
+struct kobj_attribute i2c_client_stats_timeouts_attr = {
+ .attr = { .name = "timeouts", .mode = 0444 },
+ .show = i2c_client_stats_timeouts_show,
+};
+
+static struct attribute *i2c_client_stats_attrs[] = {
+ &i2c_client_stats_messages_attr.attr,
+ &i2c_client_stats_bus_errors_attr.attr,
+ &i2c_client_stats_nacks_attr.attr,
+ &i2c_client_stats_timeouts_attr.attr,
+ NULL
+};
+
+static struct kobj_type i2c_client_stats_ktype = {
+ .sysfs_ops = &kobj_sysfs_ops,
+ .default_attrs = i2c_client_stats_attrs,
+};
+
+/**
+ * Functions for maintaining the adapter -> client stats lookup structure
+ */
+static struct i2c_client_stats *__get_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr);
+static void __insert_i2c_client_stats(struct i2c_adapter_stats *stats,
+ struct i2c_client_stats *client_stats,
+ int i2c_addr);
+static void __remove_i2c_client_stats(struct i2c_adapter_stats *stats, int i2c_addr);
+static int __count_i2c_client_stats(struct i2c_adapter_stats *stats);
+
/**
* i2c_new_client_device - instantiate an i2c device
* @adap: the adapter managing the device
@@ -1014,6 +1097,7 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
{
struct i2c_client *client;
int status;
+ struct i2c_client_stats *client_stats;
client = kzalloc(sizeof *client, GFP_KERNEL);
if (!client)
@@ -1069,6 +1153,20 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n",
client->name, dev_name(&client->dev));
+ client_stats = kzalloc(sizeof(*client_stats), GFP_KERNEL);
+ client->stats = client_stats;
+ client_stats->address = info->addr;
+
+ kobject_init_and_add(&client_stats->kobj, &i2c_client_stats_ktype,
+ &client->dev.kobj, "stats");
+
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+
+ __insert_i2c_client_stats(adap->stats, client_stats, info->addr);
+ dev_info(&adap->dev, "Added i2c_client_stats, adapter has %d client stats now",
+ __count_i2c_client_stats(adap->stats));
+
return client;
out_remove_swnode:
@@ -1103,6 +1201,12 @@ void i2c_unregister_device(struct i2c_client *client)
if (ACPI_COMPANION(&client->dev))
acpi_device_clear_enumerated(ACPI_COMPANION(&client->dev));
device_remove_software_node(&client->dev);
+
+ __remove_i2c_client_stats(client->adapter->stats, client->addr);
+ dev_info(&client->adapter->dev, "Removed i2c_client_stats, adapter has %d client stats now",
+ __count_i2c_client_stats(client->adapter->stats));
+ kfree(client->stats);
+
device_unregister(&client->dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -2176,6 +2280,8 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
unsigned long orig_jiffies;
int ret, try;
+ int i, j, addr;
+ struct i2c_client_stats *cs;
if (WARN_ON(!msgs || num < 1))
return -EINVAL;
@@ -2223,6 +2329,50 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
trace_i2c_result(adap, num, ret);
}
+ // Per-adapter I2C stats
+ if (!adap->stats)
+ i2c_adapter_create_stats_directory(adap);
+ if (ret < 0) {
+ if (ret == -ENXIO)
+ ++(adap->stats->nacks);
+ else if (ret == -ETIMEDOUT)
+ ++(adap->stats->timeouts);
+ else
+ ++(adap->stats->bus_errors);
+ } else if (num > 0) {
+ ++(adap->stats->transfers);
+ }
+
+ // Per-address I2C stats
+ // If no errors, incerement the message count per client
+ if (ret >= 0) {
+ for (i = 0; i < ret; ++i) {
+ addr = msgs[i].addr;
+ cs = __get_i2c_client_stats(adap->stats, addr);
+ if (!cs) // For muxed devices, their stats are at the leaf level
+ continue;
+ if (num > 0)
+ ++(cs->messages);
+ }
+ } else { // Otherwise, attribute the error to each of the distinct addresses
+ for (i = 0; i < num; ++i) {
+ for (j = 0; j < i; ++j) { // de-duplicate
+ if (msgs[i].addr == msgs[j].addr)
+ goto done;
+ }
+ cs = __get_i2c_client_stats(adap->stats, msgs[i].addr);
+ if (!cs)
+ continue;
+ if (ret == -ENXIO)
+ ++(cs->nacks);
+ else if (ret == -ETIMEDOUT)
+ ++(cs->timeouts);
+ else
+ ++(cs->bus_errors);
+done:
+ {}
+ }
+ }
return ret;
}
EXPORT_SYMBOL(__i2c_transfer);
@@ -2619,6 +2769,94 @@ void i2c_put_dma_safe_msg_buf(u8 *buf, struct i2c_msg *msg, bool xferred)
}
EXPORT_SYMBOL_GPL(i2c_put_dma_safe_msg_buf);
+static struct i2c_client_stats *__get_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr)
+{
+ int addr;
+ struct rb_node *n, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ n = stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (n) {
+ parent = n;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr)
+ n = n->rb_left;
+ else if (addr < i2c_addr)
+ n = n->rb_right;
+ else
+ return cs_node->stats;
+ }
+ return NULL;
+}
+
+static void __insert_i2c_client_stats(struct i2c_adapter_stats *stats,
+ struct i2c_client_stats *client_stats, int i2c_addr)
+{
+ char buf[32];
+ int ret, addr;
+ struct rb_node **link, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ ret = 1;
+ link = &stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (*link) {
+ parent = *link;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr)
+ link = &(*link)->rb_left;
+ else if (addr < i2c_addr)
+ link = &(*link)->rb_right;
+ else
+ return; // Already exists
+ }
+
+ cs_node = kzalloc(sizeof(*cs_node), GFP_KERNEL);
+
+ cs_node->stats = client_stats;
+ rb_link_node(&cs_node->rb_node, parent, link);
+ rb_insert_color(&cs_node->rb_node, &stats->i2c_client_stats_root);
+}
+
+static void __remove_i2c_client_stats(struct i2c_adapter_stats *stats, int i2c_addr)
+{
+ int addr;
+ struct rb_node *n, *parent;
+ struct i2c_client_stats_rbnode *cs_node;
+
+ n = stats->i2c_client_stats_root.rb_node;
+ parent = NULL;
+ while (n) {
+ parent = n;
+ cs_node = rb_entry(parent, struct i2c_client_stats_rbnode, rb_node);
+ addr = cs_node->stats->address;
+ if (addr > i2c_addr) {
+ n = n->rb_left;
+ } else if (addr < i2c_addr) {
+ n = n->rb_right;
+ } else {
+ rb_erase(&cs_node->rb_node, &stats->i2c_client_stats_root);
+ return;
+ }
+ }
+}
+
+static int __count_i2c_client_stats(struct i2c_adapter_stats *stats)
+{
+ struct rb_node *n = stats->i2c_client_stats_root.rb_node;
+ int ret = 0;
+
+ while (n) {
+ ++ret;
+ n = rb_next(n);
+ }
+ return ret;
+}
+
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
MODULE_DESCRIPTION("I2C-Bus main module");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 77f576e51652..50141dd42796 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -767,6 +767,100 @@ static void __exit i2c_dev_exit(void)
unregister_chrdev_region(MKDEV(I2C_MAJOR, 0), I2C_MINORS);
}
+static struct i2c_adapter *kobj_to_adapter(struct device *pdev)
+{
+ struct kobject *dev_kobj;
+ struct device *dev;
+
+ dev_kobj = ((struct kobject *)pdev)->parent;
+ dev = container_of(dev_kobj, struct device, kobj);
+ return to_i2c_adapter(dev);
+}
+
+ssize_t bus_errors_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->bus_errors);
+}
+DEVICE_ATTR_RO(bus_errors);
+
+ssize_t transfers_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->transfers);
+}
+DEVICE_ATTR_RO(transfers);
+
+ssize_t nacks_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->nacks);
+}
+DEVICE_ATTR_RO(nacks);
+
+ssize_t recovery_successes_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->recovery_successes);
+}
+DEVICE_ATTR_RO(recovery_successes);
+
+ssize_t recovery_failures_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->recovery_failures);
+}
+DEVICE_ATTR_RO(recovery_failures);
+
+ssize_t timeouts_show(struct device *pdev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%llu\n", kobj_to_adapter(pdev)->stats->timeouts);
+}
+DEVICE_ATTR_RO(timeouts);
+
+/**
+ * i2c_adapter_create_stats_directory - creates folder for I2C statistics.
+ * @adapter: the i2c_adapter to create the stats directory for.
+ *
+ * Return: 0 if successful, 1 if failed.
+ */
+int i2c_adapter_create_stats_directory(struct i2c_adapter *adapter)
+{
+ struct i2c_adapter_stats *stats;
+ int ret = 1;
+
+ stats = kzalloc(sizeof(*stats), GFP_KERNEL);
+ if (!stats) {
+ adapter->stats = NULL;
+ return ret;
+ }
+ adapter->stats = stats;
+ adapter->stats->kobj = kobject_create_and_add("stats", &adapter->dev.kobj);
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_transfers.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for tx_complete_cnt\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_bus_errors.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for bus_errors\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_nacks.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for nacks\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_recovery_successes.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for recovery_successes\n");
+
+ ret = sysfs_create_file(adapter->stats->kobj, &dev_attr_recovery_failures.attr);
+ if (ret)
+ dev_warn(&adapter->dev, "Failed to create sysfs file for recovery_failures\n");
+
+ return ret;
+}
+
MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
MODULE_DESCRIPTION("I2C /dev entries driver");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 3eb60a2e9e61..4547c4c782b6 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -20,6 +20,7 @@
#include <linux/irqdomain.h> /* for Host Notify IRQ */
#include <linux/of.h> /* for struct device_node */
#include <linux/swab.h> /* for swab16 */
+#include <linux/rbtree.h>
#include <uapi/linux/i2c.h>
extern struct bus_type i2c_bus_type;
@@ -297,6 +298,20 @@ struct i2c_driver {
};
#define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
+int i2c_adapter_create_stats_directory(struct i2c_adapter *adapter);
+
+void i2c_adapter_stats_register_counter(struct i2c_adapter *adapter,
+ const char *counter_name, void *data_source);
+
+struct i2c_client_stats {
+ struct kobject kobj;
+ int address;
+ u64 messages;
+ u64 bus_errors;
+ u64 nacks;
+ u64 timeouts;
+};
+
/**
* struct i2c_client - represent an I2C slave device
* @flags: see I2C_CLIENT_* for possible flags
@@ -342,6 +357,7 @@ struct i2c_client {
i2c_slave_cb_t slave_cb; /* callback for slave mode */
#endif
void *devres_group_id; /* ID of probe devres group */
+ struct i2c_client_stats *stats; /* i2c client stats */
};
#define to_i2c_client(d) container_of(d, struct i2c_client, dev)
@@ -684,6 +700,30 @@ struct i2c_adapter_quirks {
u16 max_comb_2nd_msg_len;
};
+/**
+ * I2C adapter statistics
+ */
+struct i2c_adapter_stats {
+ struct kobject *kobj;
+
+ u64 transfers;
+ u64 bus_errors;
+ u64 nacks;
+ u64 recovery_successes;
+ u64 recovery_failures;
+ u64 timeouts;
+
+ struct rb_root i2c_client_stats_root;
+};
+
+struct i2c_client_stats_rbnode {
+ struct i2c_client_stats *stats;
+ struct rb_node rb_node;
+};
+
+struct i2c_client_stats *__get_or_create_i2c_client_stats(struct i2c_adapter_stats *stats,
+ int i2c_addr);
+
/* enforce max_num_msgs = 2 and use max_comb_*_len for length checks */
#define I2C_AQ_COMB BIT(0)
/* first combined message must be write */
@@ -738,6 +778,7 @@ struct i2c_adapter {
struct irq_domain *host_notify_domain;
struct regulator *bus_regulator;
+ struct i2c_adapter_stats *stats;
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC Patch v5 1/1] i2c debug counters as sysfs attributes
2022-05-17 10:05 ` Sui Chen
(?)
@ 2022-05-17 22:09 ` kernel test robot
-1 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-05-17 22:09 UTC (permalink / raw)
To: Sui Chen; +Cc: llvm, kbuild-all
Hi Sui,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on linux/master linus/master v5.18-rc7 next-20220517]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Sui-Chen/i2c-core-Adapter-and-client-stats-as-sysfs-attributes/20220517-180857
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-randconfig-a003-20220516 (https://download.01.org/0day-ci/archive/20220518/202205180518.gLEg6p2y-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2d26f1bca9ffcd15ddb7ab0bbb596cacd89712ca
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sui-Chen/i2c-core-Adapter-and-client-stats-as-sysfs-attributes/20220517-180857
git checkout 2d26f1bca9ffcd15ddb7ab0bbb596cacd89712ca
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/i2c/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/i2c/i2c-core-base.c:280:6: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
int ret = adap->bus_recovery_info->recover_bus(adap);
^
>> drivers/i2c/i2c-core-base.c:910:9: warning: no previous prototype for function 'i2c_client_stats_messages_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:910:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:922:9: warning: no previous prototype for function 'i2c_client_stats_bus_errors_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:922:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:934:9: warning: no previous prototype for function 'i2c_client_stats_nacks_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:934:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:946:9: warning: no previous prototype for function 'i2c_client_stats_timeouts_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:946:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
drivers/i2c/i2c-core-base.c:968:3: error: field designator 'default_attrs' does not refer to any field in type 'struct kobj_type'
.default_attrs = i2c_client_stats_attrs,
^
>> drivers/i2c/i2c-core-base.c:1064:2: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
kobject_init_and_add(&client_stats->kobj, &i2c_client_stats_ktype,
^~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c/i2c-core-base.c:2704:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
int ret, addr;
^
>> drivers/i2c/i2c-core-base.c:2703:7: warning: unused variable 'buf' [-Wunused-variable]
char buf[32];
^
8 warnings and 1 error generated.
--
>> drivers/i2c/i2c-dev.c:786:9: warning: no previous prototype for function 'bus_errors_show' [-Wmissing-prototypes]
ssize_t bus_errors_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:786:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t bus_errors_show(struct device *pdev,
^
static
>> drivers/i2c/i2c-dev.c:793:9: warning: no previous prototype for function 'transfers_show' [-Wmissing-prototypes]
ssize_t transfers_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:793:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t transfers_show(struct device *pdev,
^
static
>> drivers/i2c/i2c-dev.c:800:9: warning: no previous prototype for function 'nacks_show' [-Wmissing-prototypes]
ssize_t nacks_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:800:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t nacks_show(struct device *pdev,
^
static
>> drivers/i2c/i2c-dev.c:807:9: warning: no previous prototype for function 'recovery_successes_show' [-Wmissing-prototypes]
ssize_t recovery_successes_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:807:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t recovery_successes_show(struct device *pdev,
^
static
>> drivers/i2c/i2c-dev.c:814:9: warning: no previous prototype for function 'recovery_failures_show' [-Wmissing-prototypes]
ssize_t recovery_failures_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:814:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t recovery_failures_show(struct device *pdev,
^
static
>> drivers/i2c/i2c-dev.c:821:9: warning: no previous prototype for function 'timeouts_show' [-Wmissing-prototypes]
ssize_t timeouts_show(struct device *pdev,
^
drivers/i2c/i2c-dev.c:821:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t timeouts_show(struct device *pdev,
^
static
6 warnings generated.
vim +280 drivers/i2c/i2c-core-base.c
270
271 int i2c_recover_bus(struct i2c_adapter *adap)
272 {
273 if (!adap->stats)
274 i2c_adapter_create_stats_directory(adap);
275
276 if (!adap->bus_recovery_info)
277 return -EBUSY;
278
279 dev_dbg(&adap->dev, "Trying i2c bus recovery\n");
> 280 int ret = adap->bus_recovery_info->recover_bus(adap);
281
282 if (ret == 0)
283 ++(adap->stats->recovery_successes);
284 else
285 ++(adap->stats->recovery_failures);
286
287 return ret;
288
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC Patch v5 1/1] i2c debug counters as sysfs attributes
2022-05-17 10:05 ` Sui Chen
(?)
(?)
@ 2022-05-17 22:09 ` kernel test robot
-1 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-05-17 22:09 UTC (permalink / raw)
To: Sui Chen; +Cc: llvm, kbuild-all
Hi Sui,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linux/master linus/master v5.18-rc7 next-20220517]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Sui-Chen/i2c-core-Adapter-and-client-stats-as-sysfs-attributes/20220517-180857
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: hexagon-randconfig-r041-20220516 (https://download.01.org/0day-ci/archive/20220518/202205180640.LEuR1qW3-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2d26f1bca9ffcd15ddb7ab0bbb596cacd89712ca
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sui-Chen/i2c-core-Adapter-and-client-stats-as-sysfs-attributes/20220517-180857
git checkout 2d26f1bca9ffcd15ddb7ab0bbb596cacd89712ca
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/i2c/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/i2c/i2c-core-base.c:280:6: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
int ret = adap->bus_recovery_info->recover_bus(adap);
^
>> drivers/i2c/i2c-core-base.c:910:9: warning: no previous prototype for function 'i2c_client_stats_messages_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:910:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:922:9: warning: no previous prototype for function 'i2c_client_stats_bus_errors_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:922:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:934:9: warning: no previous prototype for function 'i2c_client_stats_nacks_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:934:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:946:9: warning: no previous prototype for function 'i2c_client_stats_timeouts_show' [-Wmissing-prototypes]
ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
^
drivers/i2c/i2c-core-base.c:946:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
^
static
>> drivers/i2c/i2c-core-base.c:968:3: error: field designator 'default_attrs' does not refer to any field in type 'struct kobj_type'
.default_attrs = i2c_client_stats_attrs,
^
>> drivers/i2c/i2c-core-base.c:1064:2: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
kobject_init_and_add(&client_stats->kobj, &i2c_client_stats_ktype,
^~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/i2c-core-base.c:2704:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
int ret, addr;
^
drivers/i2c/i2c-core-base.c:2703:7: warning: unused variable 'buf' [-Wunused-variable]
char buf[32];
^
8 warnings and 1 error generated.
vim +968 drivers/i2c/i2c-core-base.c
909
> 910 ssize_t i2c_client_stats_messages_show(struct kobject *kobj, struct kobj_attribute *addr,
911 char *buf)
912 {
913 return sysfs_emit(buf, "%llu\n",
914 container_of(kobj, struct i2c_client_stats, kobj)->messages);
915 }
916
917 struct kobj_attribute i2c_client_stats_messages_attr = {
918 .attr = { .name = "messages", .mode = 0444 },
919 .show = i2c_client_stats_messages_show,
920 };
921
> 922 ssize_t i2c_client_stats_bus_errors_show(struct kobject *kobj, struct kobj_attribute *addr,
923 char *buf)
924 {
925 return sysfs_emit(buf, "%llu\n",
926 container_of(kobj, struct i2c_client_stats, kobj)->bus_errors);
927 }
928
929 struct kobj_attribute i2c_client_stats_bus_errors_attr = {
930 .attr = { .name = "bus_errors", .mode = 0444 },
931 .show = i2c_client_stats_bus_errors_show,
932 };
933
> 934 ssize_t i2c_client_stats_nacks_show(struct kobject *kobj, struct kobj_attribute *addr,
935 char *buf)
936 {
937 return sysfs_emit(buf, "%llu\n",
938 container_of(kobj, struct i2c_client_stats, kobj)->nacks);
939 }
940
941 struct kobj_attribute i2c_client_stats_nacks_attr = {
942 .attr = { .name = "nacks", .mode = 0444 },
943 .show = i2c_client_stats_nacks_show,
944 };
945
> 946 ssize_t i2c_client_stats_timeouts_show(struct kobject *kobj, struct kobj_attribute *addr,
947 char *buf)
948 {
949 return sysfs_emit(buf, "%llu\n",
950 container_of(kobj, struct i2c_client_stats, kobj)->timeouts);
951 }
952
953 struct kobj_attribute i2c_client_stats_timeouts_attr = {
954 .attr = { .name = "timeouts", .mode = 0444 },
955 .show = i2c_client_stats_timeouts_show,
956 };
957
958 static struct attribute *i2c_client_stats_attrs[] = {
959 &i2c_client_stats_messages_attr.attr,
960 &i2c_client_stats_bus_errors_attr.attr,
961 &i2c_client_stats_nacks_attr.attr,
962 &i2c_client_stats_timeouts_attr.attr,
963 NULL
964 };
965
966 static struct kobj_type i2c_client_stats_ktype = {
967 .sysfs_ops = &kobj_sysfs_ops,
> 968 .default_attrs = i2c_client_stats_attrs,
969 };
970
971 /**
972 * Functions for maintaining the adapter -> client stats lookup structure
973 */
974 static struct i2c_client_stats *__get_i2c_client_stats(struct i2c_adapter_stats *stats,
975 int i2c_addr);
976 static void __insert_i2c_client_stats(struct i2c_adapter_stats *stats,
977 struct i2c_client_stats *client_stats,
978 int i2c_addr);
979 static void __remove_i2c_client_stats(struct i2c_adapter_stats *stats, int i2c_addr);
980 static int __count_i2c_client_stats(struct i2c_adapter_stats *stats);
981
982 /**
983 * i2c_new_client_device - instantiate an i2c device
984 * @adap: the adapter managing the device
985 * @info: describes one I2C device; bus_num is ignored
986 * Context: can sleep
987 *
988 * Create an i2c device. Binding is handled through driver model
989 * probe()/remove() methods. A driver may be bound to this device when we
990 * return from this function, or any later moment (e.g. maybe hotplugging will
991 * load the driver module). This call is not appropriate for use by mainboard
992 * initialization logic, which usually runs during an arch_initcall() long
993 * before any i2c_adapter could exist.
994 *
995 * This returns the new i2c client, which may be saved for later use with
996 * i2c_unregister_device(); or an ERR_PTR to describe the error.
997 */
998 struct i2c_client *
999 i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
1000 {
1001 struct i2c_client *client;
1002 int status;
1003 struct i2c_client_stats *client_stats;
1004
1005 client = kzalloc(sizeof *client, GFP_KERNEL);
1006 if (!client)
1007 return ERR_PTR(-ENOMEM);
1008
1009 client->adapter = adap;
1010
1011 client->dev.platform_data = info->platform_data;
1012 client->flags = info->flags;
1013 client->addr = info->addr;
1014
1015 client->init_irq = info->irq;
1016 if (!client->init_irq)
1017 client->init_irq = i2c_dev_irq_from_resources(info->resources,
1018 info->num_resources);
1019
1020 strlcpy(client->name, info->type, sizeof(client->name));
1021
1022 status = i2c_check_addr_validity(client->addr, client->flags);
1023 if (status) {
1024 dev_err(&adap->dev, "Invalid %d-bit I2C address 0x%02hx\n",
1025 client->flags & I2C_CLIENT_TEN ? 10 : 7, client->addr);
1026 goto out_err_silent;
1027 }
1028
1029 /* Check for address business */
1030 status = i2c_check_addr_busy(adap, i2c_encode_flags_to_addr(client));
1031 if (status)
1032 goto out_err;
1033
1034 client->dev.parent = &client->adapter->dev;
1035 client->dev.bus = &i2c_bus_type;
1036 client->dev.type = &i2c_client_type;
1037 client->dev.of_node = of_node_get(info->of_node);
1038 client->dev.fwnode = info->fwnode;
1039
1040 device_enable_async_suspend(&client->dev);
1041 i2c_dev_set_name(adap, client, info);
1042
1043 if (info->swnode) {
1044 status = device_add_software_node(&client->dev, info->swnode);
1045 if (status) {
1046 dev_err(&adap->dev,
1047 "Failed to add software node to client %s: %d\n",
1048 client->name, status);
1049 goto out_err_put_of_node;
1050 }
1051 }
1052
1053 status = device_register(&client->dev);
1054 if (status)
1055 goto out_remove_swnode;
1056
1057 dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n",
1058 client->name, dev_name(&client->dev));
1059
1060 client_stats = kzalloc(sizeof(*client_stats), GFP_KERNEL);
1061 client->stats = client_stats;
1062 client_stats->address = info->addr;
1063
> 1064 kobject_init_and_add(&client_stats->kobj, &i2c_client_stats_ktype,
1065 &client->dev.kobj, "stats");
1066
1067 if (!adap->stats)
1068 i2c_adapter_create_stats_directory(adap);
1069
1070 __insert_i2c_client_stats(adap->stats, client_stats, info->addr);
1071 dev_info(&adap->dev, "Added i2c_client_stats, adapter has %d client stats now",
1072 __count_i2c_client_stats(adap->stats));
1073
1074 return client;
1075
1076 out_remove_swnode:
1077 device_remove_software_node(&client->dev);
1078 out_err_put_of_node:
1079 of_node_put(info->of_node);
1080 out_err:
1081 dev_err(&adap->dev,
1082 "Failed to register i2c client %s at 0x%02x (%d)\n",
1083 client->name, client->addr, status);
1084 out_err_silent:
1085 kfree(client);
1086 return ERR_PTR(status);
1087 }
1088 EXPORT_SYMBOL_GPL(i2c_new_client_device);
1089
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-17 22:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-17 10:05 [RFC Patch v5 0/1] i2c: core: Adapter and client stats as sysfs attributes Sui Chen
2022-05-17 10:05 ` Sui Chen
2022-05-17 10:05 ` [RFC Patch v5 1/1] i2c debug counters " Sui Chen
2022-05-17 10:05 ` Sui Chen
2022-05-17 22:09 ` kernel test robot
2022-05-17 22:09 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.