Messages in this thread | | | Subject | Re: [PATCH] can: bittiming: replace CAN units with the SI metric | From | Oliver Hartkopp <> | Date | Sun, 21 Nov 2021 19:27:47 +0100 |
| |
On 19.11.21 17:18, Vincent Mailhol wrote: > In [1], we introduced a set of units in linux/can/bittiming.h. Since > then, generic SI prefix were added to linux/units.h in [2]. Those new > prefix can perfectly replace the CAN specific units. > > This patch replaces all occurrences of the CAN units with their > corresponding prefix according to below table. > > CAN units SI metric prefix > ------------------------------- > CAN_KBPS KILO > CAN_MBPS MEGA > CAM_MHZ MEGA > > The macro declarations are then removed from linux/can/bittiming.h > > [1] commit 1d7750760b70 ("can: bittiming: add CAN_KBPS, CAN_MBPS and > CAN_MHZ macros") > > [2] commit 26471d4a6cf8 ("units: Add SI metric prefix definitions") > > Suggested-by: Jimmy Assarsson <extja@kvaser.com> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > drivers/net/can/dev/bittiming.c | 5 +++-- > drivers/net/can/usb/etas_es58x/es581_4.c | 5 +++-- > drivers/net/can/usb/etas_es58x/es58x_fd.c | 5 +++-- > include/linux/can/bittiming.h | 7 ------- > 4 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c > index 0509625c3082..a5c9f973802a 100644 > --- a/drivers/net/can/dev/bittiming.c > +++ b/drivers/net/can/dev/bittiming.c > @@ -4,6 +4,7 @@ > * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com> > */ > > +#include <linux/units.h> > #include <linux/can/dev.h> > > #ifdef CONFIG_CAN_CALC_BITTIMING > @@ -81,9 +82,9 @@ int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt, > if (bt->sample_point) { > sample_point_nominal = bt->sample_point; > } else { > - if (bt->bitrate > 800 * CAN_KBPS) > + if (bt->bitrate > 800 * KILO) > sample_point_nominal = 750; > - else if (bt->bitrate > 500 * CAN_KBPS) > + else if (bt->bitrate > 500 * KILO) > sample_point_nominal = 800; > else > sample_point_nominal = 875; > diff --git a/drivers/net/can/usb/etas_es58x/es581_4.c b/drivers/net/can/usb/etas_es58x/es581_4.c > index 14e360c9f2c9..ed340141c712 100644 > --- a/drivers/net/can/usb/etas_es58x/es581_4.c > +++ b/drivers/net/can/usb/etas_es58x/es581_4.c > @@ -10,6 +10,7 @@ > */ > > #include <linux/kernel.h> > +#include <linux/units.h> > #include <asm/unaligned.h> > > #include "es58x_core.h" > @@ -469,8 +470,8 @@ const struct es58x_parameters es581_4_param = { > .bittiming_const = &es581_4_bittiming_const, > .data_bittiming_const = NULL, > .tdc_const = NULL, > - .bitrate_max = 1 * CAN_MBPS, > - .clock = {.freq = 50 * CAN_MHZ}, > + .bitrate_max = 1 * MEGA, > + .clock = {.freq = 50 * MEGA},
IMO we are losing information here.
It feels you suggest to replace MHz with M.
So where is the Hz information then?
> .ctrlmode_supported = CAN_CTRLMODE_CC_LEN8_DLC, > .tx_start_of_frame = 0xAFAF, > .rx_start_of_frame = 0xFAFA, > diff --git a/drivers/net/can/usb/etas_es58x/es58x_fd.c b/drivers/net/can/usb/etas_es58x/es58x_fd.c > index 4f0cae29f4d8..aec299bed6dc 100644 > --- a/drivers/net/can/usb/etas_es58x/es58x_fd.c > +++ b/drivers/net/can/usb/etas_es58x/es58x_fd.c > @@ -12,6 +12,7 @@ > */ > > #include <linux/kernel.h> > +#include <linux/units.h> > #include <asm/unaligned.h> > > #include "es58x_core.h" > @@ -522,8 +523,8 @@ const struct es58x_parameters es58x_fd_param = { > * Mbps work in an optimal environment but are not recommended > * for production environment. > */ > - .bitrate_max = 8 * CAN_MBPS, > - .clock = {.freq = 80 * CAN_MHZ}, > + .bitrate_max = 8 * MEGA, > + .clock = {.freq = 80 * MEGA}, > .ctrlmode_supported = CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY | > CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO | > CAN_CTRLMODE_CC_LEN8_DLC | CAN_CTRLMODE_TDC_AUTO, > diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h > index 20b50baf3a02..a81652d1c6f3 100644 > --- a/include/linux/can/bittiming.h > +++ b/include/linux/can/bittiming.h > @@ -12,13 +12,6 @@ > #define CAN_SYNC_SEG 1 > > > -/* Kilobits and Megabits per second */ > -#define CAN_KBPS 1000UL > -#define CAN_MBPS 1000000UL > - > -/* Megahertz */ > -#define CAN_MHZ 1000000UL
So what about
#define CAN_KBPS KILO /* kilo bits per second */ #define CAN_MBPS MEGA /* mega bits per second */
#define CAN_MHZ MEGA /* mega hertz */
??
Regards, Oliver
> - > #define CAN_CTRLMODE_TDC_MASK \ > (CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_TDC_MANUAL) > >
| |