On 03/02/2016 07:46 PM, Oliver Hartkopp wrote:
> Hi Marek,
Hi!
> sorry for picking this patch up again.
nothing to be sorry about, thanks for the input :)
> After looking around in the original source I have one more comment:
>
> On 03/02/2016 11:42 AM, Marek Vasut wrote:
>
>> - if (priv->can.ctrlmode & (CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO)) {
>> - if (can_is_canfd_skb(skb)) {
>> - txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
>> - if (cf->flags & CANFD_BRS)
>> - txdlc |= IFI_CANFD_TXFIFO_DLC_BRS;
>> - }
>> + txdlc |= can_len2dlc(cf->len);
>
> At the top of the function you defined
>
> u32 txst, txid;
> u32 txdlc = 0;
>
> and here you use 'txdlc |= ...'.
>
> That's weird style IMO.
>
> Please change it to
>
> u32 txst, txid, txdlc;
>
> and
>
> txdlc = can_len2dlc(cf->len);
I'll send a V3 shortly.
btw. I forgot to add V2 to the 3/4 and 4/4 patches, sigh.
--
Best regards,
Marek Vasut
