On Mon. 9 Nov 2020 at 19:26, Oliver Hartkopp wrote: > diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h > index b2e8df8e4cb0..72671184a7a2 100644 > --- a/include/linux/can/dev.h > +++ b/include/linux/can/dev.h > @@ -183,12 +183,12 @@ static inline void can_set_static_ctrlmode(struct net_device *dev, > /* override MTU which was set by default in can_setup()? */ > if (static_mode & CAN_CTRLMODE_FD) > dev->mtu = CANFD_MTU; > } > > -/* get data length from can_dlc with sanitized can_dlc */ > -u8 can_dlc2len(u8 can_dlc); > +/* get data length from raw data length code (DLC) */ /* * convert a given data length code (dlc) of an FD CAN frame into a * valid data length of max. 64 bytes. */ I missed this point during my previous review: the can_dlc2len() function is only valid for CAN FD frames. Comments should reflect this fact. > +u8 can_dlc2len(u8 dlc); Concerning the name: * can_get_cc_len() converts a Classical CAN frame DLC into a data length. * can_dlc2len() converts an FD CAN frame DLC into a data length. Just realized that both macro/function do similar things so we could think of a similar naming as well. * Example 1: can_get_cc_len() and can_get_fd_len() * Example 2: can_cc_dlc2len() and can_fd_dlc2len() Or we could simply leave things as they are, this is not a big issue as long as the comments clearly state which one is for classical frames and which one is for FD frames. > > /* map the sanitized data length to an appropriate data length code */ > u8 can_len2dlc(u8 len); can_len2dlc() might be renamed (e.g. can_get_fd_dlc()) if Example 1 solution is chosen. > struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max, Yours sincerely, Vincent Mailhol
