On 11/10/20 7:55 AM, Oliver Hartkopp wrote: >>> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c >>> index 940589667a7f..cc0c30a33335 100644 >>> --- a/drivers/net/can/usb/gs_usb.c >>> +++ b/drivers/net/can/usb/gs_usb.c >>> @@ -330,10 +330,13 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) >>> return; >>> >>> cf->can_id = hf->can_id; >>> >>> cf->len = can_cc_dlc2len(hf->len); >>> + cf->len8_dlc = can_get_len8_dlc(dev->can.ctrlmode, cf->len, >>> + hf->len); >> >> What about introducing a function that sets len and len8_dlc at the same time: >> >> void can_frame_set_length(const struct can_priv *can, struct can_frame *cfd, u8 >> dlc); > > Good idea. > > I would suggest something like > > u8 can_get_cc_len(const u32 ctrlmode, struct can_frame *cf, u8 dlc) > > that still returns the 'len' element, so that we can replace > can_cc_dlc2len() with can_get_cc_len() for CAN drivers that add support > for len8_dlc. The regex to replace can_cc_dlc2len() with can_get_cc_len() might be simpler, but passing the cf by reference _and_ assigning the return value to a member of cf looks strange. > The assignment cf->len = can_get_cc_len() fits better into the code > which assigns cf->can_id too. > > And I would stay on 'u32 ctrlmode' as ctrlmode is the parameter which is > namely needed here. A pointer to can_priv can mean anything. OK >> And maybe a function that takes a canfd_frame, so that we don't need to cast.... > > No. The len8_dlc element is from struct can_frame. When people use the > struct canfd_frame in their driver this might have some benefits for them. > But when it comes to access the len8_dlc element this has to be casted IMO. > > But with the suggested can_get_cc_len() function a needed cast could be > put into the parameter list without adding extra code somewhere else in > the driver. OK Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: OpenPGP digital signature
