| Message ID | 20241029114622.2989827-1-dario.binacchi@amarulasolutions.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
On Tue, Oct 29, 2024 at 12:46 PM Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > > This series originates from some tests I ran on a CAN communication for > one of my clients that reports sporadic errors. After enabling BERR > reporting, I was surprised that the command: > > ip -details -statistics link show can0 > > did not display the occurrence of different types of errors, but only the > generic ones for reception and transmission. In trying to export this > information, I felt that the code related to managing statistics and handling > CAN errors (CRC, STUF, BIT, ACK, and FORM) was quite duplicated in the > implementation of various drivers, and there wasn't a generic function like > in the case of state changes (i. e. can_change_state). This led to the idea > of adding can_update_bus_error_stats() and the helpers for setting up the > CAN error frame. > > Regarding patch 5/6 ("can: netlink: extend stats to the error types (ack, > CRC, form, ..."), I ran > > ./scripts/check-uapi.sh > > which found > > "error - 1/934 UAPI headers compatible with x86 appear _not_ to be backwards > compatible." > > I included it in the series because I am currently interested in understanding > whether the idea behind each of the submitted patches makes sense, and I can > adjust them later if the response is positive, following your suggestions. > > Changes in v3: > - Drop double assignement of "priv" variable. > - Check "dev" parameter is not NULL. > - Drop the check of "cf" parameter not NULL > > Changes in v2: > - Replace macros with static inline functions > - Update the commit message > - Replace the macros with static inline funcions calls. > - Update the commit message > > Dario Binacchi (6): > can: dev: add generic function can_update_bus_error_stats() > can: flexcan: use can_update_bus_error_stats() > can: dev: add helpers to setup an error frame > can: flexcan: use helpers to setup the error frame > can: netlink: extend stats to the error types (ack, CRC, form, ...) > can: dev: update the error types stats (ack, CRC, form, ...) > > drivers/net/can/dev/dev.c | 45 ++++++++++++++++++++++++++ > drivers/net/can/flexcan/flexcan-core.c | 29 +++++------------ > include/linux/can/dev.h | 38 ++++++++++++++++++++++ > include/uapi/linux/can/netlink.h | 6 ++++ > 4 files changed, 97 insertions(+), 21 deletions(-) > > -- > 2.43.0 > A gentle ping to remind you of this series. Could this series or some of its patches make sense to consider? IMHO, if all the controllers indicate the type of error, I would expect the user space to be aware of it as well. Or is there something I might be missing? Thanks and regards, Dario
This series originates from some tests I ran on a CAN communication for one of my clients that reports sporadic errors. After enabling BERR reporting, I was surprised that the command: ip -details -statistics link show can0 did not display the occurrence of different types of errors, but only the generic ones for reception and transmission. In trying to export this information, I felt that the code related to managing statistics and handling CAN errors (CRC, STUF, BIT, ACK, and FORM) was quite duplicated in the implementation of various drivers, and there wasn't a generic function like in the case of state changes (i. e. can_change_state). This led to the idea of adding can_update_bus_error_stats() and the helpers for setting up the CAN error frame. Regarding patch 5/6 ("can: netlink: extend stats to the error types (ack, CRC, form, ..."), I ran ./scripts/check-uapi.sh which found "error - 1/934 UAPI headers compatible with x86 appear _not_ to be backwards compatible." I included it in the series because I am currently interested in understanding whether the idea behind each of the submitted patches makes sense, and I can adjust them later if the response is positive, following your suggestions. Changes in v3: - Drop double assignement of "priv" variable. - Check "dev" parameter is not NULL. - Drop the check of "cf" parameter not NULL Changes in v2: - Replace macros with static inline functions - Update the commit message - Replace the macros with static inline funcions calls. - Update the commit message Dario Binacchi (6): can: dev: add generic function can_update_bus_error_stats() can: flexcan: use can_update_bus_error_stats() can: dev: add helpers to setup an error frame can: flexcan: use helpers to setup the error frame can: netlink: extend stats to the error types (ack, CRC, form, ...) can: dev: update the error types stats (ack, CRC, form, ...) drivers/net/can/dev/dev.c | 45 ++++++++++++++++++++++++++ drivers/net/can/flexcan/flexcan-core.c | 29 +++++------------ include/linux/can/dev.h | 38 ++++++++++++++++++++++ include/uapi/linux/can/netlink.h | 6 ++++ 4 files changed, 97 insertions(+), 21 deletions(-)