From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Hartkopp Subject: Re: [PATCH 2/4] net: can: ifi: Fix TX DLC configuration Date: Wed, 2 Mar 2016 07:12:58 +0100 Message-ID: <56D6846A.9020808@hartkopp.net> References: <1456775971-4946-1-git-send-email-marex@denx.de> <1456775971-4946-3-git-send-email-marex@denx.de> <56D5DB54.6000600@hartkopp.net> <56D60938.6060903@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.216]:56546 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750797AbcCBGNC (ORCPT ); Wed, 2 Mar 2016 01:13:02 -0500 In-Reply-To: <56D60938.6060903@denx.de> Sender: linux-can-owner@vger.kernel.org List-ID: To: Marek Vasut , linux-can@vger.kernel.org Cc: netdev@vger.kernel.org, Marc Kleine-Budde , Mark Rutland , Wolfgang Grandegger On 03/01/2016 10:27 PM, Marek Vasut wrote: > On 03/01/2016 07:11 PM, Oliver Hartkopp wrote: > > Hi! > >> On 02/29/2016 08:59 PM, Marek Vasut wrote: >>> The TX DLC, the transmission length information, was not written >>> into the transmit configuration register. When using the CAN core >>> with different CAN controller, the receiving CAN controller will >>> receive only the ID part of the CAN frame, but no data at all. >>> >>> This patch adds the TX DLC into the register to fix this issue. >>> >>> Signed-off-by: Marek Vasut >>> Cc: Marc Kleine-Budde >>> Cc: Mark Rutland >>> Cc: Oliver Hartkopp >>> Cc: Wolfgang Grandegger >>> --- >>> drivers/net/can/ifi_canfd/ifi_canfd.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c >>> index 72f5205..82a33bd 100644 >>> --- a/drivers/net/can/ifi_canfd/ifi_canfd.c >>> +++ b/drivers/net/can/ifi_canfd/ifi_canfd.c >>> @@ -774,10 +774,15 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb, >>> >>> if (priv->can.ctrlmode & (CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO)) { >>> if (can_is_canfd_skb(skb)) { >>> + txdlc |= can_len2dlc(cf->len); >>> txdlc |= IFI_CANFD_TXFIFO_DLC_EDL; >>> if (cf->flags & CANFD_BRS) >>> txdlc |= IFI_CANFD_TXFIFO_DLC_BRS; >>> + } else { >>> + txdlc |= cf->len; >>> } >>> + } else { >>> + txdlc |= cf->len; >>> } >> >> Please use >> >> txdlc |= can_len2dlc(cf->len); >> >> by default (it works for CAN and CAN FD). >> >> So that it looks more like: >> >> txdlc |= can_len2dlc(cf->len); > > Roger. > >> if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) { >> txdlc |= IFI_CANFD_TXFIFO_DLC_EDL; >> if (cf->flags & CANFD_BRS) >> txdlc |= IFI_CANFD_TXFIFO_DLC_BRS; >> } >> >> Testing against CAN_CTRLMODE_FD_NON_ISO is wrong! >> This configuration bit is just for the protocol on the wire and is no >> distinction for CAN / CAN FD. > > So CAN_CTRLMODE_FD is always set if the system operates in CAN/FD mode. Not the 'system' but this specific CAN netdevice. > And in addition to that, if the system operates in CAN/FD BOSCH mode, > the CAN_CTRLMODE_FD_NON_ISO is set. Do I understand it correctly ? Yep! ('the CAN netdev') Regards, Oliver