[PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

Hi Marc,

This patch set intends to add support for NXP Flexcan CAN FD, it has
been validated on three NXP platform(i.MX8QM/QXP, S32V234, LX2160AR1).
After discussed with another two Fexcan owner, we sorted out this
version.

I hope you can pick up the patch set as it can fully meet requirement of
above three platform. And after that, we can start to do upstream about
CAN FD.

Thanks a lot!

BRs,
Joakim Zhang

Joakim Zhang (8):
  can: flexcan: allocate skb in flexcan_mailbox_read
  can: flexcan: use struct canfd_frame for CAN classic frame
  can: flexcan: add CAN FD mode support
  can: flexcan: add CANFD BRS support
  can: flexcan: add ISO CAN FD feature support
  can: flexcan: add Transceiver Delay Compensation suopport
  can: flexcan: add imx8qm support
  can: flexcan: add lx2160ar1 support

 drivers/net/can/flexcan.c      | 340 ++++++++++++++++++++++++++++-----
 drivers/net/can/rx-offload.c   |  33 +---
 include/linux/can/rx-offload.h |   5 +-
 3 files changed, 305 insertions(+), 73 deletions(-)

-- 
2.17.1

[PATCH 1/8] can: flexcan: allocate skb in flexcan_mailbox_read

[Joakim Zhang]

We need to use alloc_canfd_skb() for CAN FD frames and alloc_can_skb()
for CAN classic frames. So we have to alloc skb in flexcan_mailbox_read().

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c      | 38 ++++++++++++++++++++--------------
 drivers/net/can/rx-offload.c   | 29 +++++++-------------------
 include/linux/can/rx-offload.h |  5 +++--
 3 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e35083ff31ee..7e12f3db0915 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -789,14 +789,15 @@ static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *off
 	return container_of(offload, struct flexcan_priv, offload);
 }
 
-static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
-					 struct can_frame *cf,
-					 u32 *timestamp, unsigned int n)
+static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool drop,
+					 struct sk_buff **skb, u32 *timestamp,
+					 unsigned int n)
 {
 	struct flexcan_priv *priv = rx_offload_to_priv(offload);
 	struct flexcan_regs __iomem *regs = priv->regs;
 	struct flexcan_mb __iomem *mb;
 	u32 reg_ctrl, reg_id, reg_iflag1;
+	struct can_frame *cf = NULL;
 	int i;
 
 	mb = flexcan_get_mb(priv, n);
@@ -827,22 +828,27 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
 		reg_ctrl = priv->read(&mb->can_ctrl);
 	}
 
-	/* increase timstamp to full 32 bit */
-	*timestamp = reg_ctrl << 16;
+	if (!drop)
+		*skb = alloc_can_skb(offload->dev, &cf);
 
-	reg_id = priv->read(&mb->can_id);
-	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
-		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
-	else
-		cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
+	if (*skb && cf) {
+		/* increase timstamp to full 32 bit */
+		*timestamp = reg_ctrl << 16;
 
-	if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
-		cf->can_id |= CAN_RTR_FLAG;
-	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
+		reg_id = priv->read(&mb->can_id);
+		if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
+			cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
+		else
+			cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
 
-	for (i = 0; i < cf->can_dlc; i += sizeof(u32)) {
-		__be32 data = cpu_to_be32(priv->read(&mb->data[i / sizeof(u32)]));
-		*(__be32 *)(cf->data + i) = data;
+		if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
+			cf->can_id |= CAN_RTR_FLAG;
+		cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
+
+		for (i = 0; i < cf->can_dlc; i += sizeof(u32)) {
+			__be32 data = cpu_to_be32(priv->read(&mb->data[i / sizeof(u32)]));
+			*(__be32 *)(cf->data + i) = data;
+		}
 	}
 
 	/* mark as read */
diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index 2ce4fa8698c7..632919484ff7 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -121,32 +121,19 @@ static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
 static struct sk_buff *can_rx_offload_offload_one(struct can_rx_offload *offload, unsigned int n)
 {
 	struct sk_buff *skb = NULL;
-	struct can_rx_offload_cb *cb;
-	struct can_frame *cf;
-	int ret;
+	u32 timestamp;
 
 	/* If queue is full or skb not available, read to discard mailbox */
-	if (likely(skb_queue_len(&offload->skb_queue) <=
-		   offload->skb_queue_len_max))
-		skb = alloc_can_skb(offload->dev, &cf);
+	bool drop = unlikely(skb_queue_len(&offload->skb_queue) >
+			     offload->skb_queue_len_max);
 
-	if (!skb) {
-		struct can_frame cf_overflow;
-		u32 timestamp;
+	if (offload->mailbox_read(offload, drop, &skb, &timestamp, n) && !skb)
+		offload->dev->stats.rx_dropped++;
 
-		ret = offload->mailbox_read(offload, &cf_overflow,
-					    &timestamp, n);
-		if (ret)
-			offload->dev->stats.rx_dropped++;
+	if (skb) {
+		struct can_rx_offload_cb *cb = can_rx_offload_get_cb(skb);
 
-		return NULL;
-	}
-
-	cb = can_rx_offload_get_cb(skb);
-	ret = offload->mailbox_read(offload, cf, &cb->timestamp, n);
-	if (!ret) {
-		kfree_skb(skb);
-		return NULL;
+		cb->timestamp = timestamp;
 	}
 
 	return skb;
diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
index 8268811a697e..c54d80ef4314 100644
--- a/include/linux/can/rx-offload.h
+++ b/include/linux/can/rx-offload.h
@@ -23,8 +23,9 @@
 struct can_rx_offload {
 	struct net_device *dev;
 
-	unsigned int (*mailbox_read)(struct can_rx_offload *offload, struct can_frame *cf,
-				     u32 *timestamp, unsigned int mb);
+	unsigned int (*mailbox_read)(struct can_rx_offload *offload, bool drop,
+				     struct sk_buff **skb, u32 *timestamp,
+				     unsigned int mb);
 
 	struct sk_buff_head skb_queue;
 	u32 skb_queue_len_max;
-- 
2.17.1

[PATCH 2/8] can: flexcan: use struct canfd_frame for CAN classic frame

[Joakim Zhang]

This patch prepares for CAN FD mode, using struct canfd_frame can both
for classic format frame and fd format frame.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c    | 34 +++++++++++++++++-----------------
 drivers/net/can/rx-offload.c |  4 ++--
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 7e12f3db0915..5b0a159daa38 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -609,10 +609,10 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
-	struct can_frame *cf = (struct can_frame *)skb->data;
+	struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
 	u32 can_id;
 	u32 data;
-	u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cf->can_dlc << 16);
+	u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cfd->len << 16);
 	int i;
 
 	if (can_dropped_invalid_skb(dev, skb))
@@ -620,18 +620,18 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 
 	netif_stop_queue(dev);
 
-	if (cf->can_id & CAN_EFF_FLAG) {
-		can_id = cf->can_id & CAN_EFF_MASK;
+	if (cfd->can_id & CAN_EFF_FLAG) {
+		can_id = cfd->can_id & CAN_EFF_MASK;
 		ctrl |= FLEXCAN_MB_CNT_IDE | FLEXCAN_MB_CNT_SRR;
 	} else {
-		can_id = (cf->can_id & CAN_SFF_MASK) << 18;
+		can_id = (cfd->can_id & CAN_SFF_MASK) << 18;
 	}
 
-	if (cf->can_id & CAN_RTR_FLAG)
+	if (cfd->can_id & CAN_RTR_FLAG)
 		ctrl |= FLEXCAN_MB_CNT_RTR;
 
-	for (i = 0; i < cf->can_dlc; i += sizeof(u32)) {
-		data = be32_to_cpup((__be32 *)&cf->data[i]);
+	for (i = 0; i < cfd->len; i += sizeof(u32)) {
+		data = be32_to_cpup((__be32 *)&cfd->data[i]);
 		priv->write(data, &priv->tx_mb->data[i / sizeof(u32)]);
 	}
 
@@ -797,7 +797,7 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool dr
 	struct flexcan_regs __iomem *regs = priv->regs;
 	struct flexcan_mb __iomem *mb;
 	u32 reg_ctrl, reg_id, reg_iflag1;
-	struct can_frame *cf = NULL;
+	struct canfd_frame *cfd = NULL;
 	int i;
 
 	mb = flexcan_get_mb(priv, n);
@@ -829,25 +829,25 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool dr
 	}
 
 	if (!drop)
-		*skb = alloc_can_skb(offload->dev, &cf);
+		*skb = alloc_can_skb(offload->dev, (struct can_frame **)&cfd);
 
-	if (*skb && cf) {
+	if (*skb && cfd) {
 		/* increase timstamp to full 32 bit */
 		*timestamp = reg_ctrl << 16;
 
 		reg_id = priv->read(&mb->can_id);
 		if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
-			cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
+			cfd->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
 		else
-			cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
+			cfd->can_id = (reg_id >> 18) & CAN_SFF_MASK;
 
 		if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
-			cf->can_id |= CAN_RTR_FLAG;
-		cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
+			cfd->can_id |= CAN_RTR_FLAG;
+		cfd->len = get_can_dlc((reg_ctrl >> 16) & 0x0F);
 
-		for (i = 0; i < cf->can_dlc; i += sizeof(u32)) {
+		for (i = 0; i < cfd->len; i += sizeof(u32)) {
 			__be32 data = cpu_to_be32(priv->read(&mb->data[i / sizeof(u32)]));
-			*(__be32 *)(cf->data + i) = data;
+			*(__be32 *)(cfd->data + i) = data;
 		}
 	}
 
diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index 632919484ff7..9f8c8410e19e 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -55,11 +55,11 @@ static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
 
 	while ((work_done < quota) &&
 	       (skb = skb_dequeue(&offload->skb_queue))) {
-		struct can_frame *cf = (struct can_frame *)skb->data;
+		struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
 
 		work_done++;
 		stats->rx_packets++;
-		stats->rx_bytes += cf->can_dlc;
+		stats->rx_bytes += cfd->len;
 		netif_receive_skb(skb);
 	}
 
-- 
2.17.1

[PATCH 3/8] can: flexcan: add CAN FD mode support

[Joakim Zhang]

This patch intends to add CAN FD mode support in driver, it means that
payload size can extend up to 64 bytes.

Bit timing always set in CBT register other than CTRL1 register when CANFD
supports BRS, it will extend the range of all CAN bit timing variables
(PRESDIV, PROPSEG, PSEG1, PSEG2 and RJW), which will improve the bit
timing accuracy.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 255 +++++++++++++++++++++++++++++++++-----
 1 file changed, 225 insertions(+), 30 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 5b0a159daa38..23e9407e33ff 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -52,6 +52,7 @@
 #define FLEXCAN_MCR_IRMQ		BIT(16)
 #define FLEXCAN_MCR_LPRIO_EN		BIT(13)
 #define FLEXCAN_MCR_AEN			BIT(12)
+#define FLEXCAN_MCR_FDEN		BIT(11)
 /* MCR_MAXMB: maximum used MBs is MAXMB + 1 */
 #define FLEXCAN_MCR_MAXMB(x)		((x) & 0x7f)
 #define FLEXCAN_MCR_IDAM_A		(0x0 << 8)
@@ -137,6 +138,26 @@
 	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
 	 FLEXCAN_ESR_WAK_INT)
 
+/* FLEXCAN Bit Timing register (CBT) bits */
+#define FLEXCAN_CBT_BTF		        BIT(31)
+#define FLEXCAN_CBT_EPRESDIV(x)	        (((x) & 0x3ff) << 21)
+#define FLEXCAN_CBT_ERJW(x)		(((x) & 0x1f) << 16)
+#define FLEXCAN_CBT_EPROPSEG(x)	        (((x) & 0x3f) << 10)
+#define FLEXCAN_CBT_EPSEG1(x)		(((x) & 0x1f) << 5)
+#define FLEXCAN_CBT_EPSEG2(x)		((x) & 0x1f)
+
+/* FLEXCAN FD control register (FDCTRL) bits */
+#define FLEXCAN_FDCTRL_FDRATE		BIT(31)
+#define FLEXCAN_FDCTRL_MBDSR1(x)	(((x) & 0x3) << 19)
+#define FLEXCAN_FDCTRL_MBDSR0(x)	(((x) & 0x3) << 16)
+
+/* FLEXCAN FD Bit Timing register (FDCBT) bits */
+#define FLEXCAN_FDCBT_FPRESDIV(x)	(((x) & 0x3ff) << 20)
+#define FLEXCAN_FDCBT_FRJW(x)		(((x) & 0x07) << 16)
+#define FLEXCAN_FDCBT_FPROPSEG(x)	(((x) & 0x1f) << 10)
+#define FLEXCAN_FDCBT_FPSEG1(x)		(((x) & 0x07) << 5)
+#define FLEXCAN_FDCBT_FPSEG2(x)		((x) & 0x07)
+
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 /* Errata ERR005829 step7: Reserve first valid MB */
 #define FLEXCAN_TX_MB_RESERVED_OFF_FIFO		8
@@ -148,6 +169,10 @@
 #define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE	BIT(5)
 
 /* FLEXCAN message buffers */
+#define FLEXCAN_MB_CNT_EDL		BIT(31)
+#define FLEXCAN_MB_CNT_BRS		BIT(30)
+#define FLEXCAN_MB_CNT_ESI		BIT(29)
+
 #define FLEXCAN_MB_CODE_MASK		(0xf << 24)
 #define FLEXCAN_MB_CODE_RX_BUSY_BIT	(0x1 << 24)
 #define FLEXCAN_MB_CODE_RX_INACTIVE	(0x0 << 24)
@@ -192,6 +217,7 @@
 #define FLEXCAN_QUIRK_BROKEN_PERR_STATE	BIT(6) /* No interrupt for error passive */
 #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN	BIT(7) /* default to BE register access */
 #define FLEXCAN_QUIRK_SETUP_STOP_MODE		BIT(8) /* Setup stop mode to support wakeup */
+#define FLEXCAN_QUIRK_TIMESTAMP_SUPPORT_FD	BIT(9) /* Use timestamp then support can fd mode */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -225,7 +251,8 @@ struct flexcan_regs {
 	u32 crcr;		/* 0x44 */
 	u32 rxfgmask;		/* 0x48 */
 	u32 rxfir;		/* 0x4c */
-	u32 _reserved3[12];	/* 0x50 */
+	u32 cbt;                /* 0x50 */
+	u32 _reserved3[11];     /* 0x54 */
 	u8 mb[2][512];		/* 0x80 */
 	/* FIFO-mode:
 	 *			MB
@@ -250,6 +277,9 @@ struct flexcan_regs {
 	u32 rerrdr;		/* 0xaf4 */
 	u32 rerrsynr;		/* 0xaf8 */
 	u32 errsr;		/* 0xafc */
+	u32 _reserved7[64];     /* 0xb00 */
+	u32 fdctrl;             /* 0xc00 */
+	u32 fdcbt;              /* 0xc04 */
 };
 
 struct flexcan_devtype_data {
@@ -337,6 +367,30 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 	.brp_inc = 1,
 };
 
+static const struct can_bittiming_const flexcan_fd_bittiming_const = {
+	.name = DRV_NAME,
+	.tseg1_min = 2,
+	.tseg1_max = 96,
+	.tseg2_min = 2,
+	.tseg2_max = 32,
+	.sjw_max = 16,
+	.brp_min = 1,
+	.brp_max = 1024,
+	.brp_inc = 1,
+};
+
+static const struct can_bittiming_const flexcan_fd_data_bittiming_const = {
+	.name = DRV_NAME,
+	.tseg1_min = 2,
+	.tseg1_max = 39,
+	.tseg2_min = 2,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 1024,
+	.brp_inc = 1,
+};
+
 /* FlexCAN module is essentially modelled as a little-endian IP in most
  * SoCs, i.e the registers as well as the message buffer areas are
  * implemented in a little-endian fashion.
@@ -612,7 +666,7 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 	struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
 	u32 can_id;
 	u32 data;
-	u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cfd->len << 16);
+	u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | ((can_len2dlc(cfd->len)) << 16);
 	int i;
 
 	if (can_dropped_invalid_skb(dev, skb))
@@ -630,6 +684,9 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 	if (cfd->can_id & CAN_RTR_FLAG)
 		ctrl |= FLEXCAN_MB_CNT_RTR;
 
+	if (can_is_canfd_skb(skb))
+		ctrl |= FLEXCAN_MB_CNT_EDL;
+
 	for (i = 0; i < cfd->len; i += sizeof(u32)) {
 		data = be32_to_cpup((__be32 *)&cfd->data[i]);
 		priv->write(data, &priv->tx_mb->data[i / sizeof(u32)]);
@@ -828,8 +885,14 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool dr
 		reg_ctrl = priv->read(&mb->can_ctrl);
 	}
 
-	if (!drop)
-		*skb = alloc_can_skb(offload->dev, (struct can_frame **)&cfd);
+
+	if (!drop) {
+		if (reg_ctrl & FLEXCAN_MB_CNT_EDL)
+			*skb = alloc_canfd_skb(offload->dev, &cfd);
+		else
+			*skb = alloc_can_skb(offload->dev,
+					     (struct can_frame **)&cfd);
+	}
 
 	if (*skb && cfd) {
 		/* increase timstamp to full 32 bit */
@@ -841,9 +904,20 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool dr
 		else
 			cfd->can_id = (reg_id >> 18) & CAN_SFF_MASK;
 
-		if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
-			cfd->can_id |= CAN_RTR_FLAG;
-		cfd->len = get_can_dlc((reg_ctrl >> 16) & 0x0F);
+
+		if (reg_ctrl & FLEXCAN_MB_CNT_EDL) {
+			cfd->len = can_dlc2len(get_canfd_dlc((reg_ctrl >> 16) & 0x0F));
+		} else {
+			cfd->len = get_can_dlc((reg_ctrl >> 16) & 0x0F);
+
+			if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
+				cfd->can_id |= CAN_RTR_FLAG;
+		}
+
+		if (reg_ctrl & FLEXCAN_MB_CNT_ESI) {
+			cfd->flags |= CANFD_ESI;
+			netdev_warn(priv->can.dev, "ESI Error\n");
+		}
 
 		for (i = 0; i < cfd->len; i += sizeof(u32)) {
 			__be32 data = cpu_to_be32(priv->read(&mb->data[i / sizeof(u32)]));
@@ -989,27 +1063,14 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 static void flexcan_set_bittiming(struct net_device *dev)
 {
-	const struct flexcan_priv *priv = netdev_priv(dev);
-	const struct can_bittiming *bt = &priv->can.bittiming;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	struct can_bittiming *dbt = &priv->can.data_bittiming;
 	struct flexcan_regs __iomem *regs = priv->regs;
-	u32 reg;
+	u32 reg, reg_cbt, reg_fdcbt;
 
 	reg = priv->read(&regs->ctrl);
-	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
-		 FLEXCAN_CTRL_RJW(0x3) |
-		 FLEXCAN_CTRL_PSEG1(0x7) |
-		 FLEXCAN_CTRL_PSEG2(0x7) |
-		 FLEXCAN_CTRL_PROPSEG(0x7) |
-		 FLEXCAN_CTRL_LPB |
-		 FLEXCAN_CTRL_SMP |
-		 FLEXCAN_CTRL_LOM);
-
-	reg |= FLEXCAN_CTRL_PRESDIV(bt->brp - 1) |
-		FLEXCAN_CTRL_PSEG1(bt->phase_seg1 - 1) |
-		FLEXCAN_CTRL_PSEG2(bt->phase_seg2 - 1) |
-		FLEXCAN_CTRL_RJW(bt->sjw - 1) |
-		FLEXCAN_CTRL_PROPSEG(bt->prop_seg - 1);
-
+	reg &= ~(FLEXCAN_CTRL_LPB | FLEXCAN_CTRL_SMP | FLEXCAN_CTRL_LOM);
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
 		reg |= FLEXCAN_CTRL_LPB;
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
@@ -1020,9 +1081,102 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	netdev_dbg(dev, "writing ctrl=0x%08x\n", reg);
 	priv->write(reg, &regs->ctrl);
 
-	/* print chip status */
-	netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		   priv->read(&regs->mcr), priv->read(&regs->ctrl));
+	if (priv->can.ctrlmode_supported & CAN_CTRLMODE_FD) {
+		reg_cbt = priv->read(&regs->cbt);
+		reg_cbt &= ~(FLEXCAN_CBT_EPRESDIV(0x3ff) |
+			     FLEXCAN_CBT_EPSEG1(0x1f) |
+			     FLEXCAN_CBT_EPSEG2(0x1f) |
+			     FLEXCAN_CBT_ERJW(0x1f) |
+			     FLEXCAN_CBT_EPROPSEG(0x3f) |
+			     FLEXCAN_CBT_BTF);
+
+		/* CBT[EPSEG1] is 5 bit long and CBT[EPROPSEG] is 6 bit long.
+		 * The can_calc_bittiming tries to divide the tseg1 equally
+		 * between phase_seg1 and prop_seg, which may not fit in CBT
+		 * register. Therefore, if phase_seg1 is more than possible
+		 * value, increase prop_seg and decrease phase_seg1
+		 */
+		if (bt->phase_seg1 > 0x20) {
+			bt->prop_seg += (bt->phase_seg1 - 0x20);
+			bt->phase_seg1 = 0x20;
+		}
+
+		reg_cbt = FLEXCAN_CBT_EPRESDIV(bt->brp - 1) |
+				FLEXCAN_CBT_EPSEG1(bt->phase_seg1 - 1) |
+				FLEXCAN_CBT_EPSEG2(bt->phase_seg2 - 1) |
+				FLEXCAN_CBT_ERJW(bt->sjw - 1) |
+				FLEXCAN_CBT_EPROPSEG(bt->prop_seg - 1) |
+				FLEXCAN_CBT_BTF;
+		priv->write(reg_cbt, &regs->cbt);
+
+		netdev_dbg(dev, "bt: prediv %d seg1 %d seg2 %d rjw %d propseg %d\n",
+			   bt->brp - 1, bt->phase_seg1 - 1, bt->phase_seg2 - 1,
+			   bt->sjw - 1, bt->prop_seg - 1);
+
+		if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+			reg_fdcbt = priv->read(&regs->fdcbt);
+			reg_fdcbt &= ~(FLEXCAN_FDCBT_FPRESDIV(0x3ff) |
+				       FLEXCAN_FDCBT_FPSEG1(0x07) |
+				       FLEXCAN_FDCBT_FPSEG2(0x07) |
+				       FLEXCAN_FDCBT_FRJW(0x07) |
+				       FLEXCAN_FDCBT_FPROPSEG(0x1f));
+
+			/* FDCBT[FPSEG1] is 3 bit long and FDCBT[FPROPSEG] is 5 bit long.
+			 * The can_calc_bittiming tries to divide the tseg1 equally
+			 * between phase_seg1 and prop_seg, which may not fit in FDCBT
+			 * register. Therefore, if phase_seg1 is more than possible
+			 * value, increase prop_seg and decrease phase_seg1
+			 */
+			if (dbt->phase_seg1 > 0x8) {
+				dbt->prop_seg += (dbt->phase_seg1 - 0x8);
+				dbt->phase_seg1 = 0x8;
+			}
+
+			reg_fdcbt = FLEXCAN_FDCBT_FPRESDIV(dbt->brp - 1) |
+					FLEXCAN_FDCBT_FPSEG1(dbt->phase_seg1 - 1) |
+					FLEXCAN_FDCBT_FPSEG2(dbt->phase_seg2 - 1) |
+					FLEXCAN_FDCBT_FRJW(dbt->sjw - 1) |
+					FLEXCAN_FDCBT_FPROPSEG(dbt->prop_seg);
+			priv->write(reg_fdcbt, &regs->fdcbt);
+
+			if (bt->brp != dbt->brp)
+				netdev_warn(dev, "Warning!! data brp = %d and brp = %d don't match.\n"
+					    "flexcan may not work. consider using different bitrate or data bitrate\n",
+					    dbt->brp, bt->brp);
+
+			netdev_dbg(dev, "fdbt: prediv %d seg1 %d seg2 %d rjw %d propseg %d\n",
+				   dbt->brp - 1, dbt->phase_seg1 - 1, dbt->phase_seg2 - 1,
+				   dbt->sjw - 1, dbt->prop_seg);
+
+			netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x cbt=0x%08x fdcbt=0x%08x\n",
+				   __func__, priv->read(&regs->mcr),
+				   priv->read(&regs->ctrl),
+				   priv->read(&regs->cbt),
+				   priv->read(&regs->fdcbt));
+		}
+	} else {
+		reg = priv->read(&regs->ctrl);
+		reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
+			 FLEXCAN_CTRL_RJW(0x3) |
+			 FLEXCAN_CTRL_PSEG1(0x7) |
+			 FLEXCAN_CTRL_PSEG2(0x7) |
+			 FLEXCAN_CTRL_PROPSEG(0x7));
+
+		reg |= FLEXCAN_CTRL_PRESDIV(bt->brp - 1) |
+			FLEXCAN_CTRL_PSEG1(bt->phase_seg1 - 1) |
+			FLEXCAN_CTRL_PSEG2(bt->phase_seg2 - 1) |
+			FLEXCAN_CTRL_RJW(bt->sjw - 1) |
+			FLEXCAN_CTRL_PROPSEG(bt->prop_seg - 1);
+		priv->write(reg, &regs->ctrl);
+
+		netdev_dbg(dev, "bt: prediv %d seg1 %d seg2 %d rjw %d propseg %d\n",
+			   bt->brp - 1, bt->phase_seg1 - 1, bt->phase_seg2 - 1,
+			   bt->sjw - 1, bt->prop_seg - 1);
+
+		/* print chip status */
+		netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
+			   priv->read(&regs->mcr), priv->read(&regs->ctrl));
+	}
 }
 
 /* flexcan_chip_start
@@ -1034,7 +1188,7 @@ static int flexcan_chip_start(struct net_device *dev)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->regs;
-	u32 reg_mcr, reg_ctrl, reg_ctrl2, reg_mecr;
+	u32 reg_mcr, reg_ctrl, reg_ctrl2, reg_mecr, reg_fdctrl;
 	u64 reg_imask;
 	int err, i;
 	struct flexcan_mb __iomem *mb;
@@ -1131,6 +1285,26 @@ static int flexcan_chip_start(struct net_device *dev)
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
 	priv->write(reg_ctrl, &regs->ctrl);
 
+	/* FDCTRL */
+	if (priv->can.ctrlmode_supported & CAN_CTRLMODE_FD) {
+		reg_fdctrl = priv->read(&regs->fdctrl) & ~FLEXCAN_FDCTRL_FDRATE;
+		reg_fdctrl &= ~(FLEXCAN_FDCTRL_MBDSR1(0x3) | FLEXCAN_FDCTRL_MBDSR0(0x3));
+		reg_mcr = priv->read(&regs->mcr) & ~FLEXCAN_MCR_FDEN;
+
+		/* support BRS when set CAN FD mode
+		 * 64 bytes payload per MB and 7 MBs per RAM block by default
+		 * enable CAN FD mode
+		 */
+		if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+			reg_fdctrl |= FLEXCAN_FDCTRL_FDRATE;
+			reg_fdctrl |= FLEXCAN_FDCTRL_MBDSR1(0x3) | FLEXCAN_FDCTRL_MBDSR0(0x3);
+			reg_mcr |= FLEXCAN_MCR_FDEN;
+		}
+
+		priv->write(reg_fdctrl, &regs->fdctrl);
+		priv->write(reg_mcr, &regs->mcr);
+	}
+
 	if ((priv->devtype_data->quirks & FLEXCAN_QUIRK_ENABLE_EACEN_RRS)) {
 		reg_ctrl2 = priv->read(&regs->ctrl2);
 		reg_ctrl2 |= FLEXCAN_CTRL2_EACEN | FLEXCAN_CTRL2_RRS;
@@ -1255,6 +1429,12 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
+	if ((priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) &&
+	    (priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
+		netdev_err(dev, "three samples mode and fd mode can't be used together\n");
+		return -EINVAL;
+	}
+
 	err = pm_runtime_get_sync(priv->dev);
 	if (err < 0)
 		return err;
@@ -1267,7 +1447,10 @@ static int flexcan_open(struct net_device *dev)
 	if (err)
 		goto out_close;
 
-	priv->mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
+	if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
+		priv->mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
+	else
+		priv->mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
 	priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
 			 (sizeof(priv->regs->mb[1]) / priv->mb_size);
 
@@ -1607,6 +1790,18 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_TIMESTAMP_SUPPORT_FD) {
+		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+			priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
+			priv->can.bittiming_const = &flexcan_fd_bittiming_const;
+			priv->can.data_bittiming_const = &flexcan_fd_data_bittiming_const;
+		} else {
+			dev_err(&pdev->dev, "can fd mode can't work on fifo mode\n");
+			err = -EINVAL;
+			goto failed_register;
+		}
+	}
+
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-- 
2.17.1

[PATCH 4/8] can: flexcan: add CANFD BRS support

[Joakim Zhang]

This patch intends to add CAN FD BitRate Switch(BRS) support in driver.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 23e9407e33ff..4956ef64944a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -684,9 +684,13 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 	if (cfd->can_id & CAN_RTR_FLAG)
 		ctrl |= FLEXCAN_MB_CNT_RTR;
 
-	if (can_is_canfd_skb(skb))
+	if (can_is_canfd_skb(skb)) {
 		ctrl |= FLEXCAN_MB_CNT_EDL;
 
+		if (cfd->flags & CANFD_BRS)
+			ctrl |= FLEXCAN_MB_CNT_BRS;
+	}
+
 	for (i = 0; i < cfd->len; i += sizeof(u32)) {
 		data = be32_to_cpup((__be32 *)&cfd->data[i]);
 		priv->write(data, &priv->tx_mb->data[i / sizeof(u32)]);
@@ -907,6 +911,9 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload, bool dr
 
 		if (reg_ctrl & FLEXCAN_MB_CNT_EDL) {
 			cfd->len = can_dlc2len(get_canfd_dlc((reg_ctrl >> 16) & 0x0F));
+
+			if (reg_ctrl & FLEXCAN_MB_CNT_BRS)
+				cfd->flags |= CANFD_BRS;
 		} else {
 			cfd->len = get_can_dlc((reg_ctrl >> 16) & 0x0F);
 
-- 
2.17.1

[PATCH 5/8] can: flexcan: add ISO CAN FD feature support

[Joakim Zhang]

ISO CAN FD is introduced to increase the failture detection capability
than non-ISO CAN FD. The non-ISO CAN FD is still supported by FlexCAN so
that it can be used mainly during an intermediate phase, for evaluation
and development purposes.

Therefore, it is strongly recommended to configure FlexCAN to the ISO
CAN FD protocol by setting the ISOCANFDEN field in the CTRL2 register.

NOTE: If you only set "fd on", driver will use ISO FD mode by default.
You should set "fd-non-iso on" after setting "fd on" if you want to use
NON ISO FD mode.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 4956ef64944a..daf4f0e88224 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -92,6 +92,7 @@
 #define FLEXCAN_CTRL2_MRP		BIT(18)
 #define FLEXCAN_CTRL2_RRS		BIT(17)
 #define FLEXCAN_CTRL2_EACEN		BIT(16)
+#define FLEXCAN_CTRL2_ISOCANFDEN	BIT(12)
 
 /* FLEXCAN memory error control register (MECR) bits */
 #define FLEXCAN_MECR_ECRWRDIS		BIT(31)
@@ -1297,6 +1298,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		reg_fdctrl = priv->read(&regs->fdctrl) & ~FLEXCAN_FDCTRL_FDRATE;
 		reg_fdctrl &= ~(FLEXCAN_FDCTRL_MBDSR1(0x3) | FLEXCAN_FDCTRL_MBDSR0(0x3));
 		reg_mcr = priv->read(&regs->mcr) & ~FLEXCAN_MCR_FDEN;
+		reg_ctrl2 = priv->read(&regs->ctrl2) & ~FLEXCAN_CTRL2_ISOCANFDEN;
 
 		/* support BRS when set CAN FD mode
 		 * 64 bytes payload per MB and 7 MBs per RAM block by default
@@ -1306,10 +1308,14 @@ static int flexcan_chip_start(struct net_device *dev)
 			reg_fdctrl |= FLEXCAN_FDCTRL_FDRATE;
 			reg_fdctrl |= FLEXCAN_FDCTRL_MBDSR1(0x3) | FLEXCAN_FDCTRL_MBDSR0(0x3);
 			reg_mcr |= FLEXCAN_MCR_FDEN;
+
+			if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO))
+				reg_ctrl2 |= FLEXCAN_CTRL2_ISOCANFDEN;
 		}
 
 		priv->write(reg_fdctrl, &regs->fdctrl);
 		priv->write(reg_mcr, &regs->mcr);
+		priv->write(reg_ctrl2, &regs->ctrl2);
 	}
 
 	if ((priv->devtype_data->quirks & FLEXCAN_QUIRK_ENABLE_EACEN_RRS)) {
@@ -1799,7 +1805,7 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_TIMESTAMP_SUPPORT_FD) {
 		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
-			priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
+			priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO;
 			priv->can.bittiming_const = &flexcan_fd_bittiming_const;
 			priv->can.data_bittiming_const = &flexcan_fd_data_bittiming_const;
 		} else {
-- 
2.17.1

[PATCH 6/8] can: flexcan: add Transceiver Delay Compensation suopport

[Joakim Zhang]

The CAN FD protocol allows the transmission and reception of data at a higher
bit rate than the nominal rate used in the arbitration phase when the message's
BRS bit is set.

The TDC mechanism is effective only during the data phase of FD frames
having BRS bit set. It has no effect either on non-FD frames, or on FD
frames transmitted at normal bit rate.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index daf4f0e88224..2c48151e431f 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -149,8 +149,10 @@
 
 /* FLEXCAN FD control register (FDCTRL) bits */
 #define FLEXCAN_FDCTRL_FDRATE		BIT(31)
+#define FLEXCAN_FDCTRL_TDCEN		BIT(15)
 #define FLEXCAN_FDCTRL_MBDSR1(x)	(((x) & 0x3) << 19)
 #define FLEXCAN_FDCTRL_MBDSR0(x)	(((x) & 0x3) << 16)
+#define FLEXCAN_FDCTRL_TDCOFF(x)	(((x) & 0x1f) << 8)
 
 /* FLEXCAN FD Bit Timing register (FDCBT) bits */
 #define FLEXCAN_FDCBT_FPRESDIV(x)	(((x) & 0x3ff) << 20)
@@ -1075,7 +1077,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	struct can_bittiming *bt = &priv->can.bittiming;
 	struct can_bittiming *dbt = &priv->can.data_bittiming;
 	struct flexcan_regs __iomem *regs = priv->regs;
-	u32 reg, reg_cbt, reg_fdcbt;
+	u32 reg, reg_cbt, reg_fdcbt, reg_fdctrl;
 
 	reg = priv->read(&regs->ctrl);
 	reg &= ~(FLEXCAN_CTRL_LPB | FLEXCAN_CTRL_SMP | FLEXCAN_CTRL_LOM);
@@ -1147,6 +1149,19 @@ static void flexcan_set_bittiming(struct net_device *dev)
 					FLEXCAN_FDCBT_FPROPSEG(dbt->prop_seg);
 			priv->write(reg_fdcbt, &regs->fdcbt);
 
+			/* enable transceiver delay compensation(TDC) for fd frame.
+			 * TDC must be disabled when Loop Back mode is enabled.
+			 */
+			reg_fdctrl = priv->read(&regs->fdctrl);
+			if (!(reg & FLEXCAN_CTRL_LPB)) {
+				reg_fdctrl |= FLEXCAN_FDCTRL_TDCEN;
+				reg_fdctrl &= ~FLEXCAN_FDCTRL_TDCOFF(0x1f);
+				/* for the TDC to work reliably, the offset has to use optimal settings */
+				reg_fdctrl |= FLEXCAN_FDCTRL_TDCOFF(((dbt->phase_seg1 - 1) + dbt->prop_seg + 2) *
+								    ((dbt->brp -1) + 1));
+			}
+			priv->write(reg_fdctrl, &regs->fdctrl);
+
 			if (bt->brp != dbt->brp)
 				netdev_warn(dev, "Warning!! data brp = %d and brp = %d don't match.\n"
 					    "flexcan may not work. consider using different bitrate or data bitrate\n",
@@ -1296,6 +1311,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* FDCTRL */
 	if (priv->can.ctrlmode_supported & CAN_CTRLMODE_FD) {
 		reg_fdctrl = priv->read(&regs->fdctrl) & ~FLEXCAN_FDCTRL_FDRATE;
+		reg_fdctrl &= ~FLEXCAN_FDCTRL_TDCEN;
 		reg_fdctrl &= ~(FLEXCAN_FDCTRL_MBDSR1(0x3) | FLEXCAN_FDCTRL_MBDSR0(0x3));
 		reg_mcr = priv->read(&regs->mcr) & ~FLEXCAN_MCR_FDEN;
 		reg_ctrl2 = priv->read(&regs->ctrl2) & ~FLEXCAN_CTRL2_ISOCANFDEN;
-- 
2.17.1

[PATCH 7/8] can: flexcan: add imx8qm support

[Joakim Zhang]

The Flexcan on i.MX8QM supports CAN FD protocol.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 2c48151e431f..f1fdaae52ef4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -346,6 +346,12 @@ static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 		FLEXCAN_QUIRK_SETUP_STOP_MODE,
 };
 
+static struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
+	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_TIMESTAMP_SUPPORT_FD,
+};
+
 static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
 		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
@@ -1703,6 +1709,7 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
 }
 
 static const struct of_device_id flexcan_of_match[] = {
+	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
 	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
-- 
2.17.1

[PATCH 8/8] can: flexcan: add lx2160ar1 support

[Joakim Zhang]

The Flexcan on lx2160ar1 supports CAN FD protocol.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f1fdaae52ef4..f5c66f284c70 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -358,6 +358,12 @@ static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
 		FLEXCAN_QUIRK_BROKEN_PERR_STATE,
 };
 
+static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
+	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_TIMESTAMP_SUPPORT_FD,
+};
+
 static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
 		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
@@ -1709,6 +1715,7 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
 }
 
 static const struct of_device_id flexcan_of_match[] = {
+	{ .compatible = "fsl,lx2160ar1-flexcan", .data = &fsl_lx2160a_r1_devtype_data, },
 	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
-- 
2.17.1

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

Hi Marc,

Kindly pinging...

After you git pull request for linux-can-next-for-5.4-20190724, some patches are missing from linux-can-next/testing.
can: flexcan: flexcan_mailbox_read() make use of flexcan_write64() to mark the mailbox as read
can: flexcan: flexcan_irq(): add support for TX mailbox in iflag1
can: flexcan: flexcan_read_reg_iflag_rx(): optimize reading
can: flexcan: introduce struct flexcan_priv::tx_mask and make use of it
can: flexcan: convert struct flexcan_priv::rx_mask{1,2} to rx_mask
can: flexcan: remove TX mailbox bit from struct flexcan_priv::rx_mask{1,2}
can: flexcan: rename struct flexcan_priv::reg_imask{1,2}_default to rx_mask{1,2}
can: flexcan: flexcan_irq(): rename variable reg_iflag -> reg_iflag_rx
can: flexcan: rename macro FLEXCAN_IFLAG_MB() -> FLEXCAN_IFLAG2_MB()

You can refer to below link for the reason of adding above patches:
https://www.spinics.net/lists/linux-can/msg00777.html
https://www.spinics.net/lists/linux-can/msg01150.html

Are you prepared to add back these patches as they are necessary for Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these patches.

Thanks a lot!

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Joakim Zhang
> Sent: 2019年7月12日 16:03
> To: mkl@pengutronix.de; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; dl-linux-imx <linux-imx@nxp.com>;
> netdev@vger.kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>
> Subject: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> Hi Marc,
> 
> This patch set intends to add support for NXP Flexcan CAN FD, it has been
> validated on three NXP platform(i.MX8QM/QXP, S32V234, LX2160AR1).
> After discussed with another two Fexcan owner, we sorted out this version.
> 
> I hope you can pick up the patch set as it can fully meet requirement of above
> three platform. And after that, we can start to do upstream about CAN FD.
> 
> Thanks a lot!
> 
> BRs,
> Joakim Zhang
> 
> Joakim Zhang (8):
>   can: flexcan: allocate skb in flexcan_mailbox_read
>   can: flexcan: use struct canfd_frame for CAN classic frame
>   can: flexcan: add CAN FD mode support
>   can: flexcan: add CANFD BRS support
>   can: flexcan: add ISO CAN FD feature support
>   can: flexcan: add Transceiver Delay Compensation suopport
>   can: flexcan: add imx8qm support
>   can: flexcan: add lx2160ar1 support
> 
>  drivers/net/can/flexcan.c      | 340 ++++++++++++++++++++++++++++-----
>  drivers/net/can/rx-offload.c   |  33 +---
>  include/linux/can/rx-offload.h |   5 +-
>  3 files changed, 305 insertions(+), 73 deletions(-)
> 
> --
> 2.17.1

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Marc Kleine-Budde]

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

On 7/25/19 9:38 AM, Joakim Zhang wrote:
> Kindly pinging...
> 
> After you git pull request for linux-can-next-for-5.4-20190724, some patches are missing from linux-can-next/testing.
> can: flexcan: flexcan_mailbox_read() make use of flexcan_write64() to mark the mailbox as read
> can: flexcan: flexcan_irq(): add support for TX mailbox in iflag1
> can: flexcan: flexcan_read_reg_iflag_rx(): optimize reading
> can: flexcan: introduce struct flexcan_priv::tx_mask and make use of it
> can: flexcan: convert struct flexcan_priv::rx_mask{1,2} to rx_mask
> can: flexcan: remove TX mailbox bit from struct flexcan_priv::rx_mask{1,2}
> can: flexcan: rename struct flexcan_priv::reg_imask{1,2}_default to rx_mask{1,2}
> can: flexcan: flexcan_irq(): rename variable reg_iflag -> reg_iflag_rx
> can: flexcan: rename macro FLEXCAN_IFLAG_MB() -> FLEXCAN_IFLAG2_MB()
> 
> You can refer to below link for the reason of adding above patches:
> https://www.spinics.net/lists/linux-can/msg00777.html
> https://www.spinics.net/lists/linux-can/msg01150.html
> 
> Are you prepared to add back these patches as they are necessary for
> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
> patches.

Yes, these patches will be added back.

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: 488 bytes --]

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Marc Kleine-Budde]

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

On 7/25/19 9:53 AM, Marc Kleine-Budde wrote:
> On 7/25/19 9:38 AM, Joakim Zhang wrote:
>> Kindly pinging...
>>
>> After you git pull request for linux-can-next-for-5.4-20190724, some patches are missing from linux-can-next/testing.
>> can: flexcan: flexcan_mailbox_read() make use of flexcan_write64() to mark the mailbox as read
>> can: flexcan: flexcan_irq(): add support for TX mailbox in iflag1
>> can: flexcan: flexcan_read_reg_iflag_rx(): optimize reading
>> can: flexcan: introduce struct flexcan_priv::tx_mask and make use of it
>> can: flexcan: convert struct flexcan_priv::rx_mask{1,2} to rx_mask
>> can: flexcan: remove TX mailbox bit from struct flexcan_priv::rx_mask{1,2}
>> can: flexcan: rename struct flexcan_priv::reg_imask{1,2}_default to rx_mask{1,2}
>> can: flexcan: flexcan_irq(): rename variable reg_iflag -> reg_iflag_rx
>> can: flexcan: rename macro FLEXCAN_IFLAG_MB() -> FLEXCAN_IFLAG2_MB()
>>
>> You can refer to below link for the reason of adding above patches:
>> https://www.spinics.net/lists/linux-can/msg00777.html
>> https://www.spinics.net/lists/linux-can/msg01150.html
>>
>> Are you prepared to add back these patches as they are necessary for
>> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
>> patches.
> 
> Yes, these patches will be added back.

I've cleaned up the first patch a bit, and pushed everything to the
testing branch. Can you give it a test.

regards,
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: 488 bytes --]

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年7月25日 18:37
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; dl-linux-imx <linux-imx@nxp.com>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> On 7/25/19 9:53 AM, Marc Kleine-Budde wrote:
> > On 7/25/19 9:38 AM, Joakim Zhang wrote:
> >> Kindly pinging...
> >>
> >> After you git pull request for linux-can-next-for-5.4-20190724, some patches
> are missing from linux-can-next/testing.
> >> can: flexcan: flexcan_mailbox_read() make use of flexcan_write64() to
> >> mark the mailbox as read
> >> can: flexcan: flexcan_irq(): add support for TX mailbox in iflag1
> >> can: flexcan: flexcan_read_reg_iflag_rx(): optimize reading
> >> can: flexcan: introduce struct flexcan_priv::tx_mask and make use of
> >> it
> >> can: flexcan: convert struct flexcan_priv::rx_mask{1,2} to rx_mask
> >> can: flexcan: remove TX mailbox bit from struct
> >> flexcan_priv::rx_mask{1,2}
> >> can: flexcan: rename struct flexcan_priv::reg_imask{1,2}_default to
> >> rx_mask{1,2}
> >> can: flexcan: flexcan_irq(): rename variable reg_iflag ->
> >> reg_iflag_rx
> >> can: flexcan: rename macro FLEXCAN_IFLAG_MB() ->
> FLEXCAN_IFLAG2_MB()
> >>
> >> You can refer to below link for the reason of adding above patches:
> >> https://www.spinics.net/lists/linux-can/msg00777.html
> >> https://www.spinics.net/lists/linux-can/msg01150.html
> >>
> >> Are you prepared to add back these patches as they are necessary for
> >> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
> >> patches.
> >
> > Yes, these patches will be added back.
> 
> I've cleaned up the first patch a bit, and pushed everything to the testing
> branch. Can you give it a test.

Hi Marc,

Both Classic CAN and CAN FD can work fine on my side test, thank you for your kindly review.

Best Regards,
Joakim Zhang
> regards,
> 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   |

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Hi,

>>> Are you prepared to add back these patches as they are necessary for
>>> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
>>> patches.
>>
>> Yes, these patches will be added back.
>
>I've cleaned up the first patch a bit, and pushed everything to the
>testing branch. Can you give it a test.

What happend to that branch? FWIW I've just tried the patches on a custom
board with a LS1028A SoC. Both CAN and CAN-FD are working. I've tested
against a Peaktech USB CAN adapter. I'd love to see these patches upstream,
because our board also offers CAN and basic support for it just made it
upstream [1].

If these patches are upstream, only the device tree nodes seems to be
missing. I don't know what has happened to [2]. But the patch doesn't seem
to be necessary.

Pankaj already send a patch to add the device node to the LS1028A [3].
Thats basically the same I've used, only that mine didn't had the
"fsl,ls1028ar1-flexcan" compatiblity string, but only the
"lx2160ar1-flexcan" which is the correct way to use it, right?

Sorry for putting this all in one mail, but I've just subscribed to
linux-can and there is no message archive on lore.kernel.org for it :/

[1] https://lore.kernel.org/lkml/20200212073617.GA11096@dragon/
[2] https://www.spinics.net/lists/linux-can/msg01584.html
[3] https://www.spinics.net/lists/linux-can/msg01577.html

-michael

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

Hi Michal,

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: 2020年2月14日 3:20
> To: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> Hi,
> 
> >>> Are you prepared to add back these patches as they are necessary for
> >>> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
> >>> patches.
> >>
> >> Yes, these patches will be added back.
> >
> >I've cleaned up the first patch a bit, and pushed everything to the
> >testing branch. Can you give it a test.
> 
> What happend to that branch? FWIW I've just tried the patches on a custom
> board with a LS1028A SoC. Both CAN and CAN-FD are working. I've tested
> against a Peaktech USB CAN adapter. I'd love to see these patches upstream,
> because our board also offers CAN and basic support for it just made it
> upstream [1].
The FlexCAN CAN FD related patches have stayed in linux-can-next/flexcan branch for a long time, I still don't know why Marc doesn't merge them into Linux mainline.
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tree/?h=flexcan
Also must hope that this patch set can be upstreamed soon. :-)

> If these patches are upstream, only the device tree nodes seems to be missing.
> I don't know what has happened to [2]. But the patch doesn't seem to be
> necessary.
Yes, this patch is unnecessary. I have NACKed this patch for that, according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
But it is actually decided by SoC integration, for i.MX, the design is different.
I have not upstream i.MX FlexCAN device tree nodes, since it's dependency have not upstreamed yet.

> Pankaj already send a patch to add the device node to the LS1028A [3].
> Thats basically the same I've used, only that mine didn't had the
> "fsl,ls1028ar1-flexcan" compatiblity string, but only the "lx2160ar1-flexcan"
> which is the correct way to use it, right?
You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan" supports CAN FD, you can use this compatible string.
static const struct of_device_id flexcan_of_match[] = {
	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
	{ .compatible = "fsl,imx35-flexcan", .data = &fsl_imx25_devtype_data, },
	{ .compatible = "fsl,imx25-flexcan", .data = &fsl_imx25_devtype_data, },
	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
	{ .compatible = "fsl,ls1021ar2-flexcan", .data = &fsl_ls1021a_r2_devtype_data, },
	{ .compatible = "fsl,lx2160ar1-flexcan", .data = &fsl_lx2160a_r1_devtype_data, },
	{ /* sentinel */ },
};

> Sorry for putting this all in one mail, but I've just subscribed to linux-can and
> there is no message archive on lore.kernel.org for it :/
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flkml%2F20200212073617.GA11096%40dragon%2F&amp;data=02%
> 7C01%7Cqiangqing.zhang%40nxp.com%7Cd278fdc027a54bc9f57b08d7b0b9d2
> ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637172184473466
> 188&amp;sdata=fbIOrIG4PhL5f4Sc7P9sTFw%2FoNoinz%2Fd56TyLjnHdn8%3D&
> amp;reserved=0
> [2]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sp
> inics.net%2Flists%2Flinux-can%2Fmsg01584.html&amp;data=02%7C01%7Cqia
> ngqing.zhang%40nxp.com%7Cd278fdc027a54bc9f57b08d7b0b9d2ef%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637172184473466188&amp;sd
> ata=BXCIcEzNXN1rh5%2FLKjR8ciM3gZ%2Fdkl3%2BDAohhfgg1PQ%3D&amp;re
> served=0
> [3]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sp
> inics.net%2Flists%2Flinux-can%2Fmsg01577.html&amp;data=02%7C01%7Cqia
> ngqing.zhang%40nxp.com%7Cd278fdc027a54bc9f57b08d7b0b9d2ef%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637172184473466188&amp;sd
> ata=9O6Vg69e%2FNbR2n1F%2BKKKiEqGmYPHO4qLCTEuCQhRS4U%3D&amp;r
> eserved=0
> 
> -michael

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Hi Joakim,

Am 2020-02-14 02:55, schrieb Joakim Zhang:
> Hi Michal,
> 
>> -----Original Message-----
>> From: Michael Walle <michael@walle.cc>
>> Sent: 2020年2月14日 3:20
>> To: Marc Kleine-Budde <mkl@pengutronix.de>
>> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
>> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
>> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP 
>> Flexcan
>> 
>> Hi,
>> 
>> >>> Are you prepared to add back these patches as they are necessary for
>> >>> Flexcan CAN FD? And this Flexcan CAN FD patch set is based on these
>> >>> patches.
>> >>
>> >> Yes, these patches will be added back.
>> >
>> >I've cleaned up the first patch a bit, and pushed everything to the
>> >testing branch. Can you give it a test.
>> 
>> What happend to that branch? FWIW I've just tried the patches on a 
>> custom
>> board with a LS1028A SoC. Both CAN and CAN-FD are working. I've tested
>> against a Peaktech USB CAN adapter. I'd love to see these patches 
>> upstream,
>> because our board also offers CAN and basic support for it just made 
>> it
>> upstream [1].
> The FlexCAN CAN FD related patches have stayed in
> linux-can-next/flexcan branch for a long time, I still don't know why
> Marc doesn't merge them into Linux mainline.
> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tree/?h=flexcan
> Also must hope that this patch set can be upstreamed soon. :-)

I've took them from this branch and applied them to the latest linux 
master.

Thus,

Tested-by: Michael Walle <michael@walle.cc>


>> If these patches are upstream, only the device tree nodes seems to be 
>> missing.
>> I don't know what has happened to [2]. But the patch doesn't seem to 
>> be
>> necessary.
> Yes, this patch is unnecessary. I have NACKed this patch for that,
> according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
> oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
> But it is actually decided by SoC integration, for i.MX, the design is
> different.

ok thanks for clarifying.

> I have not upstream i.MX FlexCAN device tree nodes, since it's
> dependency have not upstreamed yet.
> 
>> Pankaj already send a patch to add the device node to the LS1028A [3].
>> Thats basically the same I've used, only that mine didn't had the
>> "fsl,ls1028ar1-flexcan" compatiblity string, but only the 
>> "lx2160ar1-flexcan"
>> which is the correct way to use it, right?
> You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
> supports CAN FD, you can use this compatible string.

correct. I've already a patch that does exactly this ;) Who would take 
the
patch for adding the LS1028A can device tree nodes to ls1028a.dtsi? You 
or
Shawn Guo?

> static const struct of_device_id flexcan_of_match[] = {
> 	{ .compatible = "fsl,imx8qm-flexcan", .data = 
> &fsl_imx8qm_devtype_data, },
> 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, 
> },
> 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, 
> },
> 	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, 
> },
> 	{ .compatible = "fsl,imx35-flexcan", .data = &fsl_imx25_devtype_data, 
> },
> 	{ .compatible = "fsl,imx25-flexcan", .data = &fsl_imx25_devtype_data, 
> },
> 	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, 
> },
> 	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, 
> },
> 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
> &fsl_ls1021a_r2_devtype_data, },
> 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
> &fsl_lx2160a_r1_devtype_data, },
> 	{ /* sentinel */ },
> };
> 

-michael

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: 2020年2月14日 16:43
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>
> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> Hi Joakim,
> 
> Am 2020-02-14 02:55, schrieb Joakim Zhang:
> > Hi Michal,
> >
> >> -----Original Message-----
> >> From: Michael Walle <michael@walle.cc>
> >> Sent: 2020年2月14日 3:20
> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> >> Flexcan
> >>
> >> Hi,
> >>
> >> >>> Are you prepared to add back these patches as they are necessary
> >> >>> for Flexcan CAN FD? And this Flexcan CAN FD patch set is based on
> >> >>> these patches.
> >> >>
> >> >> Yes, these patches will be added back.
> >> >
> >> >I've cleaned up the first patch a bit, and pushed everything to the
> >> >testing branch. Can you give it a test.
> >>
> >> What happend to that branch? FWIW I've just tried the patches on a
> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
> >> I've tested against a Peaktech USB CAN adapter. I'd love to see these
> >> patches upstream, because our board also offers CAN and basic support
> >> for it just made it upstream [1].
> > The FlexCAN CAN FD related patches have stayed in
> > linux-can-next/flexcan branch for a long time, I still don't know why
> > Marc doesn't merge them into Linux mainline.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> >
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.g
> >
> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
> xp.co
> >
> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
> 5c30163
> >
> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
> 8%2FdNSV3
> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
> > Also must hope that this patch set can be upstreamed soon. :-)
> 
> I've took them from this branch and applied them to the latest linux master.
> 
> Thus,
> 
> Tested-by: Michael Walle <michael@walle.cc>
> 
> 
> >> If these patches are upstream, only the device tree nodes seems to be
> >> missing.
> >> I don't know what has happened to [2]. But the patch doesn't seem to
> >> be necessary.
> > Yes, this patch is unnecessary. I have NACKed this patch for that,
> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
> > But it is actually decided by SoC integration, for i.MX, the design is
> > different.
> 
> ok thanks for clarifying.
> 
> > I have not upstream i.MX FlexCAN device tree nodes, since it's
> > dependency have not upstreamed yet.
> >
> >> Pankaj already send a patch to add the device node to the LS1028A [3].
> >> Thats basically the same I've used, only that mine didn't had the
> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
> >> "lx2160ar1-flexcan"
> >> which is the correct way to use it, right?
> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
> > supports CAN FD, you can use this compatible string.
> 
> correct. I've already a patch that does exactly this ;) Who would take the patch
> for adding the LS1028A can device tree nodes to ls1028a.dtsi? You or Shawn
> Guo?
Sorry, I missed the link[3], we usually write it this way:
			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
Please send patch to Shawn Guo, he will review the device tree.

> > static const struct of_device_id flexcan_of_match[] = {
> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
> > &fsl_imx8qm_devtype_data, },
> > 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data,
> > },
> > 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data,
> > },
> > 	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data,
> > },
> > 	{ .compatible = "fsl,imx35-flexcan", .data = &fsl_imx25_devtype_data,
> > },
> > 	{ .compatible = "fsl,imx25-flexcan", .data = &fsl_imx25_devtype_data,
> > },
> > 	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data,
> > },
> > 	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data,
> > },
> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
> > &fsl_ls1021a_r2_devtype_data, },
> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
> > &fsl_lx2160a_r1_devtype_data, },
> > 	{ /* sentinel */ },
> > };
> >
> 
> -michael

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Am 2020-02-14 10:18, schrieb Joakim Zhang:
> Best Regards,
> Joakim Zhang
> 
>> -----Original Message-----
>> From: Michael Walle <michael@walle.cc>
>> Sent: 2020年2月14日 16:43
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> <pankaj.bansal@nxp.com>
>> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP 
>> Flexcan
>> 
>> Hi Joakim,
>> 
>> Am 2020-02-14 02:55, schrieb Joakim Zhang:
>> > Hi Michal,
>> >
>> >> -----Original Message-----
>> >> From: Michael Walle <michael@walle.cc>
>> >> Sent: 2020年2月14日 3:20
>> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
>> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
>> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
>> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> >> Flexcan
>> >>
>> >> Hi,
>> >>
>> >> >>> Are you prepared to add back these patches as they are necessary
>> >> >>> for Flexcan CAN FD? And this Flexcan CAN FD patch set is based on
>> >> >>> these patches.
>> >> >>
>> >> >> Yes, these patches will be added back.
>> >> >
>> >> >I've cleaned up the first patch a bit, and pushed everything to the
>> >> >testing branch. Can you give it a test.
>> >>
>> >> What happend to that branch? FWIW I've just tried the patches on a
>> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
>> >> I've tested against a Peaktech USB CAN adapter. I'd love to see these
>> >> patches upstream, because our board also offers CAN and basic support
>> >> for it just made it upstream [1].
>> > The FlexCAN CAN FD related patches have stayed in
>> > linux-can-next/flexcan branch for a long time, I still don't know why
>> > Marc doesn't merge them into Linux mainline.
>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
>> >
>> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.g
>> >
>> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
>> xp.co
>> >
>> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
>> 5c30163
>> >
>> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
>> 8%2FdNSV3
>> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
>> > Also must hope that this patch set can be upstreamed soon. :-)
>> 
>> I've took them from this branch and applied them to the latest linux 
>> master.
>> 
>> Thus,
>> 
>> Tested-by: Michael Walle <michael@walle.cc>
>> 
>> 
>> >> If these patches are upstream, only the device tree nodes seems to be
>> >> missing.
>> >> I don't know what has happened to [2]. But the patch doesn't seem to
>> >> be necessary.
>> > Yes, this patch is unnecessary. I have NACKed this patch for that,
>> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
>> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
>> > But it is actually decided by SoC integration, for i.MX, the design is
>> > different.
>> 
>> ok thanks for clarifying.
>> 
>> > I have not upstream i.MX FlexCAN device tree nodes, since it's
>> > dependency have not upstreamed yet.
>> >
>> >> Pankaj already send a patch to add the device node to the LS1028A [3].
>> >> Thats basically the same I've used, only that mine didn't had the
>> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
>> >> "lx2160ar1-flexcan"
>> >> which is the correct way to use it, right?
>> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
>> > supports CAN FD, you can use this compatible string.
>> 
>> correct. I've already a patch that does exactly this ;) Who would take 
>> the patch
>> for adding the LS1028A can device tree nodes to ls1028a.dtsi? You or 
>> Shawn
>> Guo?
> Sorry, I missed the link[3], we usually write it this way:
> 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
> Please send patch to Shawn Guo, he will review the device tree.

As far as I know, there should be no undocumented binding. Eg. the 
ls1028ar1-flexcan
is neither in the source nor in the device tree binding documentation, 
thus wouldn't
be accepted.

Thus either there should be another ls1028ar1-flexcan in the 
flexcan_of_match table
and the node should only contain that string or the node should only 
contain
fsl,lx2160ar1-flexcan. Is there any advantage of the first option?


-michael


> 
>> > static const struct of_device_id flexcan_of_match[] = {
>> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
>> > &fsl_imx8qm_devtype_data, },
>> > 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,imx35-flexcan", .data = &fsl_imx25_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,imx25-flexcan", .data = &fsl_imx25_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data,
>> > },
>> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
>> > &fsl_ls1021a_r2_devtype_data, },
>> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
>> > &fsl_lx2160a_r1_devtype_data, },
>> > 	{ /* sentinel */ },
>> > };
>> >
>> 
>> -michael

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: 2020年2月14日 17:33
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> Am 2020-02-14 10:18, schrieb Joakim Zhang:
> > Best Regards,
> > Joakim Zhang
> >
> >> -----Original Message-----
> >> From: Michael Walle <michael@walle.cc>
> >> Sent: 2020年2月14日 16:43
> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> >> <pankaj.bansal@nxp.com>
> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> >> Flexcan
> >>
> >> Hi Joakim,
> >>
> >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
> >> > Hi Michal,
> >> >
> >> >> -----Original Message-----
> >> >> From: Michael Walle <michael@walle.cc>
> >> >> Sent: 2020年2月14日 3:20
> >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
> >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> >> >> Flexcan
> >> >>
> >> >> Hi,
> >> >>
> >> >> >>> Are you prepared to add back these patches as they are
> >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
> >> >> >>> set is based on these patches.
> >> >> >>
> >> >> >> Yes, these patches will be added back.
> >> >> >
> >> >> >I've cleaned up the first patch a bit, and pushed everything to
> >> >> >the testing branch. Can you give it a test.
> >> >>
> >> >> What happend to that branch? FWIW I've just tried the patches on a
> >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
> >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
> >> >> these patches upstream, because our board also offers CAN and
> >> >> basic support for it just made it upstream [1].
> >> > The FlexCAN CAN FD related patches have stayed in
> >> > linux-can-next/flexcan branch for a long time, I still don't know
> >> > why Marc doesn't merge them into Linux mainline.
> >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> >> >
> >>
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.
> >> g
> >> >
> >>
> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
> >> xp.co
> >> >
> >>
> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
> >> 5c30163
> >> >
> >>
> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
> >> 8%2FdNSV3
> >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
> >> > Also must hope that this patch set can be upstreamed soon. :-)
> >>
> >> I've took them from this branch and applied them to the latest linux
> >> master.
> >>
> >> Thus,
> >>
> >> Tested-by: Michael Walle <michael@walle.cc>
> >>
> >>
> >> >> If these patches are upstream, only the device tree nodes seems to
> >> >> be missing.
> >> >> I don't know what has happened to [2]. But the patch doesn't seem
> >> >> to be necessary.
> >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
> >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
> >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
> >> > But it is actually decided by SoC integration, for i.MX, the design
> >> > is different.
> >>
> >> ok thanks for clarifying.
> >>
> >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
> >> > dependency have not upstreamed yet.
> >> >
> >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
> >> >> Thats basically the same I've used, only that mine didn't had the
> >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
> >> >> "lx2160ar1-flexcan"
> >> >> which is the correct way to use it, right?
> >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
> >> > supports CAN FD, you can use this compatible string.
> >>
> >> correct. I've already a patch that does exactly this ;) Who would
> >> take the patch for adding the LS1028A can device tree nodes to
> >> ls1028a.dtsi? You or Shawn Guo?
> > Sorry, I missed the link[3], we usually write it this way:
> > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
> > Please send patch to Shawn Guo, he will review the device tree.
> 
> As far as I know, there should be no undocumented binding. Eg. the
> ls1028ar1-flexcan is neither in the source nor in the device tree binding
> documentation, thus wouldn't be accepted.
> 
> Thus either there should be another ls1028ar1-flexcan in the flexcan_of_match
> table and the node should only contain that string or the node should only
> contain fsl,lx2160ar1-flexcan. Is there any advantage of the first option?
From the FlexCAN binding(Documentation/devicetree/bindings/net/can/fsl-flexcan.txt)
- compatible : Should be "fsl,<processor>-flexcan"

  An implementation should also claim any of the following compatibles
  that it is fully backwards compatible with:

  - fsl,p1010-flexcan

You also can check imx6ul.dtsi imx7s.dtsi etc.

Sorry :-(, I also don't know the advantage, it's just that we're used to writing it that way. You can check nodes of other devices.
It's unnecessary to add compatible string for each SoCs since they may share the same IP. And dts had batter have a SoC specific compatible string. It's just my understanding.

Joakim

> -michael
> 
> 
> >
> >> > static const struct of_device_id flexcan_of_match[] = {
> >> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
> >> > &fsl_imx8qm_devtype_data, },
> >> > 	{ .compatible = "fsl,imx6q-flexcan", .data =
> >> > &fsl_imx6q_devtype_data, },
> >> > 	{ .compatible = "fsl,imx28-flexcan", .data =
> >> > &fsl_imx28_devtype_data, },
> >> > 	{ .compatible = "fsl,imx53-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,imx35-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,imx25-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,p1010-flexcan", .data =
> >> > &fsl_p1010_devtype_data, },
> >> > 	{ .compatible = "fsl,vf610-flexcan", .data =
> >> > &fsl_vf610_devtype_data, },
> >> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
> >> > &fsl_ls1021a_r2_devtype_data, },
> >> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
> >> > &fsl_lx2160a_r1_devtype_data, },
> >> > 	{ /* sentinel */ },
> >> > };
> >> >
> >>
> >> -michael

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Hi Joakim, Hi Shawn,


Am 2020-02-14 10:56, schrieb Joakim Zhang:
>> -----Original Message-----
>> From: Michael Walle <michael@walle.cc>
>> Sent: 2020年2月14日 17:33
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
>> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP 
>> Flexcan
>> 
>> Am 2020-02-14 10:18, schrieb Joakim Zhang:
>> > Best Regards,
>> > Joakim Zhang
>> >
>> >> -----Original Message-----
>> >> From: Michael Walle <michael@walle.cc>
>> >> Sent: 2020年2月14日 16:43
>> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> >> <pankaj.bansal@nxp.com>
>> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> >> Flexcan
>> >>
>> >> Hi Joakim,
>> >>
>> >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
>> >> > Hi Michal,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Michael Walle <michael@walle.cc>
>> >> >> Sent: 2020年2月14日 3:20
>> >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
>> >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
>> >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
>> >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> >> >> Flexcan
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> >>> Are you prepared to add back these patches as they are
>> >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
>> >> >> >>> set is based on these patches.
>> >> >> >>
>> >> >> >> Yes, these patches will be added back.
>> >> >> >
>> >> >> >I've cleaned up the first patch a bit, and pushed everything to
>> >> >> >the testing branch. Can you give it a test.
>> >> >>
>> >> >> What happend to that branch? FWIW I've just tried the patches on a
>> >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
>> >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
>> >> >> these patches upstream, because our board also offers CAN and
>> >> >> basic support for it just made it upstream [1].
>> >> > The FlexCAN CAN FD related patches have stayed in
>> >> > linux-can-next/flexcan branch for a long time, I still don't know
>> >> > why Marc doesn't merge them into Linux mainline.
>> >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
>> >> >
>> >>
>> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.
>> >> g
>> >> >
>> >>
>> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
>> >> xp.co
>> >> >
>> >>
>> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
>> >> 5c30163
>> >> >
>> >>
>> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
>> >> 8%2FdNSV3
>> >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
>> >> > Also must hope that this patch set can be upstreamed soon. :-)
>> >>
>> >> I've took them from this branch and applied them to the latest linux
>> >> master.
>> >>
>> >> Thus,
>> >>
>> >> Tested-by: Michael Walle <michael@walle.cc>
>> >>
>> >>
>> >> >> If these patches are upstream, only the device tree nodes seems to
>> >> >> be missing.
>> >> >> I don't know what has happened to [2]. But the patch doesn't seem
>> >> >> to be necessary.
>> >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
>> >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
>> >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
>> >> > But it is actually decided by SoC integration, for i.MX, the design
>> >> > is different.
>> >>
>> >> ok thanks for clarifying.
>> >>
>> >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
>> >> > dependency have not upstreamed yet.
>> >> >
>> >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
>> >> >> Thats basically the same I've used, only that mine didn't had the
>> >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
>> >> >> "lx2160ar1-flexcan"
>> >> >> which is the correct way to use it, right?
>> >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
>> >> > supports CAN FD, you can use this compatible string.
>> >>
>> >> correct. I've already a patch that does exactly this ;) Who would
>> >> take the patch for adding the LS1028A can device tree nodes to
>> >> ls1028a.dtsi? You or Shawn Guo?
>> > Sorry, I missed the link[3], we usually write it this way:
>> > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
>> > Please send patch to Shawn Guo, he will review the device tree.
>> 
>> As far as I know, there should be no undocumented binding. Eg. the
>> ls1028ar1-flexcan is neither in the source nor in the device tree 
>> binding
>> documentation, thus wouldn't be accepted.
>> 
>> Thus either there should be another ls1028ar1-flexcan in the 
>> flexcan_of_match
>> table and the node should only contain that string or the node should 
>> only
>> contain fsl,lx2160ar1-flexcan. Is there any advantage of the first 
>> option?
> From the FlexCAN
> binding(Documentation/devicetree/bindings/net/can/fsl-flexcan.txt)
> - compatible : Should be "fsl,<processor>-flexcan"
> 
>   An implementation should also claim any of the following compatibles
>   that it is fully backwards compatible with:
> 
>   - fsl,p1010-flexcan
> 
> You also can check imx6ul.dtsi imx7s.dtsi etc.
> 
> Sorry :-(, I also don't know the advantage, it's just that we're used
> to writing it that way. You can check nodes of other devices.
> It's unnecessary to add compatible string for each SoCs since they may
> share the same IP. And dts had batter have a SoC specific compatible
> string. It's just my understanding.

Ah thanks. So Pankaj's patch [1] seems to be correct (at least according
to the description in the device tree documentation).

Shawn, whats your opinion?

-michael

[1] https://www.spinics.net/lists/linux-can/msg01577.html

> 
> Joakim
> 
>> -michael
>> 
>> 
>> >
>> >> > static const struct of_device_id flexcan_of_match[] = {
>> >> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
>> >> > &fsl_imx8qm_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx6q-flexcan", .data =
>> >> > &fsl_imx6q_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx28-flexcan", .data =
>> >> > &fsl_imx28_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx53-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx35-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx25-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,p1010-flexcan", .data =
>> >> > &fsl_p1010_devtype_data, },
>> >> > 	{ .compatible = "fsl,vf610-flexcan", .data =
>> >> > &fsl_vf610_devtype_data, },
>> >> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
>> >> > &fsl_ls1021a_r2_devtype_data, },
>> >> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
>> >> > &fsl_lx2160a_r1_devtype_data, },
>> >> > 	{ /* sentinel */ },
>> >> > };
>> >> >
>> >>
>> >> -michael

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Shawn Guo]

On Fri, Feb 14, 2020 at 11:02:46AM +0100, Michael Walle wrote:
> 
> Hi Joakim, Hi Shawn,
> 
> 
> Am 2020-02-14 10:56, schrieb Joakim Zhang:
> > > -----Original Message-----
> > > From: Michael Walle <michael@walle.cc>
> > > Sent: 2020年2月14日 17:33
> > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> > > netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> > > <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> > > Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> > > Flexcan
> > > 
> > > Am 2020-02-14 10:18, schrieb Joakim Zhang:
> > > > Best Regards,
> > > > Joakim Zhang
> > > >
> > > >> -----Original Message-----
> > > >> From: Michael Walle <michael@walle.cc>
> > > >> Sent: 2020年2月14日 16:43
> > > >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> > > >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> > > >> <pankaj.bansal@nxp.com>
> > > >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> > > >> Flexcan
> > > >>
> > > >> Hi Joakim,
> > > >>
> > > >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
> > > >> > Hi Michal,
> > > >> >
> > > >> >> -----Original Message-----
> > > >> >> From: Michael Walle <michael@walle.cc>
> > > >> >> Sent: 2020年2月14日 3:20
> > > >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
> > > >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> > > >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> > > >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> > > >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> > > >> >> Flexcan
> > > >> >>
> > > >> >> Hi,
> > > >> >>
> > > >> >> >>> Are you prepared to add back these patches as they are
> > > >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
> > > >> >> >>> set is based on these patches.
> > > >> >> >>
> > > >> >> >> Yes, these patches will be added back.
> > > >> >> >
> > > >> >> >I've cleaned up the first patch a bit, and pushed everything to
> > > >> >> >the testing branch. Can you give it a test.
> > > >> >>
> > > >> >> What happend to that branch? FWIW I've just tried the patches on a
> > > >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
> > > >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
> > > >> >> these patches upstream, because our board also offers CAN and
> > > >> >> basic support for it just made it upstream [1].
> > > >> > The FlexCAN CAN FD related patches have stayed in
> > > >> > linux-can-next/flexcan branch for a long time, I still don't know
> > > >> > why Marc doesn't merge them into Linux mainline.
> > > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> > > >> >
> > > >>
> > > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.
> > > >> g
> > > >> >
> > > >>
> > > it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
> > > >> xp.co
> > > >> >
> > > >>
> > > m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
> > > >> 5c30163
> > > >> >
> > > >>
> > > 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
> > > >> 8%2FdNSV3
> > > >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
> > > >> > Also must hope that this patch set can be upstreamed soon. :-)
> > > >>
> > > >> I've took them from this branch and applied them to the latest linux
> > > >> master.
> > > >>
> > > >> Thus,
> > > >>
> > > >> Tested-by: Michael Walle <michael@walle.cc>
> > > >>
> > > >>
> > > >> >> If these patches are upstream, only the device tree nodes seems to
> > > >> >> be missing.
> > > >> >> I don't know what has happened to [2]. But the patch doesn't seem
> > > >> >> to be necessary.
> > > >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
> > > >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
> > > >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
> > > >> > But it is actually decided by SoC integration, for i.MX, the design
> > > >> > is different.
> > > >>
> > > >> ok thanks for clarifying.
> > > >>
> > > >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
> > > >> > dependency have not upstreamed yet.
> > > >> >
> > > >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
> > > >> >> Thats basically the same I've used, only that mine didn't had the
> > > >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
> > > >> >> "lx2160ar1-flexcan"
> > > >> >> which is the correct way to use it, right?
> > > >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
> > > >> > supports CAN FD, you can use this compatible string.
> > > >>
> > > >> correct. I've already a patch that does exactly this ;) Who would
> > > >> take the patch for adding the LS1028A can device tree nodes to
> > > >> ls1028a.dtsi? You or Shawn Guo?
> > > > Sorry, I missed the link[3], we usually write it this way:
> > > > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
> > > > Please send patch to Shawn Guo, he will review the device tree.
> > > 
> > > As far as I know, there should be no undocumented binding. Eg. the
> > > ls1028ar1-flexcan is neither in the source nor in the device tree
> > > binding
> > > documentation, thus wouldn't be accepted.
> > > 
> > > Thus either there should be another ls1028ar1-flexcan in the
> > > flexcan_of_match
> > > table and the node should only contain that string or the node
> > > should only
> > > contain fsl,lx2160ar1-flexcan. Is there any advantage of the first
> > > option?
> > From the FlexCAN
> > binding(Documentation/devicetree/bindings/net/can/fsl-flexcan.txt)
> > - compatible : Should be "fsl,<processor>-flexcan"
> > 
> >   An implementation should also claim any of the following compatibles
> >   that it is fully backwards compatible with:
> > 
> >   - fsl,p1010-flexcan
> > 
> > You also can check imx6ul.dtsi imx7s.dtsi etc.
> > 
> > Sorry :-(, I also don't know the advantage, it's just that we're used
> > to writing it that way. You can check nodes of other devices.
> > It's unnecessary to add compatible string for each SoCs since they may
> > share the same IP. And dts had batter have a SoC specific compatible
> > string. It's just my understanding.
> 
> Ah thanks. So Pankaj's patch [1] seems to be correct (at least according
> to the description in the device tree documentation).
> 
> Shawn, whats your opinion?

My opinion is that all compatibles should be defined explicitly in
bindings doc.  In above example, the possible values of <processor>
should be given.  This must be done anyway, as we are moving to
json-schema bindings.

Shawn

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Hi Shawn,

Am 2020-02-17 08:13, schrieb Shawn Guo:
> On Fri, Feb 14, 2020 at 11:02:46AM +0100, Michael Walle wrote:
>> 
>> Hi Joakim, Hi Shawn,
>> 
>> 
>> Am 2020-02-14 10:56, schrieb Joakim Zhang:
>> > > -----Original Message-----
>> > > From: Michael Walle <michael@walle.cc>
>> > > Sent: 2020年2月14日 17:33
>> > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> > > Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> > > netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> > > <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
>> > > Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> > > Flexcan
>> > >
>> > > Am 2020-02-14 10:18, schrieb Joakim Zhang:
>> > > > Best Regards,
>> > > > Joakim Zhang
>> > > >
>> > > >> -----Original Message-----
>> > > >> From: Michael Walle <michael@walle.cc>
>> > > >> Sent: 2020年2月14日 16:43
>> > > >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> > > >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> > > >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> > > >> <pankaj.bansal@nxp.com>
>> > > >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> > > >> Flexcan
>> > > >>
>> > > >> Hi Joakim,
>> > > >>
>> > > >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
>> > > >> > Hi Michal,
>> > > >> >
>> > > >> >> -----Original Message-----
>> > > >> >> From: Michael Walle <michael@walle.cc>
>> > > >> >> Sent: 2020年2月14日 3:20
>> > > >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
>> > > >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
>> > > >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> > > >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
>> > > >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> > > >> >> Flexcan
>> > > >> >>
>> > > >> >> Hi,
>> > > >> >>
>> > > >> >> >>> Are you prepared to add back these patches as they are
>> > > >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
>> > > >> >> >>> set is based on these patches.
>> > > >> >> >>
>> > > >> >> >> Yes, these patches will be added back.
>> > > >> >> >
>> > > >> >> >I've cleaned up the first patch a bit, and pushed everything to
>> > > >> >> >the testing branch. Can you give it a test.
>> > > >> >>
>> > > >> >> What happend to that branch? FWIW I've just tried the patches on a
>> > > >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
>> > > >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
>> > > >> >> these patches upstream, because our board also offers CAN and
>> > > >> >> basic support for it just made it upstream [1].
>> > > >> > The FlexCAN CAN FD related patches have stayed in
>> > > >> > linux-can-next/flexcan branch for a long time, I still don't know
>> > > >> > why Marc doesn't merge them into Linux mainline.
>> > > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
>> > > >> >
>> > > >>
>> > > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.
>> > > >> g
>> > > >> >
>> > > >>
>> > > it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
>> > > >> xp.co
>> > > >> >
>> > > >>
>> > > m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
>> > > >> 5c30163
>> > > >> >
>> > > >>
>> > > 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
>> > > >> 8%2FdNSV3
>> > > >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
>> > > >> > Also must hope that this patch set can be upstreamed soon. :-)
>> > > >>
>> > > >> I've took them from this branch and applied them to the latest linux
>> > > >> master.
>> > > >>
>> > > >> Thus,
>> > > >>
>> > > >> Tested-by: Michael Walle <michael@walle.cc>
>> > > >>
>> > > >>
>> > > >> >> If these patches are upstream, only the device tree nodes seems to
>> > > >> >> be missing.
>> > > >> >> I don't know what has happened to [2]. But the patch doesn't seem
>> > > >> >> to be necessary.
>> > > >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
>> > > >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
>> > > >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
>> > > >> > But it is actually decided by SoC integration, for i.MX, the design
>> > > >> > is different.
>> > > >>
>> > > >> ok thanks for clarifying.
>> > > >>
>> > > >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
>> > > >> > dependency have not upstreamed yet.
>> > > >> >
>> > > >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
>> > > >> >> Thats basically the same I've used, only that mine didn't had the
>> > > >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
>> > > >> >> "lx2160ar1-flexcan"
>> > > >> >> which is the correct way to use it, right?
>> > > >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
>> > > >> > supports CAN FD, you can use this compatible string.
>> > > >>
>> > > >> correct. I've already a patch that does exactly this ;) Who would
>> > > >> take the patch for adding the LS1028A can device tree nodes to
>> > > >> ls1028a.dtsi? You or Shawn Guo?
>> > > > Sorry, I missed the link[3], we usually write it this way:
>> > > > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
>> > > > Please send patch to Shawn Guo, he will review the device tree.
>> > >
>> > > As far as I know, there should be no undocumented binding. Eg. the
>> > > ls1028ar1-flexcan is neither in the source nor in the device tree
>> > > binding
>> > > documentation, thus wouldn't be accepted.
>> > >
>> > > Thus either there should be another ls1028ar1-flexcan in the
>> > > flexcan_of_match
>> > > table and the node should only contain that string or the node
>> > > should only
>> > > contain fsl,lx2160ar1-flexcan. Is there any advantage of the first
>> > > option?
>> > From the FlexCAN
>> > binding(Documentation/devicetree/bindings/net/can/fsl-flexcan.txt)
>> > - compatible : Should be "fsl,<processor>-flexcan"
>> >
>> >   An implementation should also claim any of the following compatibles
>> >   that it is fully backwards compatible with:
>> >
>> >   - fsl,p1010-flexcan
>> >
>> > You also can check imx6ul.dtsi imx7s.dtsi etc.
>> >
>> > Sorry :-(, I also don't know the advantage, it's just that we're used
>> > to writing it that way. You can check nodes of other devices.
>> > It's unnecessary to add compatible string for each SoCs since they may
>> > share the same IP. And dts had batter have a SoC specific compatible
>> > string. It's just my understanding.
>> 
>> Ah thanks. So Pankaj's patch [1] seems to be correct (at least 
>> according
>> to the description in the device tree documentation).
>> 
>> Shawn, whats your opinion?
> 
> My opinion is that all compatibles should be defined explicitly in
> bindings doc.  In above example, the possible values of <processor>
> should be given.  This must be done anyway, as we are moving to
> json-schema bindings.

But if they are listed in the document, they also have to be in the
of_device_id table, correct? Which somehow contradicts the talk Pankaj
mentioned [1,2]. Eg.

   compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";

Doesn't make any sense, because the "fsl,ls1028ar1-flexcan" is alreay
in the driver and the fallback "fsl,lx2160ar1-flexcan" isn't needed.

OTOH the talk is already 2 to 3 years old and things might have changed
since then.

-michael

[1] https://elinux.org/images/0/0e/OSELAS.Presentation-ELCE2017-DT.pdf
[2] https://www.youtube.com/watch?v=6iguKSJJfxo

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Shawn Guo]

On Mon, Feb 17, 2020 at 09:48:36AM +0100, Michael Walle wrote:
> > My opinion is that all compatibles should be defined explicitly in
> > bindings doc.  In above example, the possible values of <processor>
> > should be given.  This must be done anyway, as we are moving to
> > json-schema bindings.
> 
> But if they are listed in the document, they also have to be in the
> of_device_id table, correct?

I do not think so.  Documenting compatibles used in DTS now doesn't
necessarily mean we need to use it in kernel driver right away.
Bindings doc is a specification for device tree, not kernel.  With the
compatible in DTS and bindings, kernel can start using it at any time
when there is a need, like dealing with SoC quirks or bugs found later.

Shawn

> Which somehow contradicts the talk Pankaj
> mentioned [1,2]. Eg.
> 
>   compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan";
> 
> Doesn't make any sense, because the "fsl,ls1028ar1-flexcan" is alreay
> in the driver and the fallback "fsl,lx2160ar1-flexcan" isn't needed.
> 
> OTOH the talk is already 2 to 3 years old and things might have changed
> since then.
> 
> -michael
> 
> [1] https://elinux.org/images/0/0e/OSELAS.Presentation-ELCE2017-DT.pdf
> [2] https://www.youtube.com/watch?v=6iguKSJJfxo

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Pankaj Bansal]

Hi Michael,

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: Friday, February 14, 2020 3:03 PM
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> Am 2020-02-14 10:18, schrieb Joakim Zhang:
> > Best Regards,
> > Joakim Zhang
> >
> >> -----Original Message-----
> >> From: Michael Walle <michael@walle.cc>
> >> Sent: 2020年2月14日 16:43
> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> >> <pankaj.bansal@nxp.com>
> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> >> Flexcan
> >>
> >> Hi Joakim,
> >>
> >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
> >> > Hi Michal,
> >> >
> >> >> -----Original Message-----
> >> >> From: Michael Walle <michael@walle.cc>
> >> >> Sent: 2020年2月14日 3:20
> >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
> >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> >> >> Flexcan
> >> >>
> >> >> Hi,
> >> >>
> >> >> >>> Are you prepared to add back these patches as they are
> >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
> >> >> >>> set is based on these patches.
> >> >> >>
> >> >> >> Yes, these patches will be added back.
> >> >> >
> >> >> >I've cleaned up the first patch a bit, and pushed everything to
> >> >> >the testing branch. Can you give it a test.
> >> >>
> >> >> What happend to that branch? FWIW I've just tried the patches on a
> >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
> >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
> >> >> these patches upstream, because our board also offers CAN and
> >> >> basic support for it just made it upstream [1].
> >> > The FlexCAN CAN FD related patches have stayed in
> >> > linux-can-next/flexcan branch for a long time, I still don't know
> >> > why Marc doesn't merge them into Linux mainline.
> >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> >> >
> >> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-
> next.
> >> g
> >> >
> >>
> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
> >> xp.co
> >> >
> >>
> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
> >> 5c30163
> >> >
> >> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
> >> 8%2FdNSV3
> >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
> >> > Also must hope that this patch set can be upstreamed soon. :-)
> >>
> >> I've took them from this branch and applied them to the latest linux
> >> master.
> >>
> >> Thus,
> >>
> >> Tested-by: Michael Walle <michael@walle.cc>
> >>
> >>
> >> >> If these patches are upstream, only the device tree nodes seems to
> >> >> be missing.
> >> >> I don't know what has happened to [2]. But the patch doesn't seem
> >> >> to be necessary.
> >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
> >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
> >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
> >> > But it is actually decided by SoC integration, for i.MX, the design
> >> > is different.
> >>
> >> ok thanks for clarifying.
> >>
> >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
> >> > dependency have not upstreamed yet.
> >> >
> >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
> >> >> Thats basically the same I've used, only that mine didn't had the
> >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
> >> >> "lx2160ar1-flexcan"
> >> >> which is the correct way to use it, right?
> >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
> >> > supports CAN FD, you can use this compatible string.
> >>
> >> correct. I've already a patch that does exactly this ;) Who would
> >> take the patch for adding the LS1028A can device tree nodes to
> >> ls1028a.dtsi? You or Shawn Guo?
> > Sorry, I missed the link[3], we usually write it this way:
> > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-
> flexcan";
> > Please send patch to Shawn Guo, he will review the device tree.
> 
> As far as I know, there should be no undocumented binding. Eg. the ls1028ar1-
> flexcan is neither in the source nor in the device tree binding documentation,
> thus wouldn't be accepted.
> 
> Thus either there should be another ls1028ar1-flexcan in the flexcan_of_match
> table and the node should only contain that string or the node should only
> contain fsl,lx2160ar1-flexcan. Is there any advantage of the first option?
> 

This is done to ensure that device tree bindings are stable.
See this talk for more information for stable device tree ABI:

https://elinux.org/images/0/0e/OSELAS.Presentation-ELCE2017-DT.pdf
https://www.youtube.com/watch?v=6iguKSJJfxo

> 
> -michael
> 
> 
> >
> >> > static const struct of_device_id flexcan_of_match[] = {
> >> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
> >> > &fsl_imx8qm_devtype_data, },
> >> > 	{ .compatible = "fsl,imx6q-flexcan", .data =
> >> > &fsl_imx6q_devtype_data, },
> >> > 	{ .compatible = "fsl,imx28-flexcan", .data =
> >> > &fsl_imx28_devtype_data, },
> >> > 	{ .compatible = "fsl,imx53-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,imx35-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,imx25-flexcan", .data =
> >> > &fsl_imx25_devtype_data, },
> >> > 	{ .compatible = "fsl,p1010-flexcan", .data =
> >> > &fsl_p1010_devtype_data, },
> >> > 	{ .compatible = "fsl,vf610-flexcan", .data =
> >> > &fsl_vf610_devtype_data, },
> >> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
> >> > &fsl_ls1021a_r2_devtype_data, },
> >> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
> >> > &fsl_lx2160a_r1_devtype_data, },
> >> > 	{ /* sentinel */ },
> >> > };
> >> >
> >>
> >> -michael

Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Michael Walle]

Am 2020-02-14 11:01, schrieb Pankaj Bansal:
> Hi Michael,
> 
>> -----Original Message-----
>> From: Michael Walle <michael@walle.cc>
>> Sent: Friday, February 14, 2020 3:03 PM
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> <pankaj.bansal@nxp.com>; Shawn Guo <shawnguo@kernel.org>
>> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP 
>> Flexcan
>> 
>> Am 2020-02-14 10:18, schrieb Joakim Zhang:
>> > Best Regards,
>> > Joakim Zhang
>> >
>> >> -----Original Message-----
>> >> From: Michael Walle <michael@walle.cc>
>> >> Sent: 2020年2月14日 16:43
>> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> >> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
>> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> >> <pankaj.bansal@nxp.com>
>> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> >> Flexcan
>> >>
>> >> Hi Joakim,
>> >>
>> >> Am 2020-02-14 02:55, schrieb Joakim Zhang:
>> >> > Hi Michal,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Michael Walle <michael@walle.cc>
>> >> >> Sent: 2020年2月14日 3:20
>> >> >> To: Marc Kleine-Budde <mkl@pengutronix.de>
>> >> >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
>> >> >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
>> >> >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
>> >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
>> >> >> Flexcan
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> >>> Are you prepared to add back these patches as they are
>> >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch
>> >> >> >>> set is based on these patches.
>> >> >> >>
>> >> >> >> Yes, these patches will be added back.
>> >> >> >
>> >> >> >I've cleaned up the first patch a bit, and pushed everything to
>> >> >> >the testing branch. Can you give it a test.
>> >> >>
>> >> >> What happend to that branch? FWIW I've just tried the patches on a
>> >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
>> >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see
>> >> >> these patches upstream, because our board also offers CAN and
>> >> >> basic support for it just made it upstream [1].
>> >> > The FlexCAN CAN FD related patches have stayed in
>> >> > linux-can-next/flexcan branch for a long time, I still don't know
>> >> > why Marc doesn't merge them into Linux mainline.
>> >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
>> >> >
>> >> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-
>> next.
>> >> g
>> >> >
>> >>
>> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
>> >> xp.co
>> >> >
>> >>
>> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
>> >> 5c30163
>> >> >
>> >> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
>> >> 8%2FdNSV3
>> >> > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
>> >> > Also must hope that this patch set can be upstreamed soon. :-)
>> >>
>> >> I've took them from this branch and applied them to the latest linux
>> >> master.
>> >>
>> >> Thus,
>> >>
>> >> Tested-by: Michael Walle <michael@walle.cc>
>> >>
>> >>
>> >> >> If these patches are upstream, only the device tree nodes seems to
>> >> >> be missing.
>> >> >> I don't know what has happened to [2]. But the patch doesn't seem
>> >> >> to be necessary.
>> >> > Yes, this patch is unnecessary. I have NACKed this patch for that,
>> >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select
>> >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock.
>> >> > But it is actually decided by SoC integration, for i.MX, the design
>> >> > is different.
>> >>
>> >> ok thanks for clarifying.
>> >>
>> >> > I have not upstream i.MX FlexCAN device tree nodes, since it's
>> >> > dependency have not upstreamed yet.
>> >> >
>> >> >> Pankaj already send a patch to add the device node to the LS1028A [3].
>> >> >> Thats basically the same I've used, only that mine didn't had the
>> >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the
>> >> >> "lx2160ar1-flexcan"
>> >> >> which is the correct way to use it, right?
>> >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan"
>> >> > supports CAN FD, you can use this compatible string.
>> >>
>> >> correct. I've already a patch that does exactly this ;) Who would
>> >> take the patch for adding the LS1028A can device tree nodes to
>> >> ls1028a.dtsi? You or Shawn Guo?
>> > Sorry, I missed the link[3], we usually write it this way:
>> > 			compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-
>> flexcan";
>> > Please send patch to Shawn Guo, he will review the device tree.
>> 
>> As far as I know, there should be no undocumented binding. Eg. the 
>> ls1028ar1-
>> flexcan is neither in the source nor in the device tree binding 
>> documentation,
>> thus wouldn't be accepted.
>> 
>> Thus either there should be another ls1028ar1-flexcan in the 
>> flexcan_of_match
>> table and the node should only contain that string or the node should 
>> only
>> contain fsl,lx2160ar1-flexcan. Is there any advantage of the first 
>> option?
>> 
> 
> This is done to ensure that device tree bindings are stable.
> See this talk for more information for stable device tree ABI:

Thanks, yes that makes sense, actually I'd have done it the same way. I 
was
just wondering about the "it has to be documented"; as Joakim pointed 
out, thats
easy here, because there is the template here, eg. 
"fsl,<processor>-flexcan".

-michael

> 
> https://elinux.org/images/0/0e/OSELAS.Presentation-ELCE2017-DT.pdf
> https://www.youtube.com/watch?v=6iguKSJJfxo
> 
>> 
>> -michael
>> 
>> 
>> >
>> >> > static const struct of_device_id flexcan_of_match[] = {
>> >> > 	{ .compatible = "fsl,imx8qm-flexcan", .data =
>> >> > &fsl_imx8qm_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx6q-flexcan", .data =
>> >> > &fsl_imx6q_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx28-flexcan", .data =
>> >> > &fsl_imx28_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx53-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx35-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,imx25-flexcan", .data =
>> >> > &fsl_imx25_devtype_data, },
>> >> > 	{ .compatible = "fsl,p1010-flexcan", .data =
>> >> > &fsl_p1010_devtype_data, },
>> >> > 	{ .compatible = "fsl,vf610-flexcan", .data =
>> >> > &fsl_vf610_devtype_data, },
>> >> > 	{ .compatible = "fsl,ls1021ar2-flexcan", .data =
>> >> > &fsl_ls1021a_r2_devtype_data, },
>> >> > 	{ .compatible = "fsl,lx2160ar1-flexcan", .data =
>> >> > &fsl_lx2160a_r1_devtype_data, },
>> >> > 	{ /* sentinel */ },
>> >> > };
>> >> >
>> >>
>> >> -michael

RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan

[Joakim Zhang]

> -----Original Message-----
> From: linux-can-owner@vger.kernel.org <linux-can-owner@vger.kernel.org>
> On Behalf Of Joakim Zhang
> Sent: 2020年2月14日 17:18
> To: Michael Walle <michael@walle.cc>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>
> Subject: RE: [PATCH 0/8] can: flexcan: add CAN FD support for NXP Flexcan
> 
> 
> 
> Best Regards,
> Joakim Zhang
> 
> > -----Original Message-----
> > From: Michael Walle <michael@walle.cc>
> > Sent: 2020年2月14日 16:43
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: Marc Kleine-Budde <mkl@pengutronix.de>; wg@grandegger.com;
> > netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> > <pankaj.bansal@nxp.com>
> > Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> > Flexcan
> >
> > Hi Joakim,
> >
> > Am 2020-02-14 02:55, schrieb Joakim Zhang:
> > > Hi Michal,
> > >
> > >> -----Original Message-----
> > >> From: Michael Walle <michael@walle.cc>
> > >> Sent: 2020年2月14日 3:20
> > >> To: Marc Kleine-Budde <mkl@pengutronix.de>
> > >> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; wg@grandegger.com;
> > >> netdev@vger.kernel.org; linux-can@vger.kernel.org; Pankaj Bansal
> > >> <pankaj.bansal@nxp.com>; Michael Walle <michael@walle.cc>
> > >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP
> > >> Flexcan
> > >>
> > >> Hi,
> > >>
> > >> >>> Are you prepared to add back these patches as they are
> > >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch set
> > >> >>> is based on these patches.
> > >> >>
> > >> >> Yes, these patches will be added back.
> > >> >
> > >> >I've cleaned up the first patch a bit, and pushed everything to
> > >> >the testing branch. Can you give it a test.
> > >>
> > >> What happend to that branch? FWIW I've just tried the patches on a
> > >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working.
> > >> I've tested against a Peaktech USB CAN adapter. I'd love to see
> > >> these patches upstream, because our board also offers CAN and basic
> > >> support for it just made it upstream [1].
> > > The FlexCAN CAN FD related patches have stayed in
> > > linux-can-next/flexcan branch for a long time, I still don't know
> > > why Marc doesn't merge them into Linux mainline.
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> > >
> >
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next.g
> > >
> >
> it%2Ftree%2F%3Fh%3Dflexcan&amp;data=02%7C01%7Cqiangqing.zhang%40n
> > xp.co
> > >
> >
> m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c
> > 5c30163
> > >
> >
> 5%7C0%7C0%7C637172665642079192&amp;sdata=77tG6VuQCi%2FZXBKb23
> > 8%2FdNSV3
> > > NUIFrM5Y0e9yj0J3os%3D&amp;reserved=0
> > > Also must hope that this patch set can be upstreamed soon. :-)
> >
> > I've took them from this branch and applied them to the latest linux master.
> >
> > Thus,
> >
> > Tested-by: Michael Walle <michael@walle.cc>
> >
> >
Hi Marc,

How about FlexCAN CAN FD patch set, when will you send to mainline?

Thanks a lot!

Best Regards,
Joakim Zhang