* [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.