[PATCH V4 0/4] Synchronise IFI CANFD driver with real world

[PATCH V4 0/4] Synchronise IFI CANFD driver with real world

[Marek Vasut]

Thus far, this driver was only tested on a hardware synthesised in
the warm and safe insides of an FPGA, only against another IFI CANFD
core. The real hardware arrived now and I tested the IFI CANFD driver
against different, harsh, real-world CAN controller.

This uncovered a few bugs, so here are the fixes for those.

Marek Vasut (4):
  net: can: ifi: Fix clock generator configuration
  net: can: ifi: Fix TX DLC configuration
  net: can: ifi: Fix RX and TX ID mask
  net: can: ifi: Add obscure bit swap for EFF frame IDs

 drivers/net/can/ifi_canfd/ifi_canfd.c | 97 ++++++++++++++++++++++-------------
 1 file changed, 62 insertions(+), 35 deletions(-)

-- 
2.7.0

[PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marek Vasut]

The clock generation does not match reality when using the CAN IP
core outside of the FPGA design. This patch fixes the computation
of values which are programmed into the clock generator registers.

First, there are some off-by-one errors which manifest themselves
only when communicating with different controller, so those are
fixed.

Second, the bits in the clock generator registers have different
meaning depending on whether the core is in ISO CANFD mode or any
of the other modes (BOSCH CANFD or CAN2.0). Detect the ISO CANFD
mode and fix handling of this special case of clock configuration.

Finally, the CAN clock speed is in CANCLOCK register, not SYSCLOCK
register, so fix this as well.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
---
V2: none
V3: none
V4: Only use the CANFD-ISO configuration of the timing register
    for CANFD-ISO mode. Make sure this configuration is not used
    for CANFD-BOSCH mode.
---
 drivers/net/can/ifi_canfd/ifi_canfd.c | 44 ++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 639868b..86287a3 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -514,25 +514,25 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
 
 static const struct can_bittiming_const ifi_canfd_bittiming_const = {
 	.name		= KBUILD_MODNAME,
-	.tseg1_min	= 2,	/* Time segment 1 = prop_seg + phase_seg1 */
+	.tseg1_min	= 1,	/* Time segment 1 = prop_seg + phase_seg1 */
 	.tseg1_max	= 64,
-	.tseg2_min	= 1,	/* Time segment 2 = phase_seg2 */
-	.tseg2_max	= 16,
+	.tseg2_min	= 2,	/* Time segment 2 = phase_seg2 */
+	.tseg2_max	= 64,
 	.sjw_max	= 16,
-	.brp_min	= 1,
-	.brp_max	= 1024,
+	.brp_min	= 2,
+	.brp_max	= 256,
 	.brp_inc	= 1,
 };
 
 static const struct can_bittiming_const ifi_canfd_data_bittiming_const = {
 	.name		= KBUILD_MODNAME,
-	.tseg1_min	= 2,	/* Time segment 1 = prop_seg + phase_seg1 */
-	.tseg1_max	= 16,
-	.tseg2_min	= 1,	/* Time segment 2 = phase_seg2 */
-	.tseg2_max	= 8,
-	.sjw_max	= 4,
-	.brp_min	= 1,
-	.brp_max	= 32,
+	.tseg1_min	= 1,	/* Time segment 1 = prop_seg + phase_seg1 */
+	.tseg1_max	= 64,
+	.tseg2_min	= 2,	/* Time segment 2 = phase_seg2 */
+	.tseg2_max	= 64,
+	.sjw_max	= 16,
+	.brp_min	= 2,
+	.brp_max	= 256,
 	.brp_inc	= 1,
 };
 
@@ -545,32 +545,34 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
 	u32 noniso_arg = 0;
 	u32 time_off;
 
-	if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO) {
+	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
+	    !(priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)) {
+		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
+	} else {
 		noniso_arg = IFI_CANFD_TIME_SET_TIMEB_BOSCH |
 			     IFI_CANFD_TIME_SET_TIMEA_BOSCH |
 			     IFI_CANFD_TIME_SET_PRESC_BOSCH |
 			     IFI_CANFD_TIME_SET_SJW_BOSCH;
 		time_off = IFI_CANFD_TIME_SJW_OFF_BOSCH;
-	} else {
-		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
 	}
 
 	/* Configure bit timing */
-	brp = bt->brp - 1;
+	brp = bt->brp - 2;
 	sjw = bt->sjw - 1;
 	tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
-	tseg2 = bt->phase_seg2 - 1;
+	tseg2 = bt->phase_seg2 - 2;
 	writel((tseg2 << IFI_CANFD_TIME_TIMEB_OFF) |
 	       (tseg1 << IFI_CANFD_TIME_TIMEA_OFF) |
 	       (brp << IFI_CANFD_TIME_PRESCALE_OFF) |
-	       (sjw << time_off),
+	       (sjw << time_off) |
+	       noniso_arg,
 	       priv->base + IFI_CANFD_TIME);
 
 	/* Configure data bit timing */
-	brp = dbt->brp - 1;
+	brp = dbt->brp - 2;
 	sjw = dbt->sjw - 1;
 	tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
-	tseg2 = dbt->phase_seg2 - 1;
+	tseg2 = dbt->phase_seg2 - 2;
 	writel((tseg2 << IFI_CANFD_TIME_TIMEB_OFF) |
 	       (tseg1 << IFI_CANFD_TIME_TIMEA_OFF) |
 	       (brp << IFI_CANFD_TIME_PRESCALE_OFF) |
@@ -847,7 +849,7 @@ static int ifi_canfd_plat_probe(struct platform_device *pdev)
 
 	priv->can.state = CAN_STATE_STOPPED;
 
-	priv->can.clock.freq = readl(addr + IFI_CANFD_SYSCLOCK);
+	priv->can.clock.freq = readl(addr + IFI_CANFD_CANCLOCK);
 
 	priv->can.bittiming_const	= &ifi_canfd_bittiming_const;
 	priv->can.data_bittiming_const	= &ifi_canfd_data_bittiming_const;
-- 
2.7.0

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Oliver Hartkopp]

Hi Marek,

On 03/03/2016 08:45 PM, Marek Vasut wrote:

> @@ -545,32 +545,34 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
>  	u32 noniso_arg = 0;
>  	u32 time_off;
>  
> -	if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO) {
> +	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
> +	    !(priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)) {
> +		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
> +	} else {
>  		noniso_arg = IFI_CANFD_TIME_SET_TIMEB_BOSCH |
>  			     IFI_CANFD_TIME_SET_TIMEA_BOSCH |
>  			     IFI_CANFD_TIME_SET_PRESC_BOSCH |
>  			     IFI_CANFD_TIME_SET_SJW_BOSCH;
>  		time_off = IFI_CANFD_TIME_SJW_OFF_BOSCH;
> -	} else {
> -		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
>  	}
>  

This may set time_off to IFI_CANFD_TIME_SJW_OFF_BOSCH in the case of

	!(priv->can.ctrlmode & CAN_CTRLMODE_FD)

(== CAN2.0) right?

I assume this is not intended.

I would suggest to initialize time_off first:

	u32 noniso_arg = 0;
	u32 time_off = IFI_CANFD_TIME_SJW_OFF_ISO;

	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
	    (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)) {
		noniso_arg = IFI_CANFD_TIME_SET_TIMEB_BOSCH |
			     IFI_CANFD_TIME_SET_TIMEA_BOSCH |
			     IFI_CANFD_TIME_SET_PRESC_BOSCH |
			     IFI_CANFD_TIME_SET_SJW_BOSCH;
		time_off = IFI_CANFD_TIME_SJW_OFF_BOSCH;
	}

Is my assumption correct? Or does the manual require other settings?

Regards,
Oliver

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marek Vasut]

On 03/03/2016 09:18 PM, Oliver Hartkopp wrote:
> Hi Marek,

Hi Oliver,

> On 03/03/2016 08:45 PM, Marek Vasut wrote:
> 
>> @@ -545,32 +545,34 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
>>  	u32 noniso_arg = 0;
>>  	u32 time_off;
>>  
>> -	if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO) {
>> +	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
>> +	    !(priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)) {
>> +		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
>> +	} else {
>>  		noniso_arg = IFI_CANFD_TIME_SET_TIMEB_BOSCH |
>>  			     IFI_CANFD_TIME_SET_TIMEA_BOSCH |
>>  			     IFI_CANFD_TIME_SET_PRESC_BOSCH |
>>  			     IFI_CANFD_TIME_SET_SJW_BOSCH;
>>  		time_off = IFI_CANFD_TIME_SJW_OFF_BOSCH;
>> -	} else {
>> -		time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
>>  	}
>>  
> 
> This may set time_off to IFI_CANFD_TIME_SJW_OFF_BOSCH in the case of
> 
> 	!(priv->can.ctrlmode & CAN_CTRLMODE_FD)
> 
> (== CAN2.0) right?
> 
> I assume this is not intended.

My understanding (and tests) indicate that this is correct. For CAN2.0
and CANFD-BOSCH (non-ISO), we use the later part of the condition. Only
for CANFD-ISO we use the former part.

> I would suggest to initialize time_off first:
> 
> 	u32 noniso_arg = 0;
> 	u32 time_off = IFI_CANFD_TIME_SJW_OFF_ISO;
> 
> 	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
> 	    (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)) {
> 		noniso_arg = IFI_CANFD_TIME_SET_TIMEB_BOSCH |
> 			     IFI_CANFD_TIME_SET_TIMEA_BOSCH |
> 			     IFI_CANFD_TIME_SET_PRESC_BOSCH |
> 			     IFI_CANFD_TIME_SET_SJW_BOSCH;
> 		time_off = IFI_CANFD_TIME_SJW_OFF_BOSCH;
> 	}
> 
> Is my assumption correct? Or does the manual require other settings?

See above, I think the code is correct as is.

btw can you pick the remaining patches (2,3,4) if they're fine with you,
so I don't have to repost the whole series ?

-- 
Best regards,
Marek Vasut

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Oliver Hartkopp]

Hi Marek,

On 03/03/2016 09:52 PM, Marek Vasut wrote:

> 
> See above, I think the code is correct as is.
> 
> btw can you pick the remaining patches (2,3,4) if they're fine with you,
> so I don't have to repost the whole series ?
> 

As my remark was obsolete, Marc might apply the whole V4 patch series to the
can-next git tree without any changes.

So let's wait for him then :-)

Best regards,
Oliver

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 762 bytes --]

On 03/03/2016 09:58 PM, Oliver Hartkopp wrote:
> Hi Marek,
> 
> On 03/03/2016 09:52 PM, Marek Vasut wrote:
> 
>>
>> See above, I think the code is correct as is.
>>
>> btw can you pick the remaining patches (2,3,4) if they're fine with you,
>> so I don't have to repost the whole series ?
>>
> 
> As my remark was obsolete, Marc might apply the whole V4 patch series to the
> can-next git tree without any changes.
> 
> So let's wait for him then :-)

Ok. :)

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 872 bytes --]

On 03/03/2016 10:01 PM, Marc Kleine-Budde wrote:
> On 03/03/2016 09:58 PM, Oliver Hartkopp wrote:
>> Hi Marek,
>>
>> On 03/03/2016 09:52 PM, Marek Vasut wrote:
>>
>>>
>>> See above, I think the code is correct as is.
>>>
>>> btw can you pick the remaining patches (2,3,4) if they're fine with you,
>>> so I don't have to repost the whole series ?
>>>
>>
>> As my remark was obsolete, Marc might apply the whole V4 patch series to the
>> can-next git tree without any changes.
>>
>> So let's wait for him then :-)
> 
> Ok. :)

Do you want some Acked-by or Reviewed-by?

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Oliver Hartkopp]

On 03/03/2016 10:02 PM, Marc Kleine-Budde wrote:

> 
> Do you want some Acked-by or Reviewed-by?

Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>

finally seems appropriate :)

Thanks,
Oliver

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 569 bytes --]

On 03/03/2016 10:04 PM, Oliver Hartkopp wrote:
> 
> 
> On 03/03/2016 10:02 PM, Marc Kleine-Budde wrote:
> 
>>
>> Do you want some Acked-by or Reviewed-by?
> 
> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> finally seems appropriate :)

For all four patches?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Oliver Hartkopp]

On 03/03/2016 10:05 PM, Marc Kleine-Budde wrote:

>> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> finally seems appropriate :)
> 
> For all four patches?

yep.

Re: [PATCH V4 1/4] net: can: ifi: Fix clock generator configuration

[Marek Vasut]

On 03/03/2016 10:06 PM, Oliver Hartkopp wrote:
> 
> 
> On 03/03/2016 10:05 PM, Marc Kleine-Budde wrote:
> 
>>> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>>
>>> finally seems appropriate :)
>>
>> For all four patches?
> 
> yep.
> 
Whew, thank you :-)

In case I detect some more defects, I will send subsequent patches. I'm
sure there is considerable amount of real-world testing coming for this
driver.

-- 
Best regards,
Marek Vasut

[PATCH V4 2/4] net: can: ifi: Fix TX DLC configuration

[Marek Vasut]

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 <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
---
V2: - Use can_len2dlc() unconditionally to sanitize DLC in the TX path
    - The CAN_CTRLMODE_CANFD_NON_ISO can only ever be set if the
      CANFD_CTRLMODE_CANFD is set. Use only the later for checking
      if the core transmits in CANFD mode.
V3: Drop u32 txdlc = 0;, make it just u32 txdlc; and init txdlc in place
    with txdlc = can_len2dlc(); instead of doing txdlc |= can_len2dlc();
V4: none
---
 drivers/net/can/ifi_canfd/ifi_canfd.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 86287a3..c905bfa 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -749,8 +749,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
 {
 	struct ifi_canfd_priv *priv = netdev_priv(ndev);
 	struct canfd_frame *cf = (struct canfd_frame *)skb->data;
-	u32 txst, txid;
-	u32 txdlc = 0;
+	u32 txst, txid, txdlc;
 	int i;
 
 	if (can_dropped_invalid_skb(ndev, skb))
@@ -773,12 +772,11 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
 		txid = cf->can_id & CAN_SFF_MASK;
 	}
 
-	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);
+	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;
 	}
 
 	if (cf->can_id & CAN_RTR_FLAG)
-- 
2.7.0

[PATCH V4 3/4] net: can: ifi: Fix RX and TX ID mask

[Marek Vasut]

The RX and TX ID mask for CAN2.0 is 11 bits wide. This patch fixes
the incorrect mask, which caused the CAN IDs to miss the MSBit both
on receive and transmit.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
---
V2: Use CAN_SFF_MASK instead of plain bitfield
    Use CAN_EFF_MASK for the extended frame format
V3: none
V4: none
---
 drivers/net/can/ifi_canfd/ifi_canfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index c905bfa..9f527ca 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -135,8 +135,8 @@
 
 #define IFI_CANFD_RXFIFO_ID			0x6c
 #define IFI_CANFD_RXFIFO_ID_ID_OFFSET		0
-#define IFI_CANFD_RXFIFO_ID_ID_STD_MASK		0x3ff
-#define IFI_CANFD_RXFIFO_ID_ID_XTD_MASK		0x1fffffff
+#define IFI_CANFD_RXFIFO_ID_ID_STD_MASK		CAN_SFF_MASK
+#define IFI_CANFD_RXFIFO_ID_ID_XTD_MASK		CAN_EFF_MASK
 #define IFI_CANFD_RXFIFO_ID_IDE			BIT(29)
 
 #define IFI_CANFD_RXFIFO_DATA			0x70	/* 0x70..0xac */
@@ -156,8 +156,8 @@
 
 #define IFI_CANFD_TXFIFO_ID			0xbc
 #define IFI_CANFD_TXFIFO_ID_ID_OFFSET		0
-#define IFI_CANFD_TXFIFO_ID_ID_STD_MASK		0x3ff
-#define IFI_CANFD_TXFIFO_ID_ID_XTD_MASK		0x1fffffff
+#define IFI_CANFD_TXFIFO_ID_ID_STD_MASK		CAN_SFF_MASK
+#define IFI_CANFD_TXFIFO_ID_ID_XTD_MASK		CAN_EFF_MASK
 #define IFI_CANFD_TXFIFO_ID_IDE			BIT(29)
 
 #define IFI_CANFD_TXFIFO_DATA			0xc0	/* 0xb0..0xfc */
-- 
2.7.0

[PATCH V4 4/4] net: can: ifi: Add obscure bit swap for EFF frame IDs

[Marek Vasut]

In case of CAN2.0 EFF frame, the controller handles frame IDs in a
rather bizzare way. The ID is split into an extended part, IDX[28:11]
and standard part, ID[10:0]. In the TX path, the core first sends the
top 11 bits of the IDX, followed by ID and finally the rest of IDX.
In the RX path, the core stores the ID the LSbit part of IDX field,
followed by the LSbit parts of real IDX. The MSbit parts of IDX are
stored in ID field of the register.

This patch implements the necessary bit shuffling to mitigate this
obscure behavior. In case two of these controllers are connected
together, the RX and TX bit swapping nullifies itself and the issue
does not manifest. The issue only manifests when talking to another
different CAN controller.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
---
V2: Rebase on top of patch 3/4, no change to the patch
V3: none
V4: none
---
 drivers/net/can/ifi_canfd/ifi_canfd.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 9f527ca..b36374d 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -136,7 +136,11 @@
 #define IFI_CANFD_RXFIFO_ID			0x6c
 #define IFI_CANFD_RXFIFO_ID_ID_OFFSET		0
 #define IFI_CANFD_RXFIFO_ID_ID_STD_MASK		CAN_SFF_MASK
+#define IFI_CANFD_RXFIFO_ID_ID_STD_OFFSET	0
+#define IFI_CANFD_RXFIFO_ID_ID_STD_WIDTH	10
 #define IFI_CANFD_RXFIFO_ID_ID_XTD_MASK		CAN_EFF_MASK
+#define IFI_CANFD_RXFIFO_ID_ID_XTD_OFFSET	11
+#define IFI_CANFD_RXFIFO_ID_ID_XTD_WIDTH	18
 #define IFI_CANFD_RXFIFO_ID_IDE			BIT(29)
 
 #define IFI_CANFD_RXFIFO_DATA			0x70	/* 0x70..0xac */
@@ -157,7 +161,11 @@
 #define IFI_CANFD_TXFIFO_ID			0xbc
 #define IFI_CANFD_TXFIFO_ID_ID_OFFSET		0
 #define IFI_CANFD_TXFIFO_ID_ID_STD_MASK		CAN_SFF_MASK
+#define IFI_CANFD_TXFIFO_ID_ID_STD_OFFSET	0
+#define IFI_CANFD_TXFIFO_ID_ID_STD_WIDTH	10
 #define IFI_CANFD_TXFIFO_ID_ID_XTD_MASK		CAN_EFF_MASK
+#define IFI_CANFD_TXFIFO_ID_ID_XTD_OFFSET	11
+#define IFI_CANFD_TXFIFO_ID_ID_XTD_WIDTH	18
 #define IFI_CANFD_TXFIFO_ID_IDE			BIT(29)
 
 #define IFI_CANFD_TXFIFO_DATA			0xc0	/* 0xb0..0xfc */
@@ -229,10 +237,20 @@ static void ifi_canfd_read_fifo(struct net_device *ndev)
 
 	rxid = readl(priv->base + IFI_CANFD_RXFIFO_ID);
 	id = (rxid >> IFI_CANFD_RXFIFO_ID_ID_OFFSET);
-	if (id & IFI_CANFD_RXFIFO_ID_IDE)
+	if (id & IFI_CANFD_RXFIFO_ID_IDE) {
 		id &= IFI_CANFD_RXFIFO_ID_ID_XTD_MASK;
-	else
+		/*
+		 * In case the Extended ID frame is received, the standard
+		 * and extended part of the ID are swapped in the register,
+		 * so swap them back to obtain the correct ID.
+		 */
+		id = (id >> IFI_CANFD_RXFIFO_ID_ID_XTD_OFFSET) |
+		     ((id & IFI_CANFD_RXFIFO_ID_ID_STD_MASK) <<
+		       IFI_CANFD_RXFIFO_ID_ID_XTD_WIDTH);
+		id |= CAN_EFF_FLAG;
+	} else {
 		id &= IFI_CANFD_RXFIFO_ID_ID_STD_MASK;
+	}
 	cf->can_id = id;
 
 	if (rxdlc & IFI_CANFD_RXFIFO_DLC_ESI) {
@@ -767,6 +785,15 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
 
 	if (cf->can_id & CAN_EFF_FLAG) {
 		txid = cf->can_id & CAN_EFF_MASK;
+		/*
+		 * In case the Extended ID frame is transmitted, the
+		 * standard and extended part of the ID are swapped
+		 * in the register, so swap them back to send the
+		 * correct ID.
+		 */
+		txid = (txid >> IFI_CANFD_TXFIFO_ID_ID_XTD_WIDTH) |
+		       ((txid & IFI_CANFD_TXFIFO_ID_ID_XTD_MASK) <<
+		         IFI_CANFD_TXFIFO_ID_ID_XTD_OFFSET);
 		txid |= IFI_CANFD_TXFIFO_ID_IDE;
 	} else {
 		txid = cf->can_id & CAN_SFF_MASK;
-- 
2.7.0