On 20.11.20 11:49, Marc Kleine-Budde wrote:
On 11/20/20 11:04 AM, Oliver Hartkopp wrote:
The naming of can_dlc as element of struct can_frame and also as variable
name is misleading as it claims to be a 'data length CODE' but in reality
it always was a plain data length.
With the indroduction of a new 'len' element in struct can_frame we can now
remove can_dlc as name and make clear which of the former uses was a plain
length (-> 'len') or a data length code (-> 'dlc') value.
Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
[...]
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b1729b208788..940589667a7f 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -133,11 +133,11 @@ struct gs_device_bt_const {
struct gs_host_frame {
u32 echo_id;
u32 can_id;
- u8 can_dlc;
+ u8 len;
At least in the candleLight firmware, this is the dlc value from the CAN
controller, not a sanitized len value.
Oh, yes! You are right.
In fact it looks like that all USB adapters I've checked so far provide
the raw DLC value on the USB communication layer - which is really fine!
As the gs_host_frame struct defines the USB content, can_dlc is the
better naming here.
Thanks for the review.
Best,
Oliver
u8 channel;
u8 flags;
u8 reserved;
https://github.com/candle-usb/candleLight_fw/blob/master/src/can.c#L152
if (can_is_rx_pending(hcan)) {
CAN_FIFOMailBox_TypeDef *fifo = &can->sFIFOMailBox[0];
if (fifo->RIR & CAN_RI0R_IDE) {
rx_frame->can_id = CAN_EFF_FLAG | ((fifo->RIR >> 3) & 0x1FFFFFFF);
} else {
rx_frame->can_id = (fifo->RIR >> 21) & 0x7FF;
}
if (fifo->RIR & CAN_RI0R_RTR) {
rx_frame->can_id |= CAN_RTR_FLAG;
}
rx_frame->can_dlc = fifo->RDTR & CAN_RDT0R_DLC;
rx_frame->data[0] = (fifo->RDLR >> 0) & 0xFF;
rx_frame->data[1] = (fifo->RDLR >> 8) & 0xFF;
rx_frame->data[2] = (fifo->RDLR >> 16) & 0xFF;
rx_frame->data[3] = (fifo->RDLR >> 24) & 0xFF;
rx_frame->data[4] = (fifo->RDHR >> 0) & 0xFF;
rx_frame->data[5] = (fifo->RDHR >> 8) & 0xFF;
rx_frame->data[6] = (fifo->RDHR >> 16) & 0xFF;
rx_frame->data[7] = (fifo->RDHR >> 24) & 0xFF;
can->RF0R |= CAN_RF0R_RFOM0; // release FIFO
I think we should keep the variable name can_dlc.
regards,
Marc
