Re: [PATCH v2 00/17] can: bittiming: cleanups and rework SJW handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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().



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux