On Thu. 2 Feb. 2023 at 20:09, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> Factor out the functionality of assigning a SJW default value into
> can_sjw_set_default() and the checking the SJW limits into
> can_sjw_check().
>
> This functions will be improved and called from a different function
> in the following patches.
>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
> drivers/net/can/dev/bittiming.c | 30 ++++++++++++++++++++++++++----
> include/linux/can/bittiming.h | 5 +++++
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c
> index 0b0b8c767c5b..101de1b3bf30 100644
> --- a/drivers/net/can/dev/bittiming.c
> +++ b/drivers/net/can/dev/bittiming.c
> @@ -6,6 +6,24 @@
>
> #include <linux/can/dev.h>
>
> +void can_sjw_set_default(struct can_bittiming *bt)
> +{
> + if (bt->sjw)
> + return;
> +
> + /* If user space provides no sjw, use 1 as default */
> + bt->sjw = 1;
> +}
> +
> +int can_sjw_check(const struct net_device *dev, const struct can_bittiming *bt,
> + const struct can_bittiming_const *btc, struct netlink_ext_ack *extack)
> +{
> + if (bt->sjw > btc->sjw_max)
> + return -ERANGE;
You return -ERANGE here but then replace it by -EINVAL in patch #12.
Better to directly return -EINVAL here.
> + return 0;
> +}
> +
> /* Checks the validity of the specified bit-timing parameters prop_seg,
> * phase_seg1, phase_seg2 and sjw and tries to determine the bitrate
> * prescaler value brp. You can find more information in the header
> @@ -18,12 +36,16 @@ static int can_fixup_bittiming(const struct net_device *dev, struct can_bittimin
> const struct can_priv *priv = netdev_priv(dev);
> unsigned int tseg1;
> u64 brp64;
> + int err;
> +
> + can_sjw_set_default(bt);
> +
> + err = can_sjw_check(dev, bt, btc, extack);
> + if (err)
> + return err;
>
> tseg1 = bt->prop_seg + bt->phase_seg1;
> - if (!bt->sjw)
> - bt->sjw = 1;
> - if (bt->sjw > btc->sjw_max ||
> - tseg1 < btc->tseg1_min || tseg1 > btc->tseg1_max ||
> + if (tseg1 < btc->tseg1_min || tseg1 > btc->tseg1_max ||
> bt->phase_seg2 < btc->tseg2_min || bt->phase_seg2 > btc->tseg2_max)
> return -ERANGE;
>
> diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
> index 53d693ae5397..6cb2ae308e3f 100644
> --- a/include/linux/can/bittiming.h
> +++ b/include/linux/can/bittiming.h
> @@ -138,6 +138,11 @@ can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
> }
> #endif /* CONFIG_CAN_CALC_BITTIMING */
>
> +void can_sjw_set_default(struct can_bittiming *bt);
> +
> +int can_sjw_check(const struct net_device *dev, const struct can_bittiming *bt,
> + const struct can_bittiming_const *btc, struct netlink_ext_ack *extack);
> +
> int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> const struct can_bittiming_const *btc,
> const u32 *bitrate_const,
