Re: [RFC Patch v3 1/3] i2c debug counters as sysfs attributes

[Wolfram Sang]

Hi,

I finally had some time to look at your proposal. As I wrote last time,
you convinced me to have the stats in sysfs for apple-to-apple
comparisons.

One change I'd like to see is to let the I2C core handle the stats and
not the individual bus drivers. From what I see, the I2C core could
handle all this if the bus drivers use proper fault codes.

> - ber_cnt (bus error count)

I'm not sure what exactly "bus error" means in this case. But I think it
can be translated to any not-otherwise handled errno returned by
__i2c_transfer() or __i2c_smbus_transfer(). I also think it should be
named "bus_errors". Do we really need the "cnt" suffix?

> - nack_cnt (NACK count)

This would be -ENXIO for __i2c_transfer and friends. Name should be
"NACKs"?

> - rec_fail_cnt, rec_succ_cnt (recovery failure/success count)

This would be the return code of i2c_recover_bus(). Names should be
"recovery_failures" and "recovery_successes"?

> - timeout_cnt (timeout count)

This would be -ETIMEDOUT for __i2c_transfer and friends. Name should be
"timeouts"?

> - i2c_speed (bus frequency)

Yes, we can have that. I don't think this is really a stat, though. It
is an attribute of an adapter. It has been requested before:

http://patchwork.ozlabs.org/project/linux-i2c/patch/1413403411-8895-4-git-send-email-octavian.purdila@xxxxxxxxx/
http://patchwork.ozlabs.org/project/linux-i2c/patch/20181210084111.6938-2-tudor.ambarus@xxxxxxxxxxxxx/
http://patchwork.ozlabs.org/project/linux-i2c/patch/20201013100314.216154-1-tali.perry1@xxxxxxxxx/

So, I think we can tackle it again but it is orthogonal from the stats
series.

> - tx_complete_cnt (transaction completed, including both as an initiator
>   and as a target)

This would be retval == num_msgs for __i2c_transfer and friends. I also
think it should be named "transfers_completed". "tx" often goes with
"rx" as a pair. I really wondered "why only tx" first.

So, let's keep at the high level first. What do you think about my
suggestions?

Thanks and happy hacking,

   Wolfram

Attachment: signature.asc
Description: PGP signature