can: bittiming: can_calc_bittiming(): clean up SJW handling

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@mail.gmail.com

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@BL3PR11MB6484.namprd11.prod.outlook.com

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@pengutronix.de
Link: https://lore.kernel.org/all/CAMZ6RqKqhmTgUZiwe5uqUjBDnhhC2iOjZ791+Y845btJYwVDKg@mail.gmail.com
Link: https://lore.kernel.org/all/BL3PR11MB64844E3FC13C55433CDD0B3DFB449@BL3PR11MB6484.namprd11.prod.outlook.com
Suggested-by: Thomas Kopp <Thomas.Kopp@microchip.com>
Suggested-by: Vincent Mailhol <vincent.mailhol@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 46d28f3771863f3b3262d8cf69658e7428c1e387..4c9e9c0ceff300b7765ad8dee762751b8227de33 100644 (file)
--- 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;