On Thu. 2 Feb 2023 at 20:53, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: > On 02.02.2023 20:51:04, Vincent Mailhol wrote: > > 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. > > ACK. > > > Better to directly return -EINVAL here. > > This patch only factors out the functionality, but doesn't change it on > purpose. ACK. I thought it was a leftover. I would not have done it that way, but I do not want to over argue on this nitpick. So OK for me.