Re: [PATCH 2/5] can: bittiming: can_calc_bittiming(): make use of min3()

[Vincent Mailhol <vincent.mailhol@gmail.com>]

On Wed. 7 sept. 2022 à 19:59, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> In can_calc_bittiming() there are several open coded checks to ensure
> that SJW is within certain limits. Replace this by a single call to
> min3().
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/dev/calc_bittiming.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
> index d3caa040614d..ce6bef2444a2 100644
> --- a/drivers/net/can/dev/calc_bittiming.c
> +++ b/drivers/net/can/dev/calc_bittiming.c
> @@ -158,12 +158,8 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
>         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;
> +               /* sjw must not be higher than sjw_max and tseg2 */
> +               bt->sjw = min3(bt->sjw, btc->sjw_max, tseg2);

Not directly a criticism of this patch (as things were already like
that), but if the user provides an incorrect value for SJW (or any
other bittiming argument), wouldn't it be better to inform? Returning
-EINVAL might be too violent. Maybe a dmesg would be good?


Yours sincerely,
Vincent Mailhol