On 09.11.20 21:12, Marc Kleine-Budde wrote:
On 11/9/20 4:36 PM, Oliver Hartkopp wrote:
Support the Classical CAN raw DLC functionality to send and receive DLC
values from 9 .. 15 on various Classical CAN capable USB network drivers:
- gs_usb
- pcan_usb
- pcan_usb_fd
- usb_8dev
Tested-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
---
drivers/net/can/usb/gs_usb.c | 8 ++++++--
drivers/net/can/usb/peak_usb/pcan_usb.c | 8 ++++++--
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 17 ++++++++++++-----
drivers/net/can/usb/usb_8dev.c | 9 ++++++---
4 files changed, 30 insertions(+), 12 deletions(-)
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 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.
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.
(..)
@@ -1033,11 +1040,11 @@ static const struct can_bittiming_const pcan_usb_fd_data_const = {
const struct peak_usb_adapter pcan_usb_fd = {
.name = "PCAN-USB FD",
.device_id = PCAN_USBFD_PRODUCT_ID,
.ctrl_count = PCAN_USBFD_CHANNEL_COUNT,
- .ctrlmode_supported = CAN_CTRLMODE_FD |
+ .ctrlmode_supported = CAN_CTRLMODE_FD | CAN_CTRLMODE_CC_LEN8_DLC |
CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY,
Please add the new CTRLMODE at the end, so that the list ist sorted. I don't
mind if the diff is a bit larger.
I had to decide between "make the patch looking good" and "make the
result looking good" ;-)
But I'm fine with "make the patch looking good" too. Will change that in v6.
Best regards,
Oliver
