From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Hartkopp Subject: Re: [PATCH 3/4] net: can: ifi: Fix RX and TX ID mask Date: Wed, 2 Mar 2016 07:10:10 +0100 Message-ID: <56D683C2.5080402@hartkopp.net> References: <1456775971-4946-1-git-send-email-marex@denx.de> <1456775971-4946-4-git-send-email-marex@denx.de> <56D5D635.1050005@hartkopp.net> <56D60850.8080103@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56D60850.8080103@denx.de> Sender: netdev-owner@vger.kernel.org To: Marek Vasut , linux-can@vger.kernel.org Cc: netdev@vger.kernel.org, Marc Kleine-Budde , Mark Rutland , Wolfgang Grandegger List-Id: linux-can.vger.kernel.org Hi Marek, On 03/01/2016 10:23 PM, Marek Vasut wrote: > On 03/01/2016 06:49 PM, Oliver Hartkopp wrote: >>> -#define IFI_CANFD_RXFIFO_ID_ID_STD_MASK 0x3ff >>> +#define IFI_CANFD_RXFIFO_ID_ID_STD_MASK 0x7ff >>> #define IFI_CANFD_RXFIFO_ID_ID_XTD_MASK 0x1fffffff >> >> You should use the CAN_SFF_MASK and CAN_EFF_MASK in your code instead of >> defining you private IFI_CANFD_RXFIFO_ID_ID_?TD_MASK definitions. >> >> You won't have trapped into this problem then :-) > > These are register bitfield definitions, so should I really ? > > My OCD kicks in and tells me it'd be odd and inconsistent with the rest > of the bitfields, but if you prefer it that way, I'll just send an > updated patch. > Your bit mask is masking the CAN ID out of a given variable. That's what CAN_SFF_MASK and CAN_EFF_MASK is made for. So at least it should be: #define IFI_CANFD_RXFIFO_ID_ID_STD_MASK CAN_SFF_MASK #define IFI_CANFD_RXFIFO_ID_ID_XTD_MASK CAN_EFF_MASK Btw. These defines are _never_ referenced in ifi_canfd.c so they should be removed anyway. Regards, Oliver