On 10/28/20 12:54 PM, Oliver Hartkopp wrote:
>
>
> On 28.10.20 12:20, Marc Kleine-Budde wrote:
>> On 10/28/20 12:00 PM, Oliver Hartkopp wrote:
>>> Cleanup the can_dlc usage by renaming it with len from can_frame.len
>>>
>>> Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
>>
>> [...]
>>
>>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>>> index 6dee4f8f2024..537dd4636c90 100644
>>> --- a/drivers/net/can/dev.c
>>> +++ b/drivers/net/can/dev.c
>>> @@ -28,14 +28,14 @@ MODULE_AUTHOR("Wolfgang Grandegger <wg@xxxxxxxxxxxxxx>");
>>> /* CAN DLC to real data length conversion helpers */
>>>
>>> static const u8 dlc2len[] = {0, 1, 2, 3, 4, 5, 6, 7,
>>> 8, 12, 16, 20, 24, 32, 48, 64};
>>>
>>> -/* get data length from can_dlc with sanitized can_dlc */
>>> -u8 can_dlc2len(u8 can_dlc)
>>> +/* get data length from dlc with sanitized len */
>>> +u8 can_dlc2len(u8 dlc)
>>> {
>>> - return dlc2len[can_dlc & 0x0F];
>>> + return dlc2len[dlc & 0x0F];
>>> }
>>> EXPORT_SYMBOL_GPL(can_dlc2len);
>>
>> unrelated change.
>
> Probably the commit message has to be altered.
> The plan is to remove can_dlc as it was misnamed/misused.
Make it so!
> In this specific case we provide a real (raw) dlc and not the can_dlc
> which was a length value.
ok
>> [...]
>>
>>
>>> diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canfd/peak_canfd.c
>>> index cc01db0c18b8..c25cb53eba4e 100644
>>> --- a/drivers/net/can/peak_canfd/peak_canfd.c
>>> +++ b/drivers/net/can/peak_canfd/peak_canfd.c
>>
>> [...]
>>
>>> @@ -650,11 +650,11 @@ static netdev_tx_t peak_canfd_start_xmit(struct sk_buff *skb,
>>> struct pucan_tx_msg *msg;
>>> u16 msg_size, msg_flags;
>>> unsigned long flags;
>>> bool should_stop_tx_queue;
>>> int room_left;
>>> - u8 can_dlc;
>>> + u8 len;
>>
>> unrelated change
>
> Same here. can_dlc was just wrong in all cases.
ok
>>>
>>> if (can_dropped_invalid_skb(ndev, skb))
>>> return NETDEV_TX_OK;
>>>
>>> msg_size = ALIGN(sizeof(*msg) + cf->len, 4);
>>> @@ -680,22 +680,22 @@ static netdev_tx_t peak_canfd_start_xmit(struct sk_buff *skb,
>>> msg->can_id = cpu_to_le32(cf->can_id & CAN_SFF_MASK);
>>> }
>>>
>>> if (can_is_canfd_skb(skb)) {
>>> /* CAN FD frame format */
>>> - can_dlc = can_len2dlc(cf->len);
>>> + len = can_len2dlc(cf->len);
>>>
>>> msg_flags |= PUCAN_MSG_EXT_DATA_LEN;
>>>
>>> if (cf->flags & CANFD_BRS)
>>> msg_flags |= PUCAN_MSG_BITRATE_SWITCH;
>>>
>>> if (cf->flags & CANFD_ESI)
>>> msg_flags |= PUCAN_MSG_ERROR_STATE_IND;
>>> } else {
>>> /* CAN 2.0 frame format */
>>> - can_dlc = cf->len;
>>> + len = cf->len;
>>>
>>> if (cf->can_id & CAN_RTR_FLAG)
>>> msg_flags |= PUCAN_MSG_RTR;
>>> }
>>>
>>> @@ -705,11 +705,11 @@ static netdev_tx_t peak_canfd_start_xmit(struct sk_buff *skb,
>>> /* set driver specific bit to differentiate with application loopback */
>>> if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> msg_flags |= PUCAN_MSG_SELF_RECEIVE;
>>>
>>> msg->flags = cpu_to_le16(msg_flags);
>>> - msg->channel_dlc = PUCAN_MSG_CHANNEL_DLC(priv->index, can_dlc);
>>> + msg->channel_dlc = PUCAN_MSG_CHANNEL_DLC(priv->index, len);
>>> memcpy(msg->d, cf->data, cf->len);
>>>
>>> /* struct msg client field is used as an index in the echo skbs ring */
>>> msg->client = priv->echo_idx;
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
>>> index ccd649a8e37b..7e1536877993 100644
>>> --- a/drivers/net/can/softing/softing_fw.c
>>> +++ b/drivers/net/can/softing/softing_fw.c
>>> @@ -622,11 +622,11 @@ int softing_startstop(struct net_device *dev, int up)
>>> * from here, no errors should occur, or the failed: part
>>> * must be reviewed
>>> */
>>> memset(&msg, 0, sizeof(msg));
>>> msg.can_id = CAN_ERR_FLAG | CAN_ERR_RESTARTED;
>>> - msg.can_dlc = CAN_ERR_DLC;
>>> + msg.len = CAN_ERR_DLC;
>>
>> unrelated?
>
> No, definitely not. msg is a struct can_frame. Therefore 'len' should be
> used.
I missed that. tnx.
> When implementing the stuff for len8_dlc we are now very clear where we
> have a length value and where we have a data length code.
>
> (..) (skipping tons of 'same here?')
>
>>> -/* get data length from can_dlc with sanitized can_dlc */
>>> -u8 can_dlc2len(u8 can_dlc);
>>> +/* get data length from dlc with sanitized len */
>>> +u8 can_dlc2len(u8 dlc);
>>
>> unrealted?
>
> Same as at its first occurance. Remove can_dlc from the radar.
regards,
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
