I finished a first quick review and sent my nitpicks. Really great work, thank you! On Thu. 2 Feb 2023 at 20:08, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: > Hello, > > several people noticed that on modern CAN controllers with wide bit > timing registers the default SJW of 1 can result in unstable or no > synchronization to the CAN network. See Patch 14/17 for details. If this series addresses an actual issue, should it be backported to stable branches? > During review of v1 Vincent pointed out that the original code and the > series doesn't always check user provided bit timing parameters, > sometimes silently limits them and the return error values are not > consistent. > > This series first cleans up some code in bittiming.c, replacing > open-coded variants by macros or functions (Patches 1, 2). > > Patch 3 adds the missing assignment of the effective TQ if the > interface is configured with low level timing parameters. > > Patch 4 is another code cleanup. > > Patches 5, 6 check the bit timing parameter during interface > registration. > > Patch 7 adds a validation of the sample point. > > The patches 8-13 convert the error messages from netdev_err() to > NL_SET_ERR_MSG_FMT, factor out the SJW handling from > can_fixup_bittiming(), add checking and error messages for the > individual limits and harmonize the error return values. > > Patch 14 changes the default SJW value from 1 to min(Phase Seg1, Phase > Seg2 / 2). > > Patch 15 switches can_calc_bittiming() to use the new SJW handling. > > Patch 16 converts can_calc_bittiming() to NL_SET_ERR_MSG_FMT(). > > And patch 16 adds a NL_SET_ERR_MSG_FMT() error message to > can_validate_bitrate().
