In the current code, if the user configures a bitrate, a default SJW value of 1 is used. If the user configures both a bitrate and a SJW value, can_calc_bittiming() silently limits the SJW value to SJW max and TSEG2. We came to the conclusion that if the user provided an invalid SJW value, it's best to bail out and inform the user [1]. [1] https://lore.kernel.org/all/CAMZ6RqKqhmTgUZiwe5uqUjBDnhhC2iOjZ791+Y845btJYwVDKg@xxxxxxxxxxxxxx Further the ISO 11898-1:2015 standard mandates that "SJW shall be less than or equal to the minimum of these two items: Phase_Seg1 and Phase_Seg2." [2] The current code is missing that check. [2] https://lore.kernel.org/all/BL3PR11MB64844E3FC13C55433CDD0B3DFB449@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx The previous patches introduced 1) can_sjw_set_default() - sets a default value for SJW if unset 2) can_sjw_check() - implements a SJW check against SJW max, Phase Seg1 and Phase Seg2. In the error case this function reports the error to user space via netlink. Replace both the open-coded SJW default setting and the open-coded and insufficient checks of SJW with the helper functions can_sjw_set_default() and can_sjw_check(). Link: https://lore.kernel.org/all/20230202110854.2318594-16-mkl@xxxxxxxxxxxxxx Link: https://lore.kernel.org/all/CAMZ6RqKqhmTgUZiwe5uqUjBDnhhC2iOjZ791+Y845btJYwVDKg@xxxxxxxxxxxxxx Link: https://lore.kernel.org/all/BL3PR11MB64844E3FC13C55433CDD0B3DFB449@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Suggested-by: Thomas Kopp <Thomas.Kopp@xxxxxxxxxxxxx> Suggested-by: Vincent Mailhol <vincent.mailhol@xxxxxxxxx> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> --- drivers/net/can/dev/calc_bittiming.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c index 46d28f377186..4c9e9c0ceff3 100644 --- a/drivers/net/can/dev/calc_bittiming.c +++ b/drivers/net/can/dev/calc_bittiming.c @@ -76,6 +76,7 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt, unsigned int best_brp = 0; /* current best value for brp */ unsigned int brp, tsegall, tseg, tseg1 = 0, tseg2 = 0; u64 v64; + int err; /* Use CiA recommended sample points */ if (bt->sample_point) { @@ -154,17 +155,11 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt, bt->phase_seg1 = tseg1 - bt->prop_seg; bt->phase_seg2 = tseg2; - /* check for sjw user settings */ - if (!bt->sjw || !btc->sjw_max) { - bt->sjw = 1; - } else { - /* bt->sjw is at least 1 -> sanitize upper bound to sjw_max */ - if (bt->sjw > btc->sjw_max) - bt->sjw = btc->sjw_max; - /* bt->sjw must not be higher than tseg2 */ - if (tseg2 < bt->sjw) - bt->sjw = tseg2; - } + can_sjw_set_default(bt); + + err = can_sjw_check(dev, bt, btc, extack); + if (err) + return err; bt->brp = best_brp; -- 2.39.1
