[RFC Patch 0/2] I2C statistics as sysfs attributes

[RFC Patch 0/2] I2C statistics as sysfs attributes

[Sui Chen]

It appears bus errors and NACK counts are frequently found in many
I2C controllers; a few I2C bus drivers now export those counters to
debugfs. There is some effort in attempting to monitor a large
number of BMCs (board management controllers) and we are wondering if
it is possible to have a more stable and unified interface for those
counters to be exported to compared to the debugfs, so that a single
command can be used to obtain the counters from different types of I2C
controllers.

One possible prototype is included in the patchset where debug
counters are populated in /sys/class/i2c-adapter/i2c-X/stats/.

By default only an empty stats folder is created, and it is up to the
platform drivers to instantiate the counters and link the counters to
the data structures internal to the bus drivers.

From the kernel data structure point of view, the statistics are
a property of an i2c_adapter (and not an i2c_dev). From the driver
type point of view, the counters should only be instantiated by a
platform driver (such as bus drivers) and not a device driver (such
as mux drivers.) In this example, the npcm7xx driver instantiates the
counters.

The patchset is intended to request comments on whether the goal makes
sense and what a good approach looks like.

Patch 1: add i2c statistics to i2c_adapter
Patch 2: instantiate i2c statistics from the npcm7xx i2c bus driver

TODO:
- Decide on which counters to be included (maybe start with BER and
  NACK?)
- Use enums and not strings as counter names
- Make binding more flexible so a counter can be backed by some
  function and not only pointer to a value

The desired effect can be tested by building and running an OpenBMC
distribution targeted at the npcm7xx SoC and running the image in QEMU.
The testing steps are as follows:

1. Clone the OpenBMC repository
2. `devtool modify`and apply patch to the linux-nuvoton recipe
3. Build image for quanta-gsj
4. Build QEMU
5. Run the image-bmc image in QEMU

Results:
root@gsj:/sys/class/i2c-adapter/i2c-1/stats# ls
ber_cnt       i2c_speed     nack_cnt      rec_fail_cnt  rec_succ_cnt
timeout_cnt
root@gsj:/sys/class/i2c-adapter/i2c-1/stats# cat *
0
100000
0
0
0
0

Patch list:
Sui Chen (2):
i2c debug counters as sysfs attributes
add npcm7xx debug counters as sysfs attributes

drivers/i2c/busses/i2c-npcm7xx.c |  8 ++++++++
drivers/i2c/i2c-core-base.c      |  3 +++
drivers/i2c/i2c-dev.c            | 99
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/i2c.h              | 21 +++++++++++++++++++++
4 files changed, 131 insertions(+)

Re: [RFC Patch 0/2] I2C statistics as sysfs attributes

[Wolfram Sang]

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

Hi Sui Chen,

> It appears bus errors and NACK counts are frequently found in many
> I2C controllers; a few I2C bus drivers now export those counters to
> debugfs. There is some effort in attempting to monitor a large
> number of BMCs (board management controllers) and we are wondering if
> it is possible to have a more stable and unified interface for those
> counters to be exported to compared to the debugfs, so that a single
> command can be used to obtain the counters from different types of I2C
> controllers.

There are at least two things to consider here:

1) Why do you need this information? I don't think values like NACK
count describe the health of a system. NACKs are perfectly OK in regular
I2C communication. So, for now, I think debugfs is the better place.
An exception might be the bus speed. Some people already had an interest
in this.

2) Even when this information is kept in debugfs, we can still add some
core helper to have a standardized structure for the created files. This
is independent from sysfs. I don't think I want this a standardized ABI,
currently. Unless you explain good reasons to me :)

Thanks and happy hacking,

   Wolfram


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