* [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x
@ 2017-03-10 14:00 Mario Huettel
2017-03-10 14:00 ` [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > 3.0.x Mario Huettel
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Mario Huettel @ 2017-03-10 14:00 UTC (permalink / raw)
To: linux-can
This patch adds support for all newer versions of the Bosch M_CAN IP Module.
The defines for registers, bits and masks are adapted to be compatible with the
newer versions. They are named accordingl to the most recent user's manual for
M_CAN version v3.2.1.
All initialization routines check the core release version and set the registers
accordingly.
The newly added support for versions >= v3.1.x includes a major functionality
change: Instead of only one TX Buffer the whole TX FIFO is used. This is now
possible, because the configuration whether a frame is a CAN-FD Frame and the
BRS configuration are now done in the Buffer/FIFO Element of the message and not
globally via a config register as before. this implies that the functionality
change does not work for the M_CAN versions v3.0.x.
The driver reserves as many echo_skb slots as there are slots in the TX FIFO of
the M_CAN. Sending a frame is done by getting the index from the M_CAN that
determines in which slot the message shall be written. The echo_skb is written
to the same index in the Linux-internal list. On top of that, the message marker
of the message is set to the index.
Once the message is sent, the message marker (=index) is written into the TX
Event FIFO. The interrupt service routine reads this marker and uses it to echo
the correct echo skb back to the network stack.
Testen on Altera Cyclone V SoC FPGA using M_CAN v3.2.1 (CREL=0x32150320).
Mario Huettel (3):
can: m_can: Support M_CAN IP versions > v3.0.x
can: m_can: Enable M_CAN IP version dependent initialization
can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
drivers/net/can/m_can/m_can.c | 801 +++++++++++++++++++++++++++++++++---------
1 file changed, 639 insertions(+), 162 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > 3.0.x
2017-03-10 14:00 [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x Mario Huettel
@ 2017-03-10 14:00 ` Mario Huettel
2017-03-10 14:00 ` [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x Mario Huettel
` (2 subsequent siblings)
3 siblings, 0 replies; 22+ messages in thread
From: Mario Huettel @ 2017-03-10 14:00 UTC (permalink / raw)
To: linux-can
This patch adds support for all newer versions of the Bosch M_CAN IP Module.
The defines for registers, bits and masks are adapted to be compatible with the
newer versions. They are named accordingl to the most recent user's manual for
M_CAN version v3.2.1.
All initialization routines check the core release version and set the registers
accordingly.
The newly added support for versions >= v3.1.x includes a major functionality
change: Instead of only one TX Buffer the whole TX FIFO is used. This is now
possible, because the configuration whether a frame is a CAN-FD Frame and the
BRS configuration are now done in the Buffer/FIFO Element of the message and not
globally via a config register as before. this implies that the functionality
change does not work for the M_CAN versions v3.0.x.
The driver reserves as many echo_skb slots as there are slots in the TX FIFO of
the M_CAN. Sending a frame is done by getting the index from the M_CAN that
determines in which slot the message shall be written. The echo_skb is written
to the same index in the Linux-internal list. On top of that, the message marker
of the message is set to the index.
Once the message is sent, the message marker (=index) is written into the TX
Event FIFO. The interrupt service routine reads this marker and uses it to echo
the correct echo skb back to the network stack.
Testen on Altera Cyclone V SoC FPGA using M_CAN v3.2.1 (CREL=0x32150320).
Mario Huettel (3):
can: m_can: Support M_CAN IP versions > v3.0.x
can: m_can: Enable M_CAN IP version dependent initialization
can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
drivers/net/can/m_can/m_can.c | 801 +++++++++++++++++++++++++++++++++---------
1 file changed, 639 insertions(+), 162 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x
2017-03-10 14:00 [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x Mario Huettel
2017-03-10 14:00 ` [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > 3.0.x Mario Huettel
@ 2017-03-10 14:00 ` Mario Huettel
2017-03-16 8:41 ` Oliver Hartkopp
2017-03-21 10:36 ` Marc Kleine-Budde
2017-03-10 14:00 ` [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization Mario Huettel
2017-03-10 14:00 ` [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x Mario Huettel
3 siblings, 2 replies; 22+ messages in thread
From: Mario Huettel @ 2017-03-10 14:00 UTC (permalink / raw)
To: linux-can
This patch includes following changes:
* Renamed the register defines of the M_CAN to fit version 3.1.x and above.
* Replaced the old defines with the new ones in the whole code.
* Removed code that enabled interrupt line 1. The driver didn't use it.
* Removed initialization of FIFO water marks. They were not used.
Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
---
drivers/net/can/m_can/m_can.c | 197 +++++++++++++++++++++++++++---------------
1 file changed, 129 insertions(+), 68 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 7a6554e..f2656a3 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -37,17 +37,19 @@ enum m_can_reg {
M_CAN_CREL = 0x0,
M_CAN_ENDN = 0x4,
M_CAN_CUST = 0x8,
- M_CAN_FBTP = 0xc,
+ M_CAN_DBTP = 0xc,
M_CAN_TEST = 0x10,
M_CAN_RWD = 0x14,
M_CAN_CCCR = 0x18,
- M_CAN_BTP = 0x1c,
+ M_CAN_NBTP = 0x1c,
M_CAN_TSCC = 0x20,
M_CAN_TSCV = 0x24,
M_CAN_TOCC = 0x28,
M_CAN_TOCV = 0x2c,
M_CAN_ECR = 0x40,
M_CAN_PSR = 0x44,
+/* TDCR Register only available for version >=3.1.x */
+ M_CAN_TDCR = 0x48,
M_CAN_IR = 0x50,
M_CAN_IE = 0x54,
M_CAN_ILS = 0x58,
@@ -105,21 +107,21 @@ enum m_can_mram_cfg {
MRAM_CFG_NUM,
};
-/* Fast Bit Timing & Prescaler Register (FBTP) */
-#define FBTR_FBRP_MASK 0x1f
-#define FBTR_FBRP_SHIFT 16
-#define FBTR_FTSEG1_SHIFT 8
-#define FBTR_FTSEG1_MASK (0xf << FBTR_FTSEG1_SHIFT)
-#define FBTR_FTSEG2_SHIFT 4
-#define FBTR_FTSEG2_MASK (0x7 << FBTR_FTSEG2_SHIFT)
-#define FBTR_FSJW_SHIFT 0
-#define FBTR_FSJW_MASK 0x3
+/* Data Bit Timing & Prescaler Register (DBTP) */
+#define DBTP_TDC BIT(23)
+#define DBTP_DBRP_SHIFT 16
+#define DBTP_DBRP_MASK (0x1f << DBTP_DBRP_SHIFT)
+#define DBTP_DTSEG1_SHIFT 8
+#define DBTP_DTSEG1_MASK (0x1f << DBTP_DTSEG1_SHIFT)
+#define DBTP_DTSEG2_SHIFT 4
+#define DBTP_DTSEG2_MASK (0xf << DBTP_DTSEG2_SHIFT)
+#define DBTP_DSJW_SHIFT 0
+#define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
/* Test Register (TEST) */
-#define TEST_LBCK BIT(4)
+#define TEST_LBCK BIT(4)
/* CC Control Register(CCCR) */
-#define CCCR_TEST BIT(7)
#define CCCR_CMR_MASK 0x3
#define CCCR_CMR_SHIFT 10
#define CCCR_CMR_CANFD 0x1
@@ -130,21 +132,32 @@ enum m_can_mram_cfg {
#define CCCR_CME_CAN 0
#define CCCR_CME_CANFD 0x1
#define CCCR_CME_CANFD_BRS 0x2
+#define CCCR_TXP BIT(14)
#define CCCR_TEST BIT(7)
#define CCCR_MON BIT(5)
+#define CCCR_CSR BIT(4)
+#define CCCR_CSA BIT(3)
+#define CCCR_ASM BIT(2)
#define CCCR_CCE BIT(1)
#define CCCR_INIT BIT(0)
#define CCCR_CANFD 0x10
-
-/* Bit Timing & Prescaler Register (BTP) */
-#define BTR_BRP_MASK 0x3ff
-#define BTR_BRP_SHIFT 16
-#define BTR_TSEG1_SHIFT 8
-#define BTR_TSEG1_MASK (0x3f << BTR_TSEG1_SHIFT)
-#define BTR_TSEG2_SHIFT 4
-#define BTR_TSEG2_MASK (0xf << BTR_TSEG2_SHIFT)
-#define BTR_SJW_SHIFT 0
-#define BTR_SJW_MASK 0xf
+/* for version >=3.1.x */
+#define CCCR_EFBI BIT(13)
+#define CCCR_PXHD BIT(12)
+#define CCCR_BRSE BIT(9)
+#define CCCR_FDOE BIT(8)
+/* only for version >=3.2.x */
+#define CCCR_NISO BIT(15)
+
+/* Nominal Bit Timing & Prescaler Register (NBTP) */
+#define NBTP_NSJW_SHIFT 25
+#define NBTP_NSJW_MASK (0x7f << NBTP_NSJW_SHIFT)
+#define NBTP_NBRP_SHIFT 16
+#define NBTP_NBRP_MASK (0x1ff << NBTP_NBRP_SHIFT)
+#define NBTP_NTSEG1_SHIFT 8
+#define NBTP_NTSEG1_MASK (0xff << NBTP_NTSEG1_SHIFT)
+#define NBTP_NTSEG2_SHIFT 0
+#define NBTP_NTSEG2_MASK (0x7f << NBTP_NTSEG2_SHIFT)
/* Error Counter Register(ECR) */
#define ECR_RP BIT(15)
@@ -161,6 +174,13 @@ enum m_can_mram_cfg {
/* Interrupt Register(IR) */
#define IR_ALL_INT 0xffffffff
+
+/* Renamed bits for versions > 3.1.x */
+#define IR_ARA BIT(29)
+#define IR_PED BIT(28)
+#define IR_PEA BIT(27)
+
+/* Bits for version 3.0.x */
#define IR_STE BIT(31)
#define IR_FOE BIT(30)
#define IR_ACKE BIT(29)
@@ -194,33 +214,40 @@ enum m_can_mram_cfg {
#define IR_RF0W BIT(1)
#define IR_RF0N BIT(0)
#define IR_ERR_STATE (IR_BO | IR_EW | IR_EP)
-#define IR_ERR_LEC (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
-#define IR_ERR_BUS (IR_ERR_LEC | IR_WDI | IR_ELO | IR_BEU | \
+
+/* Interrupts for version 3.0.x */
+#define IR_ERR_LEC_30X (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
+#define IR_ERR_BUS_30X (IR_ERR_LEC_30X | IR_WDI | IR_ELO | IR_BEU | \
+ IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
+ IR_RF1L | IR_RF0L)
+#define IR_ERR_ALL_30X (IR_ERR_STATE | IR_ERR_BUS_30X)
+/* Interrupts for version >= 3.1.x */
+#define IR_ERR_LEC_31X (IR_PED | IR_PEA)
+#define IR_ERR_BUS_31X (IR_ERR_LEC_31X | IR_WDI | IR_ELO | IR_BEU | \
IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
IR_RF1L | IR_RF0L)
-#define IR_ERR_ALL (IR_ERR_STATE | IR_ERR_BUS)
+#define IR_ERR_ALL_31X (IR_ERR_STATE | IR_ERR_BUS_31X)
/* Interrupt Line Select (ILS) */
#define ILS_ALL_INT0 0x0
#define ILS_ALL_INT1 0xFFFFFFFF
/* Interrupt Line Enable (ILE) */
-#define ILE_EINT0 BIT(0)
#define ILE_EINT1 BIT(1)
+#define ILE_EINT0 BIT(0)
/* Rx FIFO 0/1 Configuration (RXF0C/RXF1C) */
-#define RXFC_FWM_OFF 24
-#define RXFC_FWM_MASK 0x7f
-#define RXFC_FWM_1 (1 << RXFC_FWM_OFF)
-#define RXFC_FS_OFF 16
-#define RXFC_FS_MASK 0x7f
+#define RXFC_FWM_SHIFT 24
+#define RXFC_FWM_MASK (0x7f < RXFC_FWM_SHIFT)
+#define RXFC_FS_SHIFT 16
+#define RXFC_FS_MASK (0x7f << RXFC_FS_SHIFT)
/* Rx FIFO 0/1 Status (RXF0S/RXF1S) */
#define RXFS_RFL BIT(25)
#define RXFS_FF BIT(24)
-#define RXFS_FPI_OFF 16
+#define RXFS_FPI_SHIFT 16
#define RXFS_FPI_MASK 0x3f0000
-#define RXFS_FGI_OFF 8
+#define RXFS_FGI_SHIFT 8
#define RXFS_FGI_MASK 0x3f00
#define RXFS_FFL_MASK 0x7f
@@ -229,23 +256,46 @@ enum m_can_mram_cfg {
#define M_CAN_RXESC_64BYTES 0x777
/* Tx Buffer Configuration(TXBC) */
-#define TXBC_NDTB_OFF 16
-#define TXBC_NDTB_MASK 0x3f
+#define TXBC_NDTB_SHIFT 16
+#define TXBC_NDTB_MASK (0x3f << TXBC_NDTB_SHIFT)
+#define TXBC_TFQS_SHIFT 24
+#define TXBC_TFQS_MASK (0x3f << TXBC_TFQS_SHIFT)
+
+/* Tx FIFO/Queue Status (TXFQS) */
+#define TXFQS_TFQF BIT(21)
+#define TXFQS_TFQPI_SHIFT 16
+#define TXFQS_TFQPI_MASK (0x1f << TXFQS_TFQPI_SHIFT)
+#define TXFQS_TFGI_SHIFT 8
+#define TXFQS_TFGI_MASK (0x1f << TXFQS_TFGI_SHIFT)
+#define TXFQS_TFFL_SHIFT 0
+#define TXFQS_TFFL_MASK (0x3f << TXFQS_TFFL_SHIFT)
/* Tx Buffer Element Size Configuration(TXESC) */
#define TXESC_TBDS_8BYTES 0x0
#define TXESC_TBDS_64BYTES 0x7
-/* Tx Event FIFO Con.guration (TXEFC) */
-#define TXEFC_EFS_OFF 16
-#define TXEFC_EFS_MASK 0x3f
+/* Tx Event FIFO Configuration (TXEFC) */
+#define TXEFC_EFS_SHIFT 16
+#define TXEFC_EFS_MASK (0x3f << TXEFC_EFS_SHIFT)
+
+/* Tx Event FIFO Status (TXEFS) */
+#define TXEFS_TEFL BIT(25)
+#define TXEFS_EFF BIT(24)
+#define TXEFS_EFGI_SHIFT 8
+#define TXEFS_EFGI_MASK (0x1f << TXEFS_EFGI_SHIFT)
+#define TXEFS_EFFL_SHIFT 0
+#define TXEFS_EFFL_MASK (0x3f << TXEFS_EFFL_SHIFT)
+
+/* Tx Event FIFO Acknowledge (TXEFA) */
+#define TXEFA_EFAI_SHIFT 0
+#define TXEFA_EFAI_MASK (0x1f << TXEFA_EFAI_SHIFT)
/* Message RAM Configuration (in bytes) */
#define SIDF_ELEMENT_SIZE 4
#define XIDF_ELEMENT_SIZE 8
#define RXF0_ELEMENT_SIZE 72
#define RXF1_ELEMENT_SIZE 72
-#define RXB_ELEMENT_SIZE 16
+#define RXB_ELEMENT_SIZE 72
#define TXE_ELEMENT_SIZE 8
#define TXB_ELEMENT_SIZE 72
@@ -261,13 +311,20 @@ enum m_can_mram_cfg {
#define RX_BUF_RTR BIT(29)
/* R1 */
#define RX_BUF_ANMF BIT(31)
-#define RX_BUF_EDL BIT(21)
+#define RX_BUF_FDF BIT(21)
#define RX_BUF_BRS BIT(20)
/* Tx Buffer Element */
-/* R0 */
+/* T0 */
+#define TX_BUF_ESI BIT(31)
#define TX_BUF_XTD BIT(30)
#define TX_BUF_RTR BIT(29)
+/* T1 */
+#define TX_BUF_EFC BIT(23)
+#define TX_BUF_FDF BIT(21)
+#define TX_BUF_BRS BIT(20)
+#define TX_BUF_MM_SHIFT 24
+#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
/* address offset and element number for each FIFO/Buffer in the Message RAM */
struct mram_cfg {
@@ -349,7 +406,8 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,
static inline void m_can_enable_all_interrupts(const struct m_can_priv *priv)
{
- m_can_write(priv, M_CAN_ILE, ILE_EINT0 | ILE_EINT1);
+ /* Only interrupt line 0 is used in this driver */
+ m_can_write(priv, M_CAN_ILE, ILE_EINT0);
}
static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)
@@ -367,9 +425,9 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
int i;
/* calculate the fifo get index for where to read data */
- fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
+ fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_SHIFT;
dlc = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
- if (dlc & RX_BUF_EDL)
+ if (dlc & RX_BUF_FDF)
skb = alloc_canfd_skb(dev, &cf);
else
skb = alloc_can_skb(dev, (struct can_frame **)&cf);
@@ -378,7 +436,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
return;
}
- if (dlc & RX_BUF_EDL)
+ if (dlc & RX_BUF_FDF)
cf->len = can_dlc2len((dlc >> 16) & 0x0F);
else
cf->len = get_can_dlc((dlc >> 16) & 0x0F);
@@ -394,7 +452,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
netdev_dbg(dev, "ESI Error\n");
}
- if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
+ if (!(dlc & RX_BUF_FDF) && (id & RX_BUF_RTR)) {
cf->can_id |= CAN_RTR_FLAG;
} else {
if (dlc & RX_BUF_BRS)
@@ -532,7 +590,7 @@ static int __m_can_get_berr_counter(const struct net_device *dev,
ecr = m_can_read(priv, M_CAN_ECR);
bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
- bec->txerr = ecr & ECR_TEC_MASK;
+ bec->txerr = (ecr & ECR_TEC_MASK) >> ECR_TEC_SHIFT;
return 0;
}
@@ -723,7 +781,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
if (irqstatus & IR_ERR_STATE)
work_done += m_can_handle_state_errors(dev, psr);
- if (irqstatus & IR_ERR_BUS)
+ if (irqstatus & IR_ERR_BUS_30X)
work_done += m_can_handle_bus_errors(dev, irqstatus, psr);
if (irqstatus & IR_RF0N)
@@ -758,7 +816,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
* - state change IRQ
* - bus error IRQ and bus error reporting
*/
- if ((ir & IR_RF0N) || (ir & IR_ERR_ALL)) {
+ if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
priv->irqstatus = ir;
m_can_disable_all_interrupts(priv);
napi_schedule(&priv->napi);
@@ -811,19 +869,19 @@ static int m_can_set_bittiming(struct net_device *dev)
sjw = bt->sjw - 1;
tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
tseg2 = bt->phase_seg2 - 1;
- reg_btp = (brp << BTR_BRP_SHIFT) | (sjw << BTR_SJW_SHIFT) |
- (tseg1 << BTR_TSEG1_SHIFT) | (tseg2 << BTR_TSEG2_SHIFT);
- m_can_write(priv, M_CAN_BTP, reg_btp);
+ reg_btp = (brp << NBTP_NBRP_SHIFT) | (sjw << NBTP_NSJW_SHIFT) |
+ (tseg1 << NBTP_NTSEG1_SHIFT) | (tseg2 << NBTP_NTSEG2_SHIFT);
+ m_can_write(priv, M_CAN_NBTP, reg_btp);
if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
brp = dbt->brp - 1;
sjw = dbt->sjw - 1;
tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
tseg2 = dbt->phase_seg2 - 1;
- reg_btp = (brp << FBTR_FBRP_SHIFT) | (sjw << FBTR_FSJW_SHIFT) |
- (tseg1 << FBTR_FTSEG1_SHIFT) |
- (tseg2 << FBTR_FTSEG2_SHIFT);
- m_can_write(priv, M_CAN_FBTP, reg_btp);
+ reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
+ (tseg1 << DBTP_DTSEG1_SHIFT) |
+ (tseg2 << DBTP_DTSEG2_SHIFT);
+ m_can_write(priv, M_CAN_DBTP, reg_btp);
}
return 0;
@@ -851,23 +909,23 @@ static void m_can_chip_config(struct net_device *dev)
m_can_write(priv, M_CAN_GFC, 0x0);
/* only support one Tx Buffer currently */
- m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_OFF) |
+ m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
priv->mcfg[MRAM_TXB].off);
/* support 64 bytes payload */
m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
- m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_OFF) |
+ m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
priv->mcfg[MRAM_TXE].off);
/* rx fifo configuration, blocking mode, fifo size 1 */
m_can_write(priv, M_CAN_RXF0C,
- (priv->mcfg[MRAM_RXF0].num << RXFC_FS_OFF) |
- RXFC_FWM_1 | priv->mcfg[MRAM_RXF0].off);
+ (priv->mcfg[MRAM_RXF0].num << RXFC_FS_SHIFT) |
+ priv->mcfg[MRAM_RXF0].off);
m_can_write(priv, M_CAN_RXF1C,
- (priv->mcfg[MRAM_RXF1].num << RXFC_FS_OFF) |
- RXFC_FWM_1 | priv->mcfg[MRAM_RXF1].off);
+ (priv->mcfg[MRAM_RXF1].num << RXFC_FS_SHIFT) |
+ priv->mcfg[MRAM_RXF1].off);
cccr = m_can_read(priv, M_CAN_CCCR);
cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
@@ -892,7 +950,7 @@ static void m_can_chip_config(struct net_device *dev)
/* enable interrupts */
m_can_write(priv, M_CAN_IR, IR_ALL_INT);
if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
- m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC);
+ m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
else
m_can_write(priv, M_CAN_IE, IR_ALL_INT);
@@ -1143,10 +1201,12 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
priv->mcfg[MRAM_XIDF].num = out_val[2];
priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
- priv->mcfg[MRAM_RXF0].num = out_val[3] & RXFC_FS_MASK;
+ priv->mcfg[MRAM_RXF0].num = out_val[3] &
+ (RXFC_FS_MASK >> RXFC_FS_SHIFT);
priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
- priv->mcfg[MRAM_RXF1].num = out_val[4] & RXFC_FS_MASK;
+ priv->mcfg[MRAM_RXF1].num = out_val[4] &
+ (RXFC_FS_MASK >> RXFC_FS_SHIFT);
priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
priv->mcfg[MRAM_RXB].num = out_val[5];
@@ -1155,7 +1215,8 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
priv->mcfg[MRAM_TXE].num = out_val[6];
priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
- priv->mcfg[MRAM_TXB].num = out_val[7] & TXBC_NDTB_MASK;
+ priv->mcfg[MRAM_TXB].num = out_val[7] &
+ (TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
priv->mram_base,
@@ -1191,7 +1252,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
hclk = devm_clk_get(&pdev->dev, "hclk");
cclk = devm_clk_get(&pdev->dev, "cclk");
if (IS_ERR(hclk) || IS_ERR(cclk)) {
- dev_err(&pdev->dev, "no clock find\n");
+ dev_err(&pdev->dev, "no clock found\n");
return -ENODEV;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-10 14:00 [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x Mario Huettel
2017-03-10 14:00 ` [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > 3.0.x Mario Huettel
2017-03-10 14:00 ` [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x Mario Huettel
@ 2017-03-10 14:00 ` Mario Huettel
2017-03-16 8:52 ` Oliver Hartkopp
2017-03-21 14:33 ` Marc Kleine-Budde
2017-03-10 14:00 ` [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x Mario Huettel
3 siblings, 2 replies; 22+ messages in thread
From: Mario Huettel @ 2017-03-10 14:00 UTC (permalink / raw)
To: linux-can
This patch adapts the initialization of the M_CAN. So it can be used with
all versions >= 3.0.x.
Changes:
* Implemented new enum that represents the supported core versions.
* Added version element to m_can_priv structure to hold M_CAN version.
* Renamed bittiming structs for version 3.0.x
* Added new bittiming structs for version >= 3.1.x
* Function alloc_m_can_dev takes 2 new arguments. The TX FIFO size and the
base address of the module.
* Versions >= 3.1.x use multiple echo_skb buffers. This depends on the
TX FIFO size configured in the device tree. Because this info is needed
by alloc_m_can_dev, parts of the M_RAM configuration are moved before
the call of alloc_m_can_dev. This also changes the parameters of the
m_can_of_parse_mram function.
* Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
CCCR_MON bit. In combination with TEST_LBCK it activates the internal
loopback mode. Leaving CCCR_MON '0' results in external loopback mode.
* Clocks are temporarily enabled by platform_propbe function in order to
allow read access to the Core Release register and the Control Register.
Registers are used to detect M_CAN version and optional Non-ISO Feature.
Initialization of M_CAN for version >= 3.1.x:
* TX FIFO of M_CAN is used to transmit frames. The driver does not need to
stop the tx queue after each frame sent.
* Initialization of TX Event FIFO is added.
* NON-ISO is fixed for all M_CAN versions < 3.2.x. Version 3.2.x _can_ have
the NISO (Non-ISO) bit which can switch the mode of the M_CAN to Non-ISO
mode. This bit does not have to be writeable. Therefore it is checked.
If it is writable Non-ISO support is added to the controllers supported
CAN modes.
New Functions:
* Function to read TXE (Transmit Event) FIFO. Will be used later.
* Function to check, if TXE FIFO is full.
* Function to check the Core Release version. The read value determines the
behaviour of the driver.
* Function to check if the NISO bit for version >= 3.2.x is implemented.
Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
---
drivers/net/can/m_can/m_can.c | 418 ++++++++++++++++++++++++++++++++++--------
1 file changed, 342 insertions(+), 76 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f2656a3..1538944 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -107,6 +107,21 @@ enum m_can_mram_cfg {
MRAM_CFG_NUM,
};
+enum m_can_versions {
+ M_CAN_UNKNOWN_VERSION = 0,
+ M_CAN_V30X,
+ M_CAN_V31X,
+ M_CAN_V32X
+};
+
+/* Core Release Register (CREL) */
+#define CREL_REL_SHIFT 28
+#define CREL_REL_MASK (0xF << CREL_REL_SHIFT)
+#define CREL_STEP_SHIFT 24
+#define CREL_STEP_MASK (0xF << CREL_STEP_SHIFT)
+#define CREL_SUBSTEP_SHIFT 20
+#define CREL_SUBSTEP_MASK (0xF << CREL_SUBSTEP_SHIFT)
+
/* Data Bit Timing & Prescaler Register (DBTP) */
#define DBTP_TDC BIT(23)
#define DBTP_DBRP_SHIFT 16
@@ -342,6 +357,7 @@ struct m_can_priv {
struct clk *cclk;
void __iomem *base;
u32 irqstatus;
+ enum m_can_versions version;
/* message ram configuration */
void __iomem *mram_base;
@@ -373,6 +389,22 @@ static inline void m_can_fifo_write(const struct m_can_priv *priv,
fpi * TXB_ELEMENT_SIZE + offset);
}
+static inline u32 m_can_txe_fifo_read(const struct m_can_priv *priv,
+ u32 fgi,
+ u32 offset) {
+ return readl(priv->mram_base + priv->mcfg[MRAM_TXE].off +
+ fgi * TXE_ELEMENT_SIZE + offset);
+}
+
+/* Check if TX fifo is full
+ * returns 0 if free
+ * returns 1 if occupied
+ */
+static inline int m_can_tx_fifo_full(const struct m_can_priv *priv)
+{
+ return !!(m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQF);
+}
+
static inline void m_can_config_endisable(const struct m_can_priv *priv,
bool enable)
{
@@ -833,7 +865,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static const struct can_bittiming_const m_can_bittiming_const = {
+static const struct can_bittiming_const m_can_30X_bittiming_const = {
.name = KBUILD_MODNAME,
.tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
.tseg1_max = 64,
@@ -845,7 +877,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
.brp_inc = 1,
};
-static const struct can_bittiming_const m_can_data_bittiming_const = {
+static const struct can_bittiming_const m_can_30X_data_bittiming_const = {
.name = KBUILD_MODNAME,
.tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
.tseg1_max = 16,
@@ -857,6 +889,30 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
.brp_inc = 1,
};
+static const struct can_bittiming_const m_can_31X_bittiming_const = {
+ .name = KBUILD_MODNAME,
+ .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
+ .tseg1_max = 256,
+ .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
+ .tseg2_max = 128,
+ .sjw_max = 128,
+ .brp_min = 1,
+ .brp_max = 512,
+ .brp_inc = 1,
+};
+
+static const struct can_bittiming_const m_can_31X_data_bittiming_const = {
+ .name = KBUILD_MODNAME,
+ .tseg1_min = 1, /* Time segment 1 = prop_seg + phase_seg1 */
+ .tseg1_max = 32,
+ .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
+ .tseg2_max = 16,
+ .sjw_max = 16,
+ .brp_min = 1,
+ .brp_max = 32,
+ .brp_inc = 1,
+};
+
static int m_can_set_bittiming(struct net_device *dev)
{
struct m_can_priv *priv = netdev_priv(dev);
@@ -892,6 +948,7 @@ static int m_can_set_bittiming(struct net_device *dev)
* - configure rx fifo
* - accept non-matching frame into fifo 0
* - configure tx buffer
+ * - >= v3.1.x: TX FIFO is used
* - configure mode
* - setup bittiming
*/
@@ -908,15 +965,32 @@ static void m_can_chip_config(struct net_device *dev)
/* Accept Non-matching Frames Into FIFO 0 */
m_can_write(priv, M_CAN_GFC, 0x0);
- /* only support one Tx Buffer currently */
- m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
- priv->mcfg[MRAM_TXB].off);
+ if (priv->version == M_CAN_V30X) {
+ /* only support one Tx Buffer currently */
+ m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
+ priv->mcfg[MRAM_TXB].off);
+ } else if (priv->version == M_CAN_V31X ||
+ priv->version == M_CAN_V32X) {
+ /* TX FIFO is used for newer IP Core versions */
+ m_can_write(priv, M_CAN_TXBC,
+ (priv->mcfg[MRAM_TXB].num << TXBC_TFQS_SHIFT) |
+ (priv->mcfg[MRAM_TXB].off));
+ }
/* support 64 bytes payload */
m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
- m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
- priv->mcfg[MRAM_TXE].off);
+ /* TX Event FIFO */
+ if (priv->version == M_CAN_V30X) {
+ m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
+ priv->mcfg[MRAM_TXE].off);
+ } else if (priv->version == M_CAN_V31X || priv->version == M_CAN_V32X) {
+ /* Full TX Event FIFO is used */
+ m_can_write(priv, M_CAN_TXEFC,
+ ((priv->mcfg[MRAM_TXE].num << TXEFC_EFS_SHIFT)
+ & TXEFC_EFS_MASK) |
+ priv->mcfg[MRAM_TXE].off);
+ }
/* rx fifo configuration, blocking mode, fifo size 1 */
m_can_write(priv, M_CAN_RXF0C,
@@ -928,22 +1002,50 @@ static void m_can_chip_config(struct net_device *dev)
priv->mcfg[MRAM_RXF1].off);
cccr = m_can_read(priv, M_CAN_CCCR);
- cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
- (CCCR_CME_MASK << CCCR_CME_SHIFT));
test = m_can_read(priv, M_CAN_TEST);
test &= ~TEST_LBCK;
+ if (priv->version == M_CAN_V30X) {
- if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
- cccr |= CCCR_MON;
+ cccr &= ~(CCCR_TEST | CCCR_MON |
+ (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
+ (CCCR_CME_MASK << CCCR_CME_SHIFT));
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
+ cccr |= CCCR_TEST;
+ cccr |= CCCR_MON;
+ test |= TEST_LBCK;
+ }
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
+ cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
+
+ m_can_write(priv, M_CAN_CCCR, cccr);
+ m_can_write(priv, M_CAN_TEST, test);
+
+ } else {
+ /* Version 3.1.X or 3.2.X */
+ cccr &= ~(CCCR_TEST | CCCR_MON | CCCR_BRSE | CCCR_FDOE);
+
+ /* Only 3.2.X has NISO Bit implemented */
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
+ cccr |= CCCR_NISO;
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
+ cccr |= CCCR_TEST;
+ cccr |= CCCR_MON;
+ test |= TEST_LBCK;
+ }
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
+ cccr |= (CCCR_BRSE | CCCR_FDOE);
- if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
- cccr |= CCCR_TEST;
- test |= TEST_LBCK;
}
- if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
- cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
+ /* Enable Monitoring (all versions) */
+ if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+ cccr |= CCCR_MON;
+ /* Write config */
m_can_write(priv, M_CAN_CCCR, cccr);
m_can_write(priv, M_CAN_TEST, test);
@@ -957,6 +1059,8 @@ static void m_can_chip_config(struct net_device *dev)
/* route all interrupts to INT0 */
m_can_write(priv, M_CAN_ILS, ILS_ALL_INT0);
+ /* Only 3.2.X has NISO Bit implemented */
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
/* set bittiming params */
m_can_set_bittiming(dev);
@@ -994,33 +1098,160 @@ static void free_m_can_dev(struct net_device *dev)
free_candev(dev);
}
-static struct net_device *alloc_m_can_dev(void)
+static int m_can_check_core_release(void * __iomem m_can_base)
+{
+ u32 crel_reg = 0;
+ u8 rel = 0;
+ u8 step = 0;
+ int res = 0;
+ struct m_can_priv temp_priv;
+
+ temp_priv.base = m_can_base;
+
+ /* Read Core Release Version and split into version number
+ * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1;
+ */
+ crel_reg = m_can_read((const struct m_can_priv *)&temp_priv,
+ M_CAN_CREL);
+ rel = (u8)((crel_reg & CREL_REL_MASK) >> CREL_REL_SHIFT);
+ step = (u8)((crel_reg & CREL_STEP_MASK) >> CREL_STEP_SHIFT);
+
+ if (rel == 3) {
+ switch (step) {
+ case 0:
+ /* Version 3.0.x */
+ res = M_CAN_V30X;
+ break;
+ case 1:
+ /* Version 3.1.x */
+ res = M_CAN_V31X;
+ break;
+ case 2:
+ /* Version 3.2.x */
+ res = M_CAN_V32X;
+ break;
+ default:
+ /* Unknown version */
+ res = M_CAN_UNKNOWN_VERSION;
+ break;
+ }
+ } else {
+ res = M_CAN_UNKNOWN_VERSION;
+ }
+
+ return res;
+}
+
+/* Selectable Non ISO support only in version 3.2.x
+ * This function checks if the bit is writable.
+ */
+static int m_can_check_niso_support(const struct m_can_priv *priv)
+{
+ u32 cccr_reg;
+ int timeout = 10;
+ int niso_support = 0;
+
+ m_can_config_endisable(priv, true);
+ cccr_reg = m_can_read(priv, M_CAN_CCCR);
+ cccr_reg |= CCCR_NISO;
+ m_can_write(priv, M_CAN_CCCR, cccr_reg);
+
+ while ((m_can_read(priv, M_CAN_CCCR) & (CCCR_NISO)) == 0) {
+ if (timeout == 0) {
+ /* NISO Flag could not be set NONISO not supported */
+ /* Clear NISO to prevent errors */
+ cccr_reg &= ~(CCCR_NISO);
+ m_can_write(priv, M_CAN_CCCR, cccr_reg);
+ niso_support = 0;
+ goto return_niso;
+ }
+ timeout--;
+ udelay(1);
+ }
+ /* NISO Flag is set*/
+ niso_support = 1;
+ /* Clear NISO */
+ cccr_reg &= ~(CCCR_NISO);
+ m_can_write(priv, M_CAN_CCCR, cccr_reg);
+
+return_niso:
+ m_can_config_endisable(priv, false);
+ return niso_support;
+}
+
+static struct net_device *alloc_m_can_dev(void __iomem *addr, u32 tx_fifo_size)
{
struct net_device *dev;
struct m_can_priv *priv;
+ int m_can_version = M_CAN_UNKNOWN_VERSION;
+ int niso_bit_supported = 0;
+ unsigned int loopback_buffer_count;
+
+ m_can_version = m_can_check_core_release(addr);
+ /* return if unsupported version */
+ if (m_can_version == M_CAN_UNKNOWN_VERSION) {
+ dev = NULL;
+ goto return_dev;
+ }
- dev = alloc_candev(sizeof(*priv), 1);
- if (!dev)
- return NULL;
+ /* If version < 3.1.x, then only one loopback buffer is used */
+ loopback_buffer_count = ((m_can_version == M_CAN_V30X)
+ ? 1U
+ : (unsigned int)tx_fifo_size);
+ dev = alloc_candev(sizeof(*priv), loopback_buffer_count);
+ if (!dev) {
+ dev = NULL;
+ goto return_dev;
+ }
priv = netdev_priv(dev);
netif_napi_add(dev, &priv->napi, m_can_poll, M_CAN_NAPI_WEIGHT);
+ /* Shared properties of all M_CAN versions */
+ priv->version = m_can_version;
priv->dev = dev;
- priv->can.bittiming_const = &m_can_bittiming_const;
- priv->can.data_bittiming_const = &m_can_data_bittiming_const;
+ priv->base = addr;
priv->can.do_set_mode = m_can_set_mode;
priv->can.do_get_berr_counter = m_can_get_berr_counter;
- /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */
- can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
- /* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */
- priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
- CAN_CTRLMODE_LISTENONLY |
- CAN_CTRLMODE_BERR_REPORTING |
- CAN_CTRLMODE_FD;
+ /* Set properties depending on M_CAN version */
+ switch (priv->version) {
+ case M_CAN_V30X:
+ priv->can.bittiming_const = &m_can_30X_bittiming_const;
+ priv->can.data_bittiming_const =
+ &m_can_30X_data_bittiming_const;
+ /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */
+ can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
+ break;
+ case M_CAN_V31X:
+ /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */
+ can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
+ priv->can.bittiming_const = &m_can_31X_bittiming_const;
+ priv->can.data_bittiming_const =
+ &m_can_31X_data_bittiming_const;
+ break;
+ case M_CAN_V32X:
+ priv->can.bittiming_const = &m_can_31X_bittiming_const;
+ priv->can.data_bittiming_const =
+ &m_can_31X_data_bittiming_const;
+ niso_bit_supported = m_can_check_niso_support(priv);
+ break;
+ default:
+ /* This does not happen */
+ break;
+ }
+
+ /* Set M_CAN supported operations */
+ priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
+ CAN_CTRLMODE_LISTENONLY |
+ CAN_CTRLMODE_BERR_REPORTING |
+ CAN_CTRLMODE_FD |
+ (niso_bit_supported == 1
+ ? CAN_CTRLMODE_FD_NON_ISO
+ : 0);
+return_dev:
return dev;
}
@@ -1167,58 +1398,39 @@ static int register_m_can_dev(struct net_device *dev)
return register_candev(dev);
}
-static int m_can_of_parse_mram(struct platform_device *pdev,
- struct m_can_priv *priv)
+static void m_can_of_parse_mram(void __iomem *m_ram_base,
+ const u32 *mram_config_vals,
+ struct m_can_priv *priv)
{
- struct device_node *np = pdev->dev.of_node;
- struct resource *res;
- void __iomem *addr;
- u32 out_val[MRAM_CFG_LEN];
- int i, start, end, ret;
-
- /* message ram could be shared */
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
- if (!res)
- return -ENODEV;
+ int i, start, end;
- addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!addr)
- return -ENOMEM;
-
- /* get message ram configuration */
- ret = of_property_read_u32_array(np, "bosch,mram-cfg",
- out_val, sizeof(out_val) / 4);
- if (ret) {
- dev_err(&pdev->dev, "can not get message ram configuration\n");
- return -ENODEV;
- }
-
- priv->mram_base = addr;
- priv->mcfg[MRAM_SIDF].off = out_val[0];
- priv->mcfg[MRAM_SIDF].num = out_val[1];
+ priv->mram_base = m_ram_base;
+ priv->mcfg[MRAM_SIDF].off = mram_config_vals[0];
+ priv->mcfg[MRAM_SIDF].num = mram_config_vals[1];
priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +
priv->mcfg[MRAM_SIDF].num * SIDF_ELEMENT_SIZE;
- priv->mcfg[MRAM_XIDF].num = out_val[2];
+ priv->mcfg[MRAM_XIDF].num = mram_config_vals[2];
priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
- priv->mcfg[MRAM_RXF0].num = out_val[3] &
+ priv->mcfg[MRAM_RXF0].num = mram_config_vals[3] &
(RXFC_FS_MASK >> RXFC_FS_SHIFT);
priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
- priv->mcfg[MRAM_RXF1].num = out_val[4] &
+ priv->mcfg[MRAM_RXF1].num = mram_config_vals[4] &
(RXFC_FS_MASK >> RXFC_FS_SHIFT);
priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
- priv->mcfg[MRAM_RXB].num = out_val[5];
+ priv->mcfg[MRAM_RXB].num = mram_config_vals[5];
priv->mcfg[MRAM_TXE].off = priv->mcfg[MRAM_RXB].off +
priv->mcfg[MRAM_RXB].num * RXB_ELEMENT_SIZE;
- priv->mcfg[MRAM_TXE].num = out_val[6];
+ priv->mcfg[MRAM_TXE].num = mram_config_vals[6];
priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
- priv->mcfg[MRAM_TXB].num = out_val[7] &
+ priv->mcfg[MRAM_TXB].num = mram_config_vals[7] &
(TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
- dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
+ dev_dbg(priv->device,
+ "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
priv->mram_base,
priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num,
priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num,
@@ -1237,7 +1449,6 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
for (i = start; i < end; i += 4)
writel(0x0, priv->mram_base + i);
- return 0;
}
static int m_can_plat_probe(struct platform_device *pdev)
@@ -1246,38 +1457,85 @@ static int m_can_plat_probe(struct platform_device *pdev)
struct m_can_priv *priv;
struct resource *res;
void __iomem *addr;
+ void __iomem *mram_addr;
struct clk *hclk, *cclk;
int irq, ret;
+ struct device_node *np;
+ u32 mram_config_vals[MRAM_CFG_LEN];
+ u32 tx_fifo_size;
+
+ np = pdev->dev.of_node;
hclk = devm_clk_get(&pdev->dev, "hclk");
cclk = devm_clk_get(&pdev->dev, "cclk");
+
if (IS_ERR(hclk) || IS_ERR(cclk)) {
dev_err(&pdev->dev, "no clock found\n");
- return -ENODEV;
+ ret = -ENODEV;
+ goto failed_ret;
}
+ /* Enable clocks. Necessary to read Core Release in order to determine
+ * M_CAN version
+ */
+ ret = clk_prepare_enable(hclk);
+ if (ret)
+ goto disable_hclk_ret;
+
+ ret = clk_prepare_enable(cclk);
+ if (ret)
+ goto disable_cclk_ret;
+
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
addr = devm_ioremap_resource(&pdev->dev, res);
irq = platform_get_irq_byname(pdev, "int0");
- if (IS_ERR(addr) || irq < 0)
- return -EINVAL;
- /* allocate the m_can device */
- dev = alloc_m_can_dev();
- if (!dev)
- return -ENOMEM;
+ if (IS_ERR(addr) || irq < 0) {
+ ret = -EINVAL;
+ goto disable_cclk_ret;
+ }
+
+ /* message ram could be shared */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
+ if (!res) {
+ ret = -ENODEV;
+ goto disable_cclk_ret;
+ }
+
+ mram_addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!mram_addr) {
+ ret = -ENOMEM;
+ goto disable_cclk_ret;
+ }
+ /* get message ram configuration */
+ ret = of_property_read_u32_array(np, "bosch,mram-cfg",
+ mram_config_vals,
+ sizeof(mram_config_vals) / 4);
+ if (ret) {
+ dev_err(&pdev->dev, "Could not get Message RAM configuration.");
+ goto disable_cclk_ret;
+ }
+
+ /* Get TX FIFO size
+ * Defines the total amount of echo buffers for loopback
+ */
+ tx_fifo_size = mram_config_vals[7];
+
+ /* allocate the m_can device */
+ dev = alloc_m_can_dev(addr, tx_fifo_size);
+ if (!dev) {
+ ret = -ENOMEM;
+ goto disable_cclk_ret;
+ }
priv = netdev_priv(dev);
dev->irq = irq;
- priv->base = addr;
priv->device = &pdev->dev;
priv->hclk = hclk;
priv->cclk = cclk;
priv->can.clock.freq = clk_get_rate(cclk);
- ret = m_can_of_parse_mram(pdev, priv);
- if (ret)
- goto failed_free_dev;
+ m_can_of_parse_mram(mram_addr, mram_config_vals, priv);
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -1294,10 +1552,18 @@ static int m_can_plat_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
KBUILD_MODNAME, priv->base, dev->irq);
- return 0;
+ /* Probe finished
+ * Stop clocks. They will be reactivated once the M_CAN device is opened
+ */
+ goto disable_cclk_ret;
failed_free_dev:
free_m_can_dev(dev);
+disable_cclk_ret:
+ clk_disable_unprepare(cclk);
+disable_hclk_ret:
+ clk_disable_unprepare(hclk);
+failed_ret:
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
2017-03-10 14:00 [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x Mario Huettel
` (2 preceding siblings ...)
2017-03-10 14:00 ` [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization Mario Huettel
@ 2017-03-10 14:00 ` Mario Huettel
2017-03-16 9:03 ` Oliver Hartkopp
2017-03-21 14:50 ` Marc Kleine-Budde
3 siblings, 2 replies; 22+ messages in thread
From: Mario Huettel @ 2017-03-10 14:00 UTC (permalink / raw)
To: linux-can
* Added defines for TX Event FIFO Element
* Adapted ndo_start_xmit function.
For versions >= v3.1.x it uses the TX FIFO to optimize the data
throughput. It stores the echo skb at the same index as in the
M_CAN's TX FIFO. The frame's message marker is set to this index.
This message marker is received in the TX Event FIFO after
the message was successfully transmitted. It is used to echo the
correct echo skb back to the network stack.
* Added m_can_echo_tx_event function. It reads all received
message markers in the TX Event FIFO and loops back the
corresponding echo skbs.
* ISR checks for new TX Event Entry interrupt for version >= 3.1.x.
* Added spinlock to prevent concurrent accesses on the TX queue.
Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
---
drivers/net/can/m_can/m_can.c | 198 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 174 insertions(+), 24 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 1538944..1a30d0e 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/spinlock.h>
#include <linux/can/dev.h>
@@ -341,6 +342,11 @@ enum m_can_versions {
#define TX_BUF_MM_SHIFT 24
#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
+/* Tx event FIFO Element */
+/* E1 */
+#define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT
+#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT)
+
/* address offset and element number for each FIFO/Buffer in the Message RAM */
struct mram_cfg {
u16 off;
@@ -358,6 +364,8 @@ struct m_can_priv {
void __iomem *base;
u32 irqstatus;
enum m_can_versions version;
+ /* Spinlock to protect TX queue */
+ spinlock_t xmit_lock;
/* message ram configuration */
void __iomem *mram_base;
@@ -828,6 +836,44 @@ static int m_can_poll(struct napi_struct *napi, int quota)
return work_done;
}
+static void m_can_echo_tx_event(struct net_device *dev)
+{
+ u32 txe_count = 0;
+ u32 m_can_txefs;
+ u32 fgi = 0;
+ int i = 0;
+ unsigned int msg_mark;
+
+ struct m_can_priv *priv = netdev_priv(dev);
+ struct net_device_stats *stats = &dev->stats;
+
+ /* read tx event fifo status */
+ m_can_txefs = m_can_read(priv, M_CAN_TXEFS);
+
+ /* Get Tx Event fifo element count */
+ txe_count = (m_can_txefs & TXEFS_EFFL_MASK)
+ >> TXEFS_EFFL_SHIFT;
+
+ /* Get and process all sent elements */
+ for (i = 0; i < txe_count; i++) {
+ /* retrieve get index */
+ fgi = (m_can_read(priv, M_CAN_TXEFS) & TXEFS_EFGI_MASK)
+ >> TXEFS_EFGI_SHIFT;
+
+ /* get message marker */
+ msg_mark = (m_can_txe_fifo_read(priv, fgi, 4) &
+ TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
+
+ /* ack txe element */
+ m_can_write(priv, M_CAN_TXEFA, (TXEFA_EFAI_MASK &
+ (fgi << TXEFA_EFAI_SHIFT)));
+
+ /* update stats */
+ stats->tx_bytes += can_get_echo_skb(dev, msg_mark);
+ stats->tx_packets++;
+ }
+}
+
static irqreturn_t m_can_isr(int irq, void *dev_id)
{
struct net_device *dev = (struct net_device *)dev_id;
@@ -855,11 +901,19 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
}
/* transmission complete interrupt */
- if (ir & IR_TC) {
+ if ((ir & IR_TC) && priv->version == M_CAN_V30X) {
stats->tx_bytes += can_get_echo_skb(dev, 0);
stats->tx_packets++;
can_led_event(dev, CAN_LED_EVENT_TX);
netif_wake_queue(dev);
+ } else if ((ir & IR_TEFN) &&
+ (priv->version == M_CAN_V31X ||
+ priv->version == M_CAN_V32X)) {
+ /* New TX FIFO Element arrived */
+ m_can_echo_tx_event(dev);
+ can_led_event(dev, CAN_LED_EVENT_TX);
+ if (netif_queue_stopped(dev) && !m_can_tx_fifo_full(priv))
+ netif_wake_queue(dev);
}
return IRQ_HANDLED;
@@ -1052,7 +1106,12 @@ static void m_can_chip_config(struct net_device *dev)
/* enable interrupts */
m_can_write(priv, M_CAN_IR, IR_ALL_INT);
if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
- m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
+ if (priv->version == M_CAN_V30X)
+ m_can_write(priv, M_CAN_IE, IR_ALL_INT &
+ ~(IR_ERR_LEC_30X));
+ else
+ m_can_write(priv, M_CAN_IE, IR_ALL_INT &
+ ~(IR_ERR_LEC_31X));
else
m_can_write(priv, M_CAN_IE, IR_ALL_INT);
@@ -1329,19 +1388,35 @@ static int m_can_close(struct net_device *dev)
return 0;
}
+static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
+{
+ struct m_can_priv *priv = netdev_priv(dev);
+ /*get wrap around for loopback skb index */
+ unsigned int wrap = priv->can.echo_skb_max;
+ int next_idx;
+
+ /* calculate next index */
+ next_idx = (++putidx >= wrap ? 0 : putidx);
+
+ /* check if occupied */
+ return !!priv->can.echo_skb[next_idx];
+}
+
static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct m_can_priv *priv = netdev_priv(dev);
struct canfd_frame *cf = (struct canfd_frame *)skb->data;
- u32 id, cccr;
+ u32 id, cccr, fdflags;
int i;
+ int putidx;
+ unsigned long irqsave;
if (can_dropped_invalid_skb(dev, skb))
return NETDEV_TX_OK;
- netif_stop_queue(dev);
-
+ /* Generate ID field for TX buffer Element */
+ /* Common to all supported M_CAN versions */
if (cf->can_id & CAN_EFF_FLAG) {
id = cf->can_id & CAN_EFF_MASK;
id |= TX_BUF_XTD;
@@ -1352,33 +1427,105 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
if (cf->can_id & CAN_RTR_FLAG)
id |= TX_BUF_RTR;
- /* message ram configuration */
- m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
- m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, can_len2dlc(cf->len) << 16);
+ if (priv->version == M_CAN_V30X) {
+ netif_stop_queue(dev);
- for (i = 0; i < cf->len; i += 4)
- m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
- *(u32 *)(cf->data + i));
+ /* message ram configuration */
+ m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
+ m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC,
+ can_len2dlc(cf->len) << 16);
- can_put_echo_skb(skb, dev, 0);
+ for (i = 0; i < cf->len; i += 4)
+ m_can_fifo_write(priv, 0,
+ M_CAN_FIFO_DATA(i / 4),
+ *(u32 *)(cf->data + i));
+
+ can_put_echo_skb(skb, dev, 0);
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+ cccr = m_can_read(priv, M_CAN_CCCR);
+ cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
+ if (can_is_canfd_skb(skb)) {
+ if (cf->flags & CANFD_BRS)
+ cccr |= CCCR_CMR_CANFD_BRS <<
+ CCCR_CMR_SHIFT;
+ else
+ cccr |= CCCR_CMR_CANFD <<
+ CCCR_CMR_SHIFT;
+ } else {
+ cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
+ }
+ m_can_write(priv, M_CAN_CCCR, cccr);
+ }
+ m_can_write(priv, M_CAN_TXBTIE, 0x1);
+ m_can_write(priv, M_CAN_TXBAR, 0x1);
+ /* End of xmit function for version 3.0.x */
+ } else if (priv->version == M_CAN_V31X ||
+ priv->version == M_CAN_V32X) {
+ /* Check if FIFO full */
+ if (m_can_tx_fifo_full(priv)) {
+ /* This shouldn't happen */
+ netif_stop_queue(dev);
+ netdev_warn(dev,
+ "TX queue active although FIFO is full.");
+ return NETDEV_TX_BUSY;
+ }
- if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
- cccr = m_can_read(priv, M_CAN_CCCR);
- cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
+ /* get put index for frame */
+ putidx = ((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
+ >> TXFQS_TFQPI_SHIFT);
+ /* Write ID Field to FIFO Element */
+ m_can_fifo_write(priv, putidx, M_CAN_FIFO_ID, id);
+
+ /* get CAN FD configuration of frame */
+ fdflags = 0;
if (can_is_canfd_skb(skb)) {
+ fdflags |= TX_BUF_FDF;
if (cf->flags & CANFD_BRS)
- cccr |= CCCR_CMR_CANFD_BRS << CCCR_CMR_SHIFT;
- else
- cccr |= CCCR_CMR_CANFD << CCCR_CMR_SHIFT;
- } else {
- cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
+ fdflags |= TX_BUF_BRS;
}
- m_can_write(priv, M_CAN_CCCR, cccr);
+
+ /* Construct DLC Field. Also contains CAN-FD configuration
+ * use put index of fifo as message marker
+ * it is used in TX interrupt for
+ * sending the correct echo frame
+ */
+ m_can_fifo_write(priv, putidx, M_CAN_FIFO_DLC,
+ ((putidx << TX_BUF_MM_SHIFT) &
+ TX_BUF_MM_MASK) |
+ (can_len2dlc(cf->len) << 16) |
+ fdflags | TX_BUF_EFC);
+
+ for (i = 0; i < cf->len; i += 4)
+ m_can_fifo_write(priv, putidx, M_CAN_FIFO_DATA(i / 4),
+ *(u32 *)(cf->data + i));
+
+ /* Push loopback echo.
+ * Will be looped back on TX interrupt based on message marker
+ */
+ can_put_echo_skb(skb, dev, putidx);
+
+ /* Enable TX FIFO element to start transfer */
+ m_can_write(priv, M_CAN_TXBAR, (1 << putidx));
+
+ /* wait until put index has changed */
+ while (((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
+ >> TXFQS_TFQPI_SHIFT) == putidx)
+ ;
+
+ /* Lock spinlock for TX to prevent concurrent
+ * access on the network queue
+ */
+ spin_lock_irqsave(&priv->xmit_lock, irqsave);
+
+ /* stop network queue if fifo full */
+ if (m_can_tx_fifo_full(priv) ||
+ m_can_next_echo_skb_occupied(dev, putidx))
+ netif_stop_queue(dev);
+
+ spin_unlock_irqrestore(&priv->xmit_lock, irqsave);
}
- /* enable first TX buffer to start transfer */
- m_can_write(priv, M_CAN_TXBTIE, 0x1);
- m_can_write(priv, M_CAN_TXBAR, 0x1);
return NETDEV_TX_OK;
}
@@ -1552,9 +1699,12 @@ static int m_can_plat_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
KBUILD_MODNAME, priv->base, dev->irq);
+ spin_lock_init(&priv->xmit_lock);
+
/* Probe finished
* Stop clocks. They will be reactivated once the M_CAN device is opened
*/
+
goto disable_cclk_ret;
failed_free_dev:
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x
2017-03-10 14:00 ` [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x Mario Huettel
@ 2017-03-16 8:41 ` Oliver Hartkopp
2017-03-21 10:36 ` Marc Kleine-Budde
1 sibling, 0 replies; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-16 8:41 UTC (permalink / raw)
To: Mario Huettel, linux-can
On 10.03.2017 15:00, Mario Huettel wrote:
> This patch includes following changes:
>
> * Renamed the register defines of the M_CAN to fit version 3.1.x and above.
> * Replaced the old defines with the new ones in the whole code.
> * Removed code that enabled interrupt line 1. The driver didn't use it.
> * Removed initialization of FIFO water marks. They were not used.
>
> Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> drivers/net/can/m_can/m_can.c | 197 +++++++++++++++++++++++++++---------------
> 1 file changed, 129 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 7a6554e..f2656a3 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -37,17 +37,19 @@ enum m_can_reg {
> M_CAN_CREL = 0x0,
> M_CAN_ENDN = 0x4,
> M_CAN_CUST = 0x8,
> - M_CAN_FBTP = 0xc,
> + M_CAN_DBTP = 0xc,
> M_CAN_TEST = 0x10,
> M_CAN_RWD = 0x14,
> M_CAN_CCCR = 0x18,
> - M_CAN_BTP = 0x1c,
> + M_CAN_NBTP = 0x1c,
> M_CAN_TSCC = 0x20,
> M_CAN_TSCV = 0x24,
> M_CAN_TOCC = 0x28,
> M_CAN_TOCV = 0x2c,
> M_CAN_ECR = 0x40,
> M_CAN_PSR = 0x44,
> +/* TDCR Register only available for version >=3.1.x */
> + M_CAN_TDCR = 0x48,
> M_CAN_IR = 0x50,
> M_CAN_IE = 0x54,
> M_CAN_ILS = 0x58,
> @@ -105,21 +107,21 @@ enum m_can_mram_cfg {
> MRAM_CFG_NUM,
> };
>
> -/* Fast Bit Timing & Prescaler Register (FBTP) */
> -#define FBTR_FBRP_MASK 0x1f
> -#define FBTR_FBRP_SHIFT 16
> -#define FBTR_FTSEG1_SHIFT 8
> -#define FBTR_FTSEG1_MASK (0xf << FBTR_FTSEG1_SHIFT)
> -#define FBTR_FTSEG2_SHIFT 4
> -#define FBTR_FTSEG2_MASK (0x7 << FBTR_FTSEG2_SHIFT)
> -#define FBTR_FSJW_SHIFT 0
> -#define FBTR_FSJW_MASK 0x3
> +/* Data Bit Timing & Prescaler Register (DBTP) */
> +#define DBTP_TDC BIT(23)
> +#define DBTP_DBRP_SHIFT 16
> +#define DBTP_DBRP_MASK (0x1f << DBTP_DBRP_SHIFT)
> +#define DBTP_DTSEG1_SHIFT 8
> +#define DBTP_DTSEG1_MASK (0x1f << DBTP_DTSEG1_SHIFT)
> +#define DBTP_DTSEG2_SHIFT 4
> +#define DBTP_DTSEG2_MASK (0xf << DBTP_DTSEG2_SHIFT)
> +#define DBTP_DSJW_SHIFT 0
> +#define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>
> /* Test Register (TEST) */
> -#define TEST_LBCK BIT(4)
> +#define TEST_LBCK BIT(4)
>
> /* CC Control Register(CCCR) */
> -#define CCCR_TEST BIT(7)
> #define CCCR_CMR_MASK 0x3
> #define CCCR_CMR_SHIFT 10
> #define CCCR_CMR_CANFD 0x1
> @@ -130,21 +132,32 @@ enum m_can_mram_cfg {
> #define CCCR_CME_CAN 0
> #define CCCR_CME_CANFD 0x1
> #define CCCR_CME_CANFD_BRS 0x2
> +#define CCCR_TXP BIT(14)
> #define CCCR_TEST BIT(7)
> #define CCCR_MON BIT(5)
> +#define CCCR_CSR BIT(4)
> +#define CCCR_CSA BIT(3)
> +#define CCCR_ASM BIT(2)
> #define CCCR_CCE BIT(1)
> #define CCCR_INIT BIT(0)
> #define CCCR_CANFD 0x10
> -
> -/* Bit Timing & Prescaler Register (BTP) */
> -#define BTR_BRP_MASK 0x3ff
> -#define BTR_BRP_SHIFT 16
> -#define BTR_TSEG1_SHIFT 8
> -#define BTR_TSEG1_MASK (0x3f << BTR_TSEG1_SHIFT)
> -#define BTR_TSEG2_SHIFT 4
> -#define BTR_TSEG2_MASK (0xf << BTR_TSEG2_SHIFT)
> -#define BTR_SJW_SHIFT 0
> -#define BTR_SJW_MASK 0xf
> +/* for version >=3.1.x */
> +#define CCCR_EFBI BIT(13)
> +#define CCCR_PXHD BIT(12)
> +#define CCCR_BRSE BIT(9)
> +#define CCCR_FDOE BIT(8)
> +/* only for version >=3.2.x */
> +#define CCCR_NISO BIT(15)
> +
> +/* Nominal Bit Timing & Prescaler Register (NBTP) */
> +#define NBTP_NSJW_SHIFT 25
> +#define NBTP_NSJW_MASK (0x7f << NBTP_NSJW_SHIFT)
> +#define NBTP_NBRP_SHIFT 16
> +#define NBTP_NBRP_MASK (0x1ff << NBTP_NBRP_SHIFT)
> +#define NBTP_NTSEG1_SHIFT 8
> +#define NBTP_NTSEG1_MASK (0xff << NBTP_NTSEG1_SHIFT)
> +#define NBTP_NTSEG2_SHIFT 0
> +#define NBTP_NTSEG2_MASK (0x7f << NBTP_NTSEG2_SHIFT)
>
> /* Error Counter Register(ECR) */
> #define ECR_RP BIT(15)
> @@ -161,6 +174,13 @@ enum m_can_mram_cfg {
>
> /* Interrupt Register(IR) */
> #define IR_ALL_INT 0xffffffff
> +
> +/* Renamed bits for versions > 3.1.x */
> +#define IR_ARA BIT(29)
> +#define IR_PED BIT(28)
> +#define IR_PEA BIT(27)
> +
> +/* Bits for version 3.0.x */
> #define IR_STE BIT(31)
> #define IR_FOE BIT(30)
> #define IR_ACKE BIT(29)
> @@ -194,33 +214,40 @@ enum m_can_mram_cfg {
> #define IR_RF0W BIT(1)
> #define IR_RF0N BIT(0)
> #define IR_ERR_STATE (IR_BO | IR_EW | IR_EP)
> -#define IR_ERR_LEC (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
> -#define IR_ERR_BUS (IR_ERR_LEC | IR_WDI | IR_ELO | IR_BEU | \
> +
> +/* Interrupts for version 3.0.x */
> +#define IR_ERR_LEC_30X (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
> +#define IR_ERR_BUS_30X (IR_ERR_LEC_30X | IR_WDI | IR_ELO | IR_BEU | \
> + IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
> + IR_RF1L | IR_RF0L)
> +#define IR_ERR_ALL_30X (IR_ERR_STATE | IR_ERR_BUS_30X)
> +/* Interrupts for version >= 3.1.x */
> +#define IR_ERR_LEC_31X (IR_PED | IR_PEA)
> +#define IR_ERR_BUS_31X (IR_ERR_LEC_31X | IR_WDI | IR_ELO | IR_BEU | \
> IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
> IR_RF1L | IR_RF0L)
> -#define IR_ERR_ALL (IR_ERR_STATE | IR_ERR_BUS)
> +#define IR_ERR_ALL_31X (IR_ERR_STATE | IR_ERR_BUS_31X)
>
> /* Interrupt Line Select (ILS) */
> #define ILS_ALL_INT0 0x0
> #define ILS_ALL_INT1 0xFFFFFFFF
>
> /* Interrupt Line Enable (ILE) */
> -#define ILE_EINT0 BIT(0)
> #define ILE_EINT1 BIT(1)
> +#define ILE_EINT0 BIT(0)
>
> /* Rx FIFO 0/1 Configuration (RXF0C/RXF1C) */
> -#define RXFC_FWM_OFF 24
> -#define RXFC_FWM_MASK 0x7f
> -#define RXFC_FWM_1 (1 << RXFC_FWM_OFF)
> -#define RXFC_FS_OFF 16
> -#define RXFC_FS_MASK 0x7f
> +#define RXFC_FWM_SHIFT 24
> +#define RXFC_FWM_MASK (0x7f < RXFC_FWM_SHIFT)
> +#define RXFC_FS_SHIFT 16
> +#define RXFC_FS_MASK (0x7f << RXFC_FS_SHIFT)
>
> /* Rx FIFO 0/1 Status (RXF0S/RXF1S) */
> #define RXFS_RFL BIT(25)
> #define RXFS_FF BIT(24)
> -#define RXFS_FPI_OFF 16
> +#define RXFS_FPI_SHIFT 16
> #define RXFS_FPI_MASK 0x3f0000
> -#define RXFS_FGI_OFF 8
> +#define RXFS_FGI_SHIFT 8
> #define RXFS_FGI_MASK 0x3f00
> #define RXFS_FFL_MASK 0x7f
>
> @@ -229,23 +256,46 @@ enum m_can_mram_cfg {
> #define M_CAN_RXESC_64BYTES 0x777
>
> /* Tx Buffer Configuration(TXBC) */
> -#define TXBC_NDTB_OFF 16
> -#define TXBC_NDTB_MASK 0x3f
> +#define TXBC_NDTB_SHIFT 16
> +#define TXBC_NDTB_MASK (0x3f << TXBC_NDTB_SHIFT)
> +#define TXBC_TFQS_SHIFT 24
> +#define TXBC_TFQS_MASK (0x3f << TXBC_TFQS_SHIFT)
> +
> +/* Tx FIFO/Queue Status (TXFQS) */
> +#define TXFQS_TFQF BIT(21)
> +#define TXFQS_TFQPI_SHIFT 16
> +#define TXFQS_TFQPI_MASK (0x1f << TXFQS_TFQPI_SHIFT)
> +#define TXFQS_TFGI_SHIFT 8
> +#define TXFQS_TFGI_MASK (0x1f << TXFQS_TFGI_SHIFT)
> +#define TXFQS_TFFL_SHIFT 0
> +#define TXFQS_TFFL_MASK (0x3f << TXFQS_TFFL_SHIFT)
>
> /* Tx Buffer Element Size Configuration(TXESC) */
> #define TXESC_TBDS_8BYTES 0x0
> #define TXESC_TBDS_64BYTES 0x7
>
> -/* Tx Event FIFO Con.guration (TXEFC) */
> -#define TXEFC_EFS_OFF 16
> -#define TXEFC_EFS_MASK 0x3f
> +/* Tx Event FIFO Configuration (TXEFC) */
> +#define TXEFC_EFS_SHIFT 16
> +#define TXEFC_EFS_MASK (0x3f << TXEFC_EFS_SHIFT)
> +
> +/* Tx Event FIFO Status (TXEFS) */
> +#define TXEFS_TEFL BIT(25)
> +#define TXEFS_EFF BIT(24)
> +#define TXEFS_EFGI_SHIFT 8
> +#define TXEFS_EFGI_MASK (0x1f << TXEFS_EFGI_SHIFT)
> +#define TXEFS_EFFL_SHIFT 0
> +#define TXEFS_EFFL_MASK (0x3f << TXEFS_EFFL_SHIFT)
> +
> +/* Tx Event FIFO Acknowledge (TXEFA) */
> +#define TXEFA_EFAI_SHIFT 0
> +#define TXEFA_EFAI_MASK (0x1f << TXEFA_EFAI_SHIFT)
>
> /* Message RAM Configuration (in bytes) */
> #define SIDF_ELEMENT_SIZE 4
> #define XIDF_ELEMENT_SIZE 8
> #define RXF0_ELEMENT_SIZE 72
> #define RXF1_ELEMENT_SIZE 72
> -#define RXB_ELEMENT_SIZE 16
> +#define RXB_ELEMENT_SIZE 72
> #define TXE_ELEMENT_SIZE 8
> #define TXB_ELEMENT_SIZE 72
>
> @@ -261,13 +311,20 @@ enum m_can_mram_cfg {
> #define RX_BUF_RTR BIT(29)
> /* R1 */
> #define RX_BUF_ANMF BIT(31)
> -#define RX_BUF_EDL BIT(21)
> +#define RX_BUF_FDF BIT(21)
> #define RX_BUF_BRS BIT(20)
>
> /* Tx Buffer Element */
> -/* R0 */
> +/* T0 */
> +#define TX_BUF_ESI BIT(31)
> #define TX_BUF_XTD BIT(30)
> #define TX_BUF_RTR BIT(29)
> +/* T1 */
> +#define TX_BUF_EFC BIT(23)
> +#define TX_BUF_FDF BIT(21)
> +#define TX_BUF_BRS BIT(20)
> +#define TX_BUF_MM_SHIFT 24
> +#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
>
> /* address offset and element number for each FIFO/Buffer in the Message RAM */
> struct mram_cfg {
> @@ -349,7 +406,8 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,
>
> static inline void m_can_enable_all_interrupts(const struct m_can_priv *priv)
> {
> - m_can_write(priv, M_CAN_ILE, ILE_EINT0 | ILE_EINT1);
> + /* Only interrupt line 0 is used in this driver */
> + m_can_write(priv, M_CAN_ILE, ILE_EINT0);
> }
>
> static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)
> @@ -367,9 +425,9 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> int i;
>
> /* calculate the fifo get index for where to read data */
> - fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
> + fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_SHIFT;
> dlc = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> - if (dlc & RX_BUF_EDL)
> + if (dlc & RX_BUF_FDF)
> skb = alloc_canfd_skb(dev, &cf);
> else
> skb = alloc_can_skb(dev, (struct can_frame **)&cf);
> @@ -378,7 +436,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> return;
> }
>
> - if (dlc & RX_BUF_EDL)
> + if (dlc & RX_BUF_FDF)
> cf->len = can_dlc2len((dlc >> 16) & 0x0F);
> else
> cf->len = get_can_dlc((dlc >> 16) & 0x0F);
> @@ -394,7 +452,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> netdev_dbg(dev, "ESI Error\n");
> }
>
> - if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
> + if (!(dlc & RX_BUF_FDF) && (id & RX_BUF_RTR)) {
> cf->can_id |= CAN_RTR_FLAG;
> } else {
> if (dlc & RX_BUF_BRS)
> @@ -532,7 +590,7 @@ static int __m_can_get_berr_counter(const struct net_device *dev,
>
> ecr = m_can_read(priv, M_CAN_ECR);
> bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
> - bec->txerr = ecr & ECR_TEC_MASK;
> + bec->txerr = (ecr & ECR_TEC_MASK) >> ECR_TEC_SHIFT;
>
> return 0;
> }
> @@ -723,7 +781,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> if (irqstatus & IR_ERR_STATE)
> work_done += m_can_handle_state_errors(dev, psr);
>
> - if (irqstatus & IR_ERR_BUS)
> + if (irqstatus & IR_ERR_BUS_30X)
> work_done += m_can_handle_bus_errors(dev, irqstatus, psr);
>
> if (irqstatus & IR_RF0N)
> @@ -758,7 +816,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> * - state change IRQ
> * - bus error IRQ and bus error reporting
> */
> - if ((ir & IR_RF0N) || (ir & IR_ERR_ALL)) {
> + if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
> priv->irqstatus = ir;
> m_can_disable_all_interrupts(priv);
> napi_schedule(&priv->napi);
> @@ -811,19 +869,19 @@ static int m_can_set_bittiming(struct net_device *dev)
> sjw = bt->sjw - 1;
> tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> tseg2 = bt->phase_seg2 - 1;
> - reg_btp = (brp << BTR_BRP_SHIFT) | (sjw << BTR_SJW_SHIFT) |
> - (tseg1 << BTR_TSEG1_SHIFT) | (tseg2 << BTR_TSEG2_SHIFT);
> - m_can_write(priv, M_CAN_BTP, reg_btp);
> + reg_btp = (brp << NBTP_NBRP_SHIFT) | (sjw << NBTP_NSJW_SHIFT) |
> + (tseg1 << NBTP_NTSEG1_SHIFT) | (tseg2 << NBTP_NTSEG2_SHIFT);
> + m_can_write(priv, M_CAN_NBTP, reg_btp);
>
> if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> brp = dbt->brp - 1;
> sjw = dbt->sjw - 1;
> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> tseg2 = dbt->phase_seg2 - 1;
> - reg_btp = (brp << FBTR_FBRP_SHIFT) | (sjw << FBTR_FSJW_SHIFT) |
> - (tseg1 << FBTR_FTSEG1_SHIFT) |
> - (tseg2 << FBTR_FTSEG2_SHIFT);
> - m_can_write(priv, M_CAN_FBTP, reg_btp);
> + reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
> + (tseg1 << DBTP_DTSEG1_SHIFT) |
> + (tseg2 << DBTP_DTSEG2_SHIFT);
> + m_can_write(priv, M_CAN_DBTP, reg_btp);
> }
>
> return 0;
> @@ -851,23 +909,23 @@ static void m_can_chip_config(struct net_device *dev)
> m_can_write(priv, M_CAN_GFC, 0x0);
>
> /* only support one Tx Buffer currently */
> - m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_OFF) |
> + m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
> priv->mcfg[MRAM_TXB].off);
>
> /* support 64 bytes payload */
> m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
>
> - m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_OFF) |
> + m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
> priv->mcfg[MRAM_TXE].off);
>
> /* rx fifo configuration, blocking mode, fifo size 1 */
> m_can_write(priv, M_CAN_RXF0C,
> - (priv->mcfg[MRAM_RXF0].num << RXFC_FS_OFF) |
> - RXFC_FWM_1 | priv->mcfg[MRAM_RXF0].off);
> + (priv->mcfg[MRAM_RXF0].num << RXFC_FS_SHIFT) |
> + priv->mcfg[MRAM_RXF0].off);
>
> m_can_write(priv, M_CAN_RXF1C,
> - (priv->mcfg[MRAM_RXF1].num << RXFC_FS_OFF) |
> - RXFC_FWM_1 | priv->mcfg[MRAM_RXF1].off);
> + (priv->mcfg[MRAM_RXF1].num << RXFC_FS_SHIFT) |
> + priv->mcfg[MRAM_RXF1].off);
>
> cccr = m_can_read(priv, M_CAN_CCCR);
> cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> @@ -892,7 +950,7 @@ static void m_can_chip_config(struct net_device *dev)
> /* enable interrupts */
> m_can_write(priv, M_CAN_IR, IR_ALL_INT);
> if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> - m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC);
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
> else
> m_can_write(priv, M_CAN_IE, IR_ALL_INT);
>
> @@ -1143,10 +1201,12 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_XIDF].num = out_val[2];
> priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
> priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF0].num = out_val[3] & RXFC_FS_MASK;
> + priv->mcfg[MRAM_RXF0].num = out_val[3] &
> + (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
> priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF1].num = out_val[4] & RXFC_FS_MASK;
> + priv->mcfg[MRAM_RXF1].num = out_val[4] &
> + (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
> priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
> priv->mcfg[MRAM_RXB].num = out_val[5];
> @@ -1155,7 +1215,8 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_TXE].num = out_val[6];
> priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
> priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXB].num = out_val[7] & TXBC_NDTB_MASK;
> + priv->mcfg[MRAM_TXB].num = out_val[7] &
> + (TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
>
> dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> priv->mram_base,
> @@ -1191,7 +1252,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
> hclk = devm_clk_get(&pdev->dev, "hclk");
> cclk = devm_clk_get(&pdev->dev, "cclk");
> if (IS_ERR(hclk) || IS_ERR(cclk)) {
> - dev_err(&pdev->dev, "no clock find\n");
> + dev_err(&pdev->dev, "no clock found\n");
> return -ENODEV;
> }
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-10 14:00 ` [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization Mario Huettel
@ 2017-03-16 8:52 ` Oliver Hartkopp
[not found] ` <c3d26079-554b-104d-4836-825a8496b66f@de.bosch.com>
2017-03-21 14:33 ` Marc Kleine-Budde
1 sibling, 1 reply; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-16 8:52 UTC (permalink / raw)
To: Mario Huettel, linux-can
Hello Mario,
thanks for your contribution!
On 10.03.2017 15:00, Mario Huettel wrote:
> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
> CCCR_MON bit. In combination with TEST_LBCK it activates the internal
> loopback mode. Leaving CCCR_MON '0' results in external loopback mode.
How is the interaction with the IFF_ECHO functionality then?
> * NON-ISO is fixed for all M_CAN versions < 3.2.x. Version 3.2.x _can_ have
> the NISO (Non-ISO) bit which can switch the mode of the M_CAN to Non-ISO
> mode. This bit does not have to be writeable. Therefore it is checked.
> If it is writable Non-ISO support is added to the controllers supported
> CAN modes.
Funny thing. So there are 3.2.x IP cores the only support the ISO mode?
> + if (priv->version == M_CAN_V30X) {
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> - cccr |= CCCR_MON;
> + cccr &= ~(CCCR_TEST | CCCR_MON |
> + (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> + (CCCR_CME_MASK << CCCR_CME_SHIFT));
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> + cccr |= CCCR_TEST;
> + cccr |= CCCR_MON;
> + test |= TEST_LBCK;
> + }
this code ...
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> + cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
> +
> + m_can_write(priv, M_CAN_CCCR, cccr);
> + m_can_write(priv, M_CAN_TEST, test);
You invoke m_can_write() after the if-statement too. Can this one be removed?
> +
> + } else {
> + /* Version 3.1.X or 3.2.X */
> + cccr &= ~(CCCR_TEST | CCCR_MON | CCCR_BRSE | CCCR_FDOE);
> +
> + /* Only 3.2.X has NISO Bit implemented */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
> + cccr |= CCCR_NISO;
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> + cccr |= CCCR_TEST;
> + cccr |= CCCR_MON;
> + test |= TEST_LBCK;
> + }
... and this code is identically, right?
So you could move it out of the if-statement?
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> + cccr |= (CCCR_BRSE | CCCR_FDOE);
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> - cccr |= CCCR_TEST;
> - test |= TEST_LBCK;
> }
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> - cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
> + /* Enable Monitoring (all versions) */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + cccr |= CCCR_MON;
>
> + /* Write config */
> m_can_write(priv, M_CAN_CCCR, cccr);
> m_can_write(priv, M_CAN_TEST, test);
>
> @@ -957,6 +1059,8 @@ static void m_can_chip_config(struct net_device *dev)
> /* route all interrupts to INT0 */
> m_can_write(priv, M_CAN_ILS, ILS_ALL_INT0);
>
> + /* Only 3.2.X has NISO Bit implemented */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
> /* set bittiming params */
> m_can_set_bittiming(dev);
This looks strange/wrong.
There's no indention and setting the bittiming is not depending on NON-ISO right?
>
> @@ -994,33 +1098,160 @@ static void free_m_can_dev(struct net_device *dev)
> free_candev(dev);
> }
>
> -static struct net_device *alloc_m_can_dev(void)
> +static int m_can_check_core_release(void * __iomem m_can_base)
> +{
> + u32 crel_reg = 0;
> + u8 rel = 0;
> + u8 step = 0;
> + int res = 0;
> + struct m_can_priv temp_priv;
> +
> + temp_priv.base = m_can_base;
> +
> + /* Read Core Release Version and split into version number
> + * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1;
> + */
> + crel_reg = m_can_read((const struct m_can_priv *)&temp_priv,
> + M_CAN_CREL);
> + rel = (u8)((crel_reg & CREL_REL_MASK) >> CREL_REL_SHIFT);
> + step = (u8)((crel_reg & CREL_STEP_MASK) >> CREL_STEP_SHIFT);
> +
> + if (rel == 3) {
> + switch (step) {
What's the reason to implement this enums?
Won't it make more sense to use plain values like
30 for 3.0.x
31 for 3.1.x
32 for 3.2.x
so that you can make easy if statements like
if (version > 30) ...
if (version == 32) ...
?
> + case 0:
> + /* Version 3.0.x */
> + res = M_CAN_V30X;
> + break;
> + case 1:
> + /* Version 3.1.x */
> + res = M_CAN_V31X;
> + break;
> + case 2:
> + /* Version 3.2.x */
> + res = M_CAN_V32X;
> + break;
> + default:
> + /* Unknown version */
> + res = M_CAN_UNKNOWN_VERSION;
> + break;
> + }
> + } else {
> + res = M_CAN_UNKNOWN_VERSION;
> + }
> +
> + return res;
> +}
> +
> +/* Selectable Non ISO support only in version 3.2.x
> + * This function checks if the bit is writable.
> + */
> +static int m_can_check_niso_support(const struct m_can_priv *priv)
> +{
> + u32 cccr_reg;
> + int timeout = 10;
> + int niso_support = 0;
> +
> + m_can_config_endisable(priv, true);
> + cccr_reg = m_can_read(priv, M_CAN_CCCR);
> + cccr_reg |= CCCR_NISO;
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> +
> + while ((m_can_read(priv, M_CAN_CCCR) & (CCCR_NISO)) == 0) {
> + if (timeout == 0) {
> + /* NISO Flag could not be set NONISO not supported */
> + /* Clear NISO to prevent errors */
> + cccr_reg &= ~(CCCR_NISO);
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> + niso_support = 0;
> + goto return_niso;
> + }
> + timeout--;
> + udelay(1);
> + }
> + /* NISO Flag is set*/
> + niso_support = 1;
> + /* Clear NISO */
> + cccr_reg &= ~(CCCR_NISO);
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> +
> +return_niso:
> + m_can_config_endisable(priv, false);
> + return niso_support;
> +}
> +
> +static struct net_device *alloc_m_can_dev(void __iomem *addr, u32 tx_fifo_size)
> {
> struct net_device *dev;
> struct m_can_priv *priv;
> + int m_can_version = M_CAN_UNKNOWN_VERSION;
> + int niso_bit_supported = 0;
> + unsigned int loopback_buffer_count;
> +
> + m_can_version = m_can_check_core_release(addr);
> + /* return if unsupported version */
> + if (m_can_version == M_CAN_UNKNOWN_VERSION) {
> + dev = NULL;
> + goto return_dev;
> + }
>
> - dev = alloc_candev(sizeof(*priv), 1);
> - if (!dev)
> - return NULL;
> + /* If version < 3.1.x, then only one loopback buffer is used */
> + loopback_buffer_count = ((m_can_version == M_CAN_V30X)
> + ? 1U
> + : (unsigned int)tx_fifo_size);
>
> + dev = alloc_candev(sizeof(*priv), loopback_buffer_count);
> + if (!dev) {
> + dev = NULL;
> + goto return_dev;
> + }
> priv = netdev_priv(dev);
> netif_napi_add(dev, &priv->napi, m_can_poll, M_CAN_NAPI_WEIGHT);
>
> + /* Shared properties of all M_CAN versions */
> + priv->version = m_can_version;
> priv->dev = dev;
> - priv->can.bittiming_const = &m_can_bittiming_const;
> - priv->can.data_bittiming_const = &m_can_data_bittiming_const;
> + priv->base = addr;
> priv->can.do_set_mode = m_can_set_mode;
> priv->can.do_get_berr_counter = m_can_get_berr_counter;
>
> - /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */
> - can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
>
> - /* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */
> - priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> - CAN_CTRLMODE_LISTENONLY |
> - CAN_CTRLMODE_BERR_REPORTING |
> - CAN_CTRLMODE_FD;
>
> + /* Set properties depending on M_CAN version */
> + switch (priv->version) {
> + case M_CAN_V30X:
> + priv->can.bittiming_const = &m_can_30X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_30X_data_bittiming_const;
> + /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */
> + can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
> + break;
> + case M_CAN_V31X:
> + /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */
> + can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
> + priv->can.bittiming_const = &m_can_31X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_31X_data_bittiming_const;
> + break;
> + case M_CAN_V32X:
> + priv->can.bittiming_const = &m_can_31X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_31X_data_bittiming_const;
> + niso_bit_supported = m_can_check_niso_support(priv);
> + break;
> + default:
> + /* This does not happen */
> + break;
> + }
> +
> + /* Set M_CAN supported operations */
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_BERR_REPORTING |
> + CAN_CTRLMODE_FD |
> + (niso_bit_supported == 1
> + ? CAN_CTRLMODE_FD_NON_ISO
> + : 0);
> +return_dev:
> return dev;
> }
>
> @@ -1167,58 +1398,39 @@ static int register_m_can_dev(struct net_device *dev)
> return register_candev(dev);
> }
>
> -static int m_can_of_parse_mram(struct platform_device *pdev,
> - struct m_can_priv *priv)
> +static void m_can_of_parse_mram(void __iomem *m_ram_base,
> + const u32 *mram_config_vals,
> + struct m_can_priv *priv)
> {
> - struct device_node *np = pdev->dev.of_node;
> - struct resource *res;
> - void __iomem *addr;
> - u32 out_val[MRAM_CFG_LEN];
> - int i, start, end, ret;
> -
> - /* message ram could be shared */
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> - if (!res)
> - return -ENODEV;
> + int i, start, end;
>
> - addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!addr)
> - return -ENOMEM;
> -
> - /* get message ram configuration */
> - ret = of_property_read_u32_array(np, "bosch,mram-cfg",
> - out_val, sizeof(out_val) / 4);
> - if (ret) {
> - dev_err(&pdev->dev, "can not get message ram configuration\n");
> - return -ENODEV;
> - }
> -
> - priv->mram_base = addr;
> - priv->mcfg[MRAM_SIDF].off = out_val[0];
> - priv->mcfg[MRAM_SIDF].num = out_val[1];
> + priv->mram_base = m_ram_base;
> + priv->mcfg[MRAM_SIDF].off = mram_config_vals[0];
> + priv->mcfg[MRAM_SIDF].num = mram_config_vals[1];
> priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +
> priv->mcfg[MRAM_SIDF].num * SIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_XIDF].num = out_val[2];
> + priv->mcfg[MRAM_XIDF].num = mram_config_vals[2];
> priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
> priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF0].num = out_val[3] &
> + priv->mcfg[MRAM_RXF0].num = mram_config_vals[3] &
> (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
> priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF1].num = out_val[4] &
> + priv->mcfg[MRAM_RXF1].num = mram_config_vals[4] &
> (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
> priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXB].num = out_val[5];
> + priv->mcfg[MRAM_RXB].num = mram_config_vals[5];
> priv->mcfg[MRAM_TXE].off = priv->mcfg[MRAM_RXB].off +
> priv->mcfg[MRAM_RXB].num * RXB_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXE].num = out_val[6];
> + priv->mcfg[MRAM_TXE].num = mram_config_vals[6];
> priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
> priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXB].num = out_val[7] &
> + priv->mcfg[MRAM_TXB].num = mram_config_vals[7] &
> (TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
>
> - dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> + dev_dbg(priv->device,
> + "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> priv->mram_base,
> priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num,
> priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num,
> @@ -1237,7 +1449,6 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> for (i = start; i < end; i += 4)
> writel(0x0, priv->mram_base + i);
>
> - return 0;
> }
>
> static int m_can_plat_probe(struct platform_device *pdev)
> @@ -1246,38 +1457,85 @@ static int m_can_plat_probe(struct platform_device *pdev)
> struct m_can_priv *priv;
> struct resource *res;
> void __iomem *addr;
> + void __iomem *mram_addr;
> struct clk *hclk, *cclk;
> int irq, ret;
> + struct device_node *np;
> + u32 mram_config_vals[MRAM_CFG_LEN];
> + u32 tx_fifo_size;
> +
> + np = pdev->dev.of_node;
>
> hclk = devm_clk_get(&pdev->dev, "hclk");
> cclk = devm_clk_get(&pdev->dev, "cclk");
> +
> if (IS_ERR(hclk) || IS_ERR(cclk)) {
> dev_err(&pdev->dev, "no clock found\n");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto failed_ret;
> }
>
> + /* Enable clocks. Necessary to read Core Release in order to determine
> + * M_CAN version
> + */
> + ret = clk_prepare_enable(hclk);
> + if (ret)
> + goto disable_hclk_ret;
> +
> + ret = clk_prepare_enable(cclk);
> + if (ret)
> + goto disable_cclk_ret;
> +
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
> addr = devm_ioremap_resource(&pdev->dev, res);
> irq = platform_get_irq_byname(pdev, "int0");
> - if (IS_ERR(addr) || irq < 0)
> - return -EINVAL;
>
> - /* allocate the m_can device */
> - dev = alloc_m_can_dev();
> - if (!dev)
> - return -ENOMEM;
> + if (IS_ERR(addr) || irq < 0) {
> + ret = -EINVAL;
> + goto disable_cclk_ret;
> + }
> +
> + /* message ram could be shared */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> + if (!res) {
> + ret = -ENODEV;
> + goto disable_cclk_ret;
> + }
> +
> + mram_addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!mram_addr) {
> + ret = -ENOMEM;
> + goto disable_cclk_ret;
> + }
>
> + /* get message ram configuration */
> + ret = of_property_read_u32_array(np, "bosch,mram-cfg",
> + mram_config_vals,
> + sizeof(mram_config_vals) / 4);
> + if (ret) {
> + dev_err(&pdev->dev, "Could not get Message RAM configuration.");
> + goto disable_cclk_ret;
> + }
> +
> + /* Get TX FIFO size
> + * Defines the total amount of echo buffers for loopback
> + */
> + tx_fifo_size = mram_config_vals[7];
> +
> + /* allocate the m_can device */
> + dev = alloc_m_can_dev(addr, tx_fifo_size);
> + if (!dev) {
> + ret = -ENOMEM;
> + goto disable_cclk_ret;
> + }
> priv = netdev_priv(dev);
> dev->irq = irq;
> - priv->base = addr;
> priv->device = &pdev->dev;
> priv->hclk = hclk;
> priv->cclk = cclk;
> priv->can.clock.freq = clk_get_rate(cclk);
>
> - ret = m_can_of_parse_mram(pdev, priv);
> - if (ret)
> - goto failed_free_dev;
> + m_can_of_parse_mram(mram_addr, mram_config_vals, priv);
>
> platform_set_drvdata(pdev, dev);
> SET_NETDEV_DEV(dev, &pdev->dev);
> @@ -1294,10 +1552,18 @@ static int m_can_plat_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> KBUILD_MODNAME, priv->base, dev->irq);
>
> - return 0;
> + /* Probe finished
> + * Stop clocks. They will be reactivated once the M_CAN device is opened
> + */
> + goto disable_cclk_ret;
>
> failed_free_dev:
> free_m_can_dev(dev);
> +disable_cclk_ret:
> + clk_disable_unprepare(cclk);
> +disable_hclk_ret:
> + clk_disable_unprepare(hclk);
> +failed_ret:
> return ret;
> }
>
>
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
2017-03-10 14:00 ` [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x Mario Huettel
@ 2017-03-16 9:03 ` Oliver Hartkopp
2017-03-21 14:50 ` Marc Kleine-Budde
1 sibling, 0 replies; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-16 9:03 UTC (permalink / raw)
To: Mario Huettel, linux-can
On 10.03.2017 15:00, Mario Huettel wrote:
> * Added defines for TX Event FIFO Element
> * Adapted ndo_start_xmit function.
> For versions >= v3.1.x it uses the TX FIFO to optimize the data
> throughput. It stores the echo skb at the same index as in the
> M_CAN's TX FIFO. The frame's message marker is set to this index.
> This message marker is received in the TX Event FIFO after
> the message was successfully transmitted. It is used to echo the
> correct echo skb back to the network stack.
That's a very cool approach!
> * Added m_can_echo_tx_event function. It reads all received
> message markers in the TX Event FIFO and loops back the
> corresponding echo skbs.
> * ISR checks for new TX Event Entry interrupt for version >= 3.1.x.
> * Added spinlock to prevent concurrent accesses on the TX queue.
>
> Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
> ---
> drivers/net/can/m_can/m_can.c | 198 +++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 174 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 1538944..1a30d0e 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -23,6 +23,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/spinlock.h>
>
> #include <linux/can/dev.h>
>
> @@ -341,6 +342,11 @@ enum m_can_versions {
> #define TX_BUF_MM_SHIFT 24
> #define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
>
> +/* Tx event FIFO Element */
> +/* E1 */
> +#define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT
> +#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT)
> +
> /* address offset and element number for each FIFO/Buffer in the Message RAM */
> struct mram_cfg {
> u16 off;
> @@ -358,6 +364,8 @@ struct m_can_priv {
> void __iomem *base;
> u32 irqstatus;
> enum m_can_versions version;
> + /* Spinlock to protect TX queue */
> + spinlock_t xmit_lock;
>
> /* message ram configuration */
> void __iomem *mram_base;
> @@ -828,6 +836,44 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> return work_done;
> }
>
> +static void m_can_echo_tx_event(struct net_device *dev)
> +{
> + u32 txe_count = 0;
> + u32 m_can_txefs;
> + u32 fgi = 0;
> + int i = 0;
> + unsigned int msg_mark;
> +
> + struct m_can_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> +
> + /* read tx event fifo status */
> + m_can_txefs = m_can_read(priv, M_CAN_TXEFS);
> +
> + /* Get Tx Event fifo element count */
> + txe_count = (m_can_txefs & TXEFS_EFFL_MASK)
> + >> TXEFS_EFFL_SHIFT;
> +
> + /* Get and process all sent elements */
> + for (i = 0; i < txe_count; i++) {
> + /* retrieve get index */
> + fgi = (m_can_read(priv, M_CAN_TXEFS) & TXEFS_EFGI_MASK)
> + >> TXEFS_EFGI_SHIFT;
> +
> + /* get message marker */
> + msg_mark = (m_can_txe_fifo_read(priv, fgi, 4) &
> + TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
> +
> + /* ack txe element */
> + m_can_write(priv, M_CAN_TXEFA, (TXEFA_EFAI_MASK &
> + (fgi << TXEFA_EFAI_SHIFT)));
> +
> + /* update stats */
> + stats->tx_bytes += can_get_echo_skb(dev, msg_mark);
> + stats->tx_packets++;
> + }
> +}
> +
> static irqreturn_t m_can_isr(int irq, void *dev_id)
> {
> struct net_device *dev = (struct net_device *)dev_id;
> @@ -855,11 +901,19 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> }
>
> /* transmission complete interrupt */
> - if (ir & IR_TC) {
> + if ((ir & IR_TC) && priv->version == M_CAN_V30X) {
> stats->tx_bytes += can_get_echo_skb(dev, 0);
> stats->tx_packets++;
> can_led_event(dev, CAN_LED_EVENT_TX);
> netif_wake_queue(dev);
> + } else if ((ir & IR_TEFN) &&
> + (priv->version == M_CAN_V31X ||
> + priv->version == M_CAN_V32X)) {
> + /* New TX FIFO Element arrived */
> + m_can_echo_tx_event(dev);
> + can_led_event(dev, CAN_LED_EVENT_TX);
> + if (netif_queue_stopped(dev) && !m_can_tx_fifo_full(priv))
> + netif_wake_queue(dev);
> }
>
> return IRQ_HANDLED;
> @@ -1052,7 +1106,12 @@ static void m_can_chip_config(struct net_device *dev)
> /* enable interrupts */
> m_can_write(priv, M_CAN_IR, IR_ALL_INT);
> if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> - m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
> + if (priv->version == M_CAN_V30X)
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT &
> + ~(IR_ERR_LEC_30X));
> + else
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT &
> + ~(IR_ERR_LEC_31X));
> else
> m_can_write(priv, M_CAN_IE, IR_ALL_INT);
>
> @@ -1329,19 +1388,35 @@ static int m_can_close(struct net_device *dev)
> return 0;
> }
>
> +static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
> +{
> + struct m_can_priv *priv = netdev_priv(dev);
> + /*get wrap around for loopback skb index */
> + unsigned int wrap = priv->can.echo_skb_max;
> + int next_idx;
> +
> + /* calculate next index */
> + next_idx = (++putidx >= wrap ? 0 : putidx);
> +
> + /* check if occupied */
> + return !!priv->can.echo_skb[next_idx];
> +}
> +
> static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> struct m_can_priv *priv = netdev_priv(dev);
> struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> - u32 id, cccr;
> + u32 id, cccr, fdflags;
> int i;
> + int putidx;
> + unsigned long irqsave;
>
> if (can_dropped_invalid_skb(dev, skb))
> return NETDEV_TX_OK;
>
> - netif_stop_queue(dev);
> -
> + /* Generate ID field for TX buffer Element */
> + /* Common to all supported M_CAN versions */
> if (cf->can_id & CAN_EFF_FLAG) {
> id = cf->can_id & CAN_EFF_MASK;
> id |= TX_BUF_XTD;
> @@ -1352,33 +1427,105 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> if (cf->can_id & CAN_RTR_FLAG)
> id |= TX_BUF_RTR;
>
> - /* message ram configuration */
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, can_len2dlc(cf->len) << 16);
> + if (priv->version == M_CAN_V30X) {
> + netif_stop_queue(dev);
>
> - for (i = 0; i < cf->len; i += 4)
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
> - *(u32 *)(cf->data + i));
> + /* message ram configuration */
> + m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
> + m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC,
> + can_len2dlc(cf->len) << 16);
>
> - can_put_echo_skb(skb, dev, 0);
> + for (i = 0; i < cf->len; i += 4)
> + m_can_fifo_write(priv, 0,
> + M_CAN_FIFO_DATA(i / 4),
> + *(u32 *)(cf->data + i));
> +
> + can_put_echo_skb(skb, dev, 0);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> + cccr = m_can_read(priv, M_CAN_CCCR);
> + cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
> + if (can_is_canfd_skb(skb)) {
> + if (cf->flags & CANFD_BRS)
> + cccr |= CCCR_CMR_CANFD_BRS <<
> + CCCR_CMR_SHIFT;
> + else
> + cccr |= CCCR_CMR_CANFD <<
> + CCCR_CMR_SHIFT;
> + } else {
> + cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
> + }
> + m_can_write(priv, M_CAN_CCCR, cccr);
> + }
> + m_can_write(priv, M_CAN_TXBTIE, 0x1);
> + m_can_write(priv, M_CAN_TXBAR, 0x1);
> + /* End of xmit function for version 3.0.x */
> + } else if (priv->version == M_CAN_V31X ||
> + priv->version == M_CAN_V32X) {
Is it needed to check this very time in hot path?
Won't it make more sense to report an unsuccessful initialization at startup
instead of checking these IP versions here again and again?
> + /* Check if FIFO full */
> + if (m_can_tx_fifo_full(priv)) {
> + /* This shouldn't happen */
> + netif_stop_queue(dev);
> + netdev_warn(dev,
> + "TX queue active although FIFO is full.");
> + return NETDEV_TX_BUSY;
> + }
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> - cccr = m_can_read(priv, M_CAN_CCCR);
> - cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
> + /* get put index for frame */
> + putidx = ((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
> + >> TXFQS_TFQPI_SHIFT);
> + /* Write ID Field to FIFO Element */
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_ID, id);
> +
> + /* get CAN FD configuration of frame */
> + fdflags = 0;
> if (can_is_canfd_skb(skb)) {
> + fdflags |= TX_BUF_FDF;
> if (cf->flags & CANFD_BRS)
> - cccr |= CCCR_CMR_CANFD_BRS << CCCR_CMR_SHIFT;
> - else
> - cccr |= CCCR_CMR_CANFD << CCCR_CMR_SHIFT;
> - } else {
> - cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
> + fdflags |= TX_BUF_BRS;
> }
> - m_can_write(priv, M_CAN_CCCR, cccr);
> +
> + /* Construct DLC Field. Also contains CAN-FD configuration
> + * use put index of fifo as message marker
> + * it is used in TX interrupt for
> + * sending the correct echo frame
> + */
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_DLC,
> + ((putidx << TX_BUF_MM_SHIFT) &
> + TX_BUF_MM_MASK) |
> + (can_len2dlc(cf->len) << 16) |
> + fdflags | TX_BUF_EFC);
> +
> + for (i = 0; i < cf->len; i += 4)
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_DATA(i / 4),
> + *(u32 *)(cf->data + i));
> +
> + /* Push loopback echo.
> + * Will be looped back on TX interrupt based on message marker
> + */
> + can_put_echo_skb(skb, dev, putidx);
> +
> + /* Enable TX FIFO element to start transfer */
> + m_can_write(priv, M_CAN_TXBAR, (1 << putidx));
> +
> + /* wait until put index has changed */
> + while (((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
> + >> TXFQS_TFQPI_SHIFT) == putidx)
> + ;
> +
> + /* Lock spinlock for TX to prevent concurrent
> + * access on the network queue
> + */
> + spin_lock_irqsave(&priv->xmit_lock, irqsave);
> +
> + /* stop network queue if fifo full */
> + if (m_can_tx_fifo_full(priv) ||
> + m_can_next_echo_skb_occupied(dev, putidx))
> + netif_stop_queue(dev);
> +
> + spin_unlock_irqrestore(&priv->xmit_lock, irqsave);
> }
>
> - /* enable first TX buffer to start transfer */
> - m_can_write(priv, M_CAN_TXBTIE, 0x1);
> - m_can_write(priv, M_CAN_TXBAR, 0x1);
>
> return NETDEV_TX_OK;
> }
> @@ -1552,9 +1699,12 @@ static int m_can_plat_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> KBUILD_MODNAME, priv->base, dev->irq);
>
> + spin_lock_init(&priv->xmit_lock);
> +
> /* Probe finished
> * Stop clocks. They will be reactivated once the M_CAN device is opened
> */
> +
> goto disable_cclk_ret;
>
> failed_free_dev:
>
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
[not found] ` <c3d26079-554b-104d-4836-825a8496b66f@de.bosch.com>
@ 2017-03-17 17:52 ` Oliver Hartkopp
2017-03-18 13:27 ` Oliver Hartkopp
0 siblings, 1 reply; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-17 17:52 UTC (permalink / raw)
To: Mario.Huettel, linux-can
Hello Mario,
On 03/17/2017 12:06 PM, Mario.Huettel wrote:
> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>> CCCR_MON bit. In combination with TEST_LBCK it activates the internal
>>> loopback mode. Leaving CCCR_MON '0' results in external loopback mode.
>>
>> How is the interaction with the IFF_ECHO functionality then?
>
> It is the same as before. Internal loopback mode cuts the connection to
> the CAN RX and TX pins.
>
>
> However, sending a frame with enabled loopback results in following
> situation:
> * The socket that sent a frame receives the looped back frame
In fact it receives the RX frame and not the echo'ed frame, right?
> * Any other socket also receives the looped back frame but, in addition
> to that, they
> also receive the echo frame created by the driver. Thus the frame is
> received 2 times.
This is bad.
If I understood the frame flow correctly the 'non originating' socket
receives two frames:
1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
2. the frame received from the RX path from the hw loopback shortcut
I would suggest to drop the frame coming from the RX path (case 2) when
the CAN_CTRLMODE_LOOPBACK gets enabled.
> I also observe this behavior in the old driver.
Then it was always broken - and should be fixed.
Unfortunately I never had any hardware to test this.
>> Funny thing. So there are 3.2.x IP cores the only support the ISO mode?
>
> Yes. We leave the choice, whether to support Non-ISO operation, to the
> chip manufacturer.
> Many customers want a fixed ISO-behavior as it reduces the risk of a
> false configuration.
Interesting background.
> Regarding the code issues you highlighted. I fully agree with them and
> will fix them for patch v2.
Thanks.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-17 17:52 ` Oliver Hartkopp
@ 2017-03-18 13:27 ` Oliver Hartkopp
2017-03-19 13:59 ` Wolfgang Grandegger
0 siblings, 1 reply; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-18 13:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger; +Cc: Mario.Huettel, linux-can
Hi Marc, Wolfgang,
On 03/17/2017 06:52 PM, Oliver Hartkopp wrote:
> On 03/17/2017 12:06 PM, Mario.Huettel wrote:
>> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>>> CCCR_MON bit. In combination with TEST_LBCK it activates the internal
>>>> loopback mode. Leaving CCCR_MON '0' results in external loopback
>>>> mode.
>>>
>>> How is the interaction with the IFF_ECHO functionality then?
>>
>> It is the same as before. Internal loopback mode cuts the connection to
>> the CAN RX and TX pins.
>>
>> However, sending a frame with enabled loopback results in following
>> situation:
>> * The socket that sent a frame receives the looped back frame
>
> In fact it receives the RX frame and not the echo'ed frame, right?
>
>> * Any other socket also receives the looped back frame but, in addition
>> to that, they
>> also receive the echo frame created by the driver. Thus the frame is
>> received 2 times.
>
> This is bad.
>
> If I understood the frame flow correctly the 'non originating' socket
> receives two frames:
>
> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
> 2. the frame received from the RX path from the hw loopback shortcut
>
> I would suggest to drop the frame coming from the RX path (case 2) when
> the CAN_CTRLMODE_LOOPBACK gets enabled.
Do we have an agreement on this?
Did we ever discuss this topic in this detailed manner before?
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-18 13:27 ` Oliver Hartkopp
@ 2017-03-19 13:59 ` Wolfgang Grandegger
2017-03-19 14:44 ` Wolfgang Grandegger
2017-03-19 17:43 ` Oliver Hartkopp
0 siblings, 2 replies; 22+ messages in thread
From: Wolfgang Grandegger @ 2017-03-19 13:59 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
Hi Oliver,
Am 18.03.2017 um 14:27 schrieb Oliver Hartkopp:
> Hi Marc, Wolfgang,
>
> On 03/17/2017 06:52 PM, Oliver Hartkopp wrote:
>> On 03/17/2017 12:06 PM, Mario.Huettel wrote:
>>> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>>>> CCCR_MON bit. In combination with TEST_LBCK it activates the
>>>>> internal
>>>>> loopback mode. Leaving CCCR_MON '0' results in external loopback
>>>>> mode.
>>>>
>>>> How is the interaction with the IFF_ECHO functionality then?
>>>
>>> It is the same as before. Internal loopback mode cuts the connection to
>>> the CAN RX and TX pins.
>>>
>>> However, sending a frame with enabled loopback results in following
>>> situation:
>>> * The socket that sent a frame receives the looped back frame
>>
>> In fact it receives the RX frame and not the echo'ed frame, right?
>>
>>> * Any other socket also receives the looped back frame but, in addition
>>> to that, they
>>> also receive the echo frame created by the driver. Thus the frame is
>>> received 2 times.
>>
>> This is bad.
>>
>> If I understood the frame flow correctly the 'non originating' socket
>> receives two frames:
>>
>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>> 2. the frame received from the RX path from the hw loopback shortcut
Yep.
>> I would suggest to drop the frame coming from the RX path (case 2) when
>> the CAN_CTRLMODE_LOOPBACK gets enabled.
Why? Mario, what is you motivation/use case?
> Do we have an agreement on this?
> Did we ever discuss this topic in this detailed manner before?
As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
loopback to test the send and receive software (including echo skb). It
has noting to to with echo skbs. The loopback mode never got big
attention. I think it's rarely used and if, then just for testing
purposes. That's maybe the reason why this issue was never discussed.
There might even be hardware specific issues, e.g. some controller may
not even do the loopback like the SJA1000.
Wolfgang.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 13:59 ` Wolfgang Grandegger
@ 2017-03-19 14:44 ` Wolfgang Grandegger
2017-03-19 17:43 ` Oliver Hartkopp
1 sibling, 0 replies; 22+ messages in thread
From: Wolfgang Grandegger @ 2017-03-19 14:44 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
Am 19.03.2017 um 14:59 schrieb Wolfgang Grandegger:
> Hi Oliver,
>
> Am 18.03.2017 um 14:27 schrieb Oliver Hartkopp:
>> Hi Marc, Wolfgang,
>>
>> On 03/17/2017 06:52 PM, Oliver Hartkopp wrote:
>>> On 03/17/2017 12:06 PM, Mario.Huettel wrote:
>>>> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>>>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>>>>> CCCR_MON bit. In combination with TEST_LBCK it activates the
>>>>>> internal
>>>>>> loopback mode. Leaving CCCR_MON '0' results in external loopback
>>>>>> mode.
>>>>>
>>>>> How is the interaction with the IFF_ECHO functionality then?
>>>>
>>>> It is the same as before. Internal loopback mode cuts the connection to
>>>> the CAN RX and TX pins.
>>>>
>>>> However, sending a frame with enabled loopback results in following
>>>> situation:
>>>> * The socket that sent a frame receives the looped back frame
>>>
>>> In fact it receives the RX frame and not the echo'ed frame, right?
>>>
>>>> * Any other socket also receives the looped back frame but, in addition
>>>> to that, they
>>>> also receive the echo frame created by the driver. Thus the frame is
>>>> received 2 times.
>>>
>>> This is bad.
>>>
>>> If I understood the frame flow correctly the 'non originating' socket
>>> receives two frames:
>>>
>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>> 2. the frame received from the RX path from the hw loopback shortcut
>
> Yep.
>
>>> I would suggest to drop the frame coming from the RX path (case 2) when
>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>
> Why? Mario, what is you motivation/use case?
>
>> Do we have an agreement on this?
>> Did we ever discuss this topic in this detailed manner before?
>
> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
> loopback to test the send and receive software (including echo skb). It
> has noting to to with echo skbs. The loopback mode never got big
> attention. I think it's rarely used and if, then just for testing
> purposes. That's maybe the reason why this issue was never discussed.
> There might even be hardware specific issues, e.g. some controller may
> not even do the loopback like the SJA1000.
I just see that CAN_CTRLMODE_LOOPBACK is even undocumented :(, like most
other CTRLMODEs.
Wolfgang.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 13:59 ` Wolfgang Grandegger
2017-03-19 14:44 ` Wolfgang Grandegger
@ 2017-03-19 17:43 ` Oliver Hartkopp
2017-03-19 19:12 ` Wolfgang Grandegger
1 sibling, 1 reply; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-19 17:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
Hi Wolfgang,
On 03/19/2017 02:59 PM, Wolfgang Grandegger wrote:
> Am 18.03.2017 um 14:27 schrieb Oliver Hartkopp:
>> On 03/17/2017 06:52 PM, Oliver Hartkopp wrote:
>>> On 03/17/2017 12:06 PM, Mario.Huettel wrote:
>>>> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>>>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>>>>> CCCR_MON bit. In combination with TEST_LBCK it activates the
>>>>>> internal
>>>>>> loopback mode. Leaving CCCR_MON '0' results in external loopback
>>>>>> mode.
>>>>>
>>>>> How is the interaction with the IFF_ECHO functionality then?
>>>>
>>>> It is the same as before. Internal loopback mode cuts the connection to
>>>> the CAN RX and TX pins.
>>>>
>>>> However, sending a frame with enabled loopback results in following
>>>> situation:
>>>> * The socket that sent a frame receives the looped back frame
>>>
>>> In fact it receives the RX frame and not the echo'ed frame, right?
>>>
>>>> * Any other socket also receives the looped back frame but, in addition
>>>> to that, they
>>>> also receive the echo frame created by the driver. Thus the frame is
>>>> received 2 times.
>>>
>>> This is bad.
>>>
>>> If I understood the frame flow correctly the 'non originating' socket
>>> receives two frames:
>>>
>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>> 2. the frame received from the RX path from the hw loopback shortcut
>
> Yep.
>
>>> I would suggest to drop the frame coming from the RX path (case 2) when
>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>
> Why? Mario, what is you motivation/use case?
This was just my (Olivers) suggestion.
The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
using a virtual CAN interface.
>> Do we have an agreement on this?
>> Did we ever discuss this topic in this detailed manner before?
>
> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
> loopback to test the send and receive software (including echo skb). It
> has noting to to with echo skbs.
Agreed.
But the question is what happens to the IFF_ECHO 'mechanism' when the
CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
we get IFF_ECHO AND a reception due to the controllers loopback?
> The loopback mode never got big
> attention. I think it's rarely used and if, then just for testing
> purposes. That's maybe the reason why this issue was never discussed.
Right.
> There might even be hardware specific issues, e.g. some controller may
> not even do the loopback like the SJA1000.
Controllers that do not provide the loopback (like the SJA1000 or the
M_CAN) do not (must not) set CAN_CTRLMODE_LOOPBACK in the drivers flags.
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 17:43 ` Oliver Hartkopp
@ 2017-03-19 19:12 ` Wolfgang Grandegger
2017-03-19 20:16 ` Oliver Hartkopp
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Grandegger @ 2017-03-19 19:12 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
Am 19.03.2017 um 18:43 schrieb Oliver Hartkopp:
> Hi Wolfgang,
>
> On 03/19/2017 02:59 PM, Wolfgang Grandegger wrote:
>> Am 18.03.2017 um 14:27 schrieb Oliver Hartkopp:
>>> On 03/17/2017 06:52 PM, Oliver Hartkopp wrote:
>>>> On 03/17/2017 12:06 PM, Mario.Huettel wrote:
>>>>> On 03/16/2017 09:52 AM, Oliver Hartkopp wrote:
>>>>>>> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
>>>>>>> CCCR_MON bit. In combination with TEST_LBCK it activates the
>>>>>>> internal
>>>>>>> loopback mode. Leaving CCCR_MON '0' results in external loopback
>>>>>>> mode.
>>>>>>
>>>>>> How is the interaction with the IFF_ECHO functionality then?
>>>>>
>>>>> It is the same as before. Internal loopback mode cuts the
>>>>> connection to
>>>>> the CAN RX and TX pins.
>>>>>
>>>>> However, sending a frame with enabled loopback results in following
>>>>> situation:
>>>>> * The socket that sent a frame receives the looped back frame
>>>>
>>>> In fact it receives the RX frame and not the echo'ed frame, right?
>>>>
>>>>> * Any other socket also receives the looped back frame but, in
>>>>> addition
>>>>> to that, they
>>>>> also receive the echo frame created by the driver. Thus the
>>>>> frame is
>>>>> received 2 times.
>>>>
>>>> This is bad.
>>>>
>>>> If I understood the frame flow correctly the 'non originating' socket
>>>> receives two frames:
>>>>
>>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>>> 2. the frame received from the RX path from the hw loopback shortcut
>>
>> Yep.
>>
>>>> I would suggest to drop the frame coming from the RX path (case 2) when
>>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>>
>> Why? Mario, what is you motivation/use case?
>
> This was just my (Olivers) suggestion.
>
> The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
> using a virtual CAN interface.
>
>>> Do we have an agreement on this?
>>> Did we ever discuss this topic in this detailed manner before?
>>
>> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
>> loopback to test the send and receive software (including echo skb). It
>> has noting to to with echo skbs.
>
> Agreed.
>
> But the question is what happens to the IFF_ECHO 'mechanism' when the
> CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
Nothing special. The software does *not* known that the message was
looped back by the hardware. The TX and RX is treated a usual.
> Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
> we get IFF_ECHO AND a reception due to the controllers loopback?
The latter: The echoed message when TX is done and the RX (from the
hardware loopback).
>> The loopback mode never got big
>> attention. I think it's rarely used and if, then just for testing
>> purposes. That's maybe the reason why this issue was never discussed.
>
> Right.
>
>> There might even be hardware specific issues, e.g. some controller may
>> not even do the loopback like the SJA1000.
>
> Controllers that do not provide the loopback (like the SJA1000 or the
> M_CAN) do not (must not) set CAN_CTRLMODE_LOOPBACK in the drivers flags.
Yep, but the SJA1000 does support that feature.
Wolfgang.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 19:12 ` Wolfgang Grandegger
@ 2017-03-19 20:16 ` Oliver Hartkopp
2017-03-19 20:33 ` Wolfgang Grandegger
0 siblings, 1 reply; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-19 20:16 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
On 03/19/2017 08:12 PM, Wolfgang Grandegger wrote:
> Am 19.03.2017 um 18:43 schrieb Oliver Hartkopp:
>>>>> If I understood the frame flow correctly the 'non originating' socket
>>>>> receives two frames:
>>>>>
>>>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>>>> 2. the frame received from the RX path from the hw loopback shortcut
>>>
>>> Yep.
>>>
>>>>> I would suggest to drop the frame coming from the RX path (case 2)
>>>>> when
>>>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>>>
>>> Why? Mario, what is you motivation/use case?
>>
>> This was just my (Olivers) suggestion.
>>
>> The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
>> using a virtual CAN interface.
>>
>>>> Do we have an agreement on this?
>>>> Did we ever discuss this topic in this detailed manner before?
>>>
>>> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
>>> loopback to test the send and receive software (including echo skb). It
>>> has noting to to with echo skbs.
>>
>> Agreed.
>>
>> But the question is what happens to the IFF_ECHO 'mechanism' when the
>> CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
>
> Nothing special. The software does *not* known that the message was
> looped back by the hardware. The TX and RX is treated a usual.
>
>> Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
>> we get IFF_ECHO AND a reception due to the controllers loopback?
>
> The latter: The echoed message when TX is done and the RX (from the
> hardware loopback).
But this would be different to normal operation then!
When you have a second CAN interface on a real CAN bus that only ACKs
your frames but doesn't send own traffic, you only see your echo'ed
frames in the system (due to successful TX irq notification) - but no RX
frames.
>>> The loopback mode never got big
>>> attention. I think it's rarely used and if, then just for testing
>>> purposes. That's maybe the reason why this issue was never discussed.
So you vote for "The echoed message when TX is done and the RX (from the
hardware loopback)." - which reflects the current implementation in
SJA1000 and M_CAN.
But my question is:
Are developers using CAN_CTRLMODE_LOOPBACK to
1. Run their CAN controller without a working CAN bus hardware
2. To test the hardware loopback mode of the controller
For case 1 the RX frames should be dropped and only the echo'ed frames
should be provided to the system.
I hopefully was able to clearly describe my concern ...
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 20:16 ` Oliver Hartkopp
@ 2017-03-19 20:33 ` Wolfgang Grandegger
2017-03-21 10:30 ` Marc Kleine-Budde
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Grandegger @ 2017-03-19 20:33 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: Mario.Huettel, linux-can
Am 19.03.2017 um 21:16 schrieb Oliver Hartkopp:
>
>
> On 03/19/2017 08:12 PM, Wolfgang Grandegger wrote:
>> Am 19.03.2017 um 18:43 schrieb Oliver Hartkopp:
>
>>>>>> If I understood the frame flow correctly the 'non originating' socket
>>>>>> receives two frames:
>>>>>>
>>>>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>>>>> 2. the frame received from the RX path from the hw loopback shortcut
>>>>
>>>> Yep.
>>>>
>>>>>> I would suggest to drop the frame coming from the RX path (case 2)
>>>>>> when
>>>>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>>>>
>>>> Why? Mario, what is you motivation/use case?
>>>
>>> This was just my (Olivers) suggestion.
>>>
>>> The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
>>> using a virtual CAN interface.
>>>
>>>>> Do we have an agreement on this?
>>>>> Did we ever discuss this topic in this detailed manner before?
>>>>
>>>> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
>>>> loopback to test the send and receive software (including echo skb). It
>>>> has noting to to with echo skbs.
>>>
>>> Agreed.
>>>
>>> But the question is what happens to the IFF_ECHO 'mechanism' when the
>>> CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
>>
>> Nothing special. The software does *not* known that the message was
>> looped back by the hardware. The TX and RX is treated a usual.
>>
>>> Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
>>> we get IFF_ECHO AND a reception due to the controllers loopback?
>>
>> The latter: The echoed message when TX is done and the RX (from the
>> hardware loopback).
>
> But this would be different to normal operation then!
Depends on the intended usage. For testing if TX and RX software works
that's just fine.
> When you have a second CAN interface on a real CAN bus that only ACKs
> your frames but doesn't send own traffic, you only see your echo'ed
> frames in the system (due to successful TX irq notification) - but no RX
> frames.
Using the loopback as a virtual CAN device came never into my mind.
>>>> The loopback mode never got big
>>>> attention. I think it's rarely used and if, then just for testing
>>>> purposes. That's maybe the reason why this issue was never discussed.
>
> So you vote for "The echoed message when TX is done and the RX (from the
> hardware loopback)." - which reflects the current implementation in
> SJA1000 and M_CAN.
>
> But my question is:
>
> Are developers using CAN_CTRLMODE_LOOPBACK to
>
> 1. Run their CAN controller without a working CAN bus hardware
> 2. To test the hardware loopback mode of the controller
>
> For case 1 the RX frames should be dropped and only the echo'ed frames
> should be provided to the system.
Yes, feedback would really be useful.
In the past I used it for the following purpose. After configuring and
starting the controller, I'm not able to get messages out to the bus or
receive from it. To understand if it's a software or hardware problem, I
switch to loopback mode. If it still doesn't work, it's software.
>
> I hopefully was able to clearly describe my concern ...
Yes, there seem to be some confusion about loopback mode as I realize
from the following line.
http://lxr.free-electrons.com/source/Documentation/networking/can.txt#L1003
Wolfgang.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-19 20:33 ` Wolfgang Grandegger
@ 2017-03-21 10:30 ` Marc Kleine-Budde
2017-03-21 11:33 ` Oliver Hartkopp
0 siblings, 1 reply; 22+ messages in thread
From: Marc Kleine-Budde @ 2017-03-21 10:30 UTC (permalink / raw)
To: Wolfgang Grandegger, Oliver Hartkopp; +Cc: Mario.Huettel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 3926 bytes --]
On 03/19/2017 09:33 PM, Wolfgang Grandegger wrote:
> Am 19.03.2017 um 21:16 schrieb Oliver Hartkopp:
>>
>>
>> On 03/19/2017 08:12 PM, Wolfgang Grandegger wrote:
>>> Am 19.03.2017 um 18:43 schrieb Oliver Hartkopp:
>>
>>>>>>> If I understood the frame flow correctly the 'non originating' socket
>>>>>>> receives two frames:
>>>>>>>
>>>>>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>>>>>> 2. the frame received from the RX path from the hw loopback shortcut
>>>>>
>>>>> Yep.
>>>>>
>>>>>>> I would suggest to drop the frame coming from the RX path (case 2)
>>>>>>> when
>>>>>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>>>>>
>>>>> Why? Mario, what is you motivation/use case?
>>>>
>>>> This was just my (Olivers) suggestion.
>>>>
>>>> The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
>>>> using a virtual CAN interface.
>>>>
>>>>>> Do we have an agreement on this?
>>>>>> Did we ever discuss this topic in this detailed manner before?
>>>>>
>>>>> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
>>>>> loopback to test the send and receive software (including echo skb). It
>>>>> has noting to to with echo skbs.
>>>>
>>>> Agreed.
>>>>
>>>> But the question is what happens to the IFF_ECHO 'mechanism' when the
>>>> CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
>>>
>>> Nothing special. The software does *not* known that the message was
>>> looped back by the hardware. The TX and RX is treated a usual.
>>>
>>>> Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
>>>> we get IFF_ECHO AND a reception due to the controllers loopback?
>>>
>>> The latter: The echoed message when TX is done and the RX (from the
>>> hardware loopback).
>>
>> But this would be different to normal operation then!
>
> Depends on the intended usage. For testing if TX and RX software works
> that's just fine.
>
>> When you have a second CAN interface on a real CAN bus that only ACKs
>> your frames but doesn't send own traffic, you only see your echo'ed
>> frames in the system (due to successful TX irq notification) - but no RX
>> frames.
>
> Using the loopback as a virtual CAN device came never into my mind.
>
>>>>> The loopback mode never got big
>>>>> attention. I think it's rarely used and if, then just for testing
>>>>> purposes. That's maybe the reason why this issue was never discussed.
>>
>> So you vote for "The echoed message when TX is done and the RX (from the
>> hardware loopback)." - which reflects the current implementation in
>> SJA1000 and M_CAN.
>>
>> But my question is:
>>
>> Are developers using CAN_CTRLMODE_LOOPBACK to
>>
>> 1. Run their CAN controller without a working CAN bus hardware
>> 2. To test the hardware loopback mode of the controller
>>
>> For case 1 the RX frames should be dropped and only the echo'ed frames
>> should be provided to the system.
>
> Yes, feedback would really be useful.
>
> In the past I used it for the following purpose. After configuring and
> starting the controller, I'm not able to get messages out to the bus or
> receive from it. To understand if it's a software or hardware problem, I
> switch to loopback mode. If it still doesn't work, it's software.
From my point of view, the CAN_CTRLMODE_LOOPBACK mode should behave like
an external loopback connector, but controllable by software. Like a
short-circuit between RX and TX for RS232.
So you would receive the echo packges on TX and the "real" packages on
"RX". If you want a virtual CAN interface, make use of a vcan interace.
my 2¢,
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x
2017-03-10 14:00 ` [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x Mario Huettel
2017-03-16 8:41 ` Oliver Hartkopp
@ 2017-03-21 10:36 ` Marc Kleine-Budde
1 sibling, 0 replies; 22+ messages in thread
From: Marc Kleine-Budde @ 2017-03-21 10:36 UTC (permalink / raw)
To: Mario Huettel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 16332 bytes --]
On 03/10/2017 03:00 PM, Mario Huettel wrote:
> This patch includes following changes:
Please split this patch into 3:
> * Renamed the register defines of the M_CAN to fit version 3.1.x and above.
> * Replaced the old defines with the new ones in the whole code.
Put these two patches into the 3rd patch.
> * Removed code that enabled interrupt line 1. The driver didn't use it.
Can you put this into a separate patch, i.e. the first one.
> * Removed initialization of FIFO water marks. They were not used.
Move this into a seperate patch, too. Make it the 2nd.
>
> Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
> ---
> drivers/net/can/m_can/m_can.c | 197 +++++++++++++++++++++++++++---------------
> 1 file changed, 129 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 7a6554e..f2656a3 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -37,17 +37,19 @@ enum m_can_reg {
> M_CAN_CREL = 0x0,
> M_CAN_ENDN = 0x4,
> M_CAN_CUST = 0x8,
> - M_CAN_FBTP = 0xc,
> + M_CAN_DBTP = 0xc,
> M_CAN_TEST = 0x10,
> M_CAN_RWD = 0x14,
> M_CAN_CCCR = 0x18,
> - M_CAN_BTP = 0x1c,
> + M_CAN_NBTP = 0x1c,
> M_CAN_TSCC = 0x20,
> M_CAN_TSCV = 0x24,
> M_CAN_TOCC = 0x28,
> M_CAN_TOCV = 0x2c,
> M_CAN_ECR = 0x40,
> M_CAN_PSR = 0x44,
> +/* TDCR Register only available for version >=3.1.x */
> + M_CAN_TDCR = 0x48,
> M_CAN_IR = 0x50,
> M_CAN_IE = 0x54,
> M_CAN_ILS = 0x58,
> @@ -105,21 +107,21 @@ enum m_can_mram_cfg {
> MRAM_CFG_NUM,
> };
>
> -/* Fast Bit Timing & Prescaler Register (FBTP) */
> -#define FBTR_FBRP_MASK 0x1f
> -#define FBTR_FBRP_SHIFT 16
> -#define FBTR_FTSEG1_SHIFT 8
> -#define FBTR_FTSEG1_MASK (0xf << FBTR_FTSEG1_SHIFT)
> -#define FBTR_FTSEG2_SHIFT 4
> -#define FBTR_FTSEG2_MASK (0x7 << FBTR_FTSEG2_SHIFT)
> -#define FBTR_FSJW_SHIFT 0
> -#define FBTR_FSJW_MASK 0x3
> +/* Data Bit Timing & Prescaler Register (DBTP) */
> +#define DBTP_TDC BIT(23)
> +#define DBTP_DBRP_SHIFT 16
> +#define DBTP_DBRP_MASK (0x1f << DBTP_DBRP_SHIFT)
> +#define DBTP_DTSEG1_SHIFT 8
> +#define DBTP_DTSEG1_MASK (0x1f << DBTP_DTSEG1_SHIFT)
> +#define DBTP_DTSEG2_SHIFT 4
> +#define DBTP_DTSEG2_MASK (0xf << DBTP_DTSEG2_SHIFT)
> +#define DBTP_DSJW_SHIFT 0
> +#define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>
> /* Test Register (TEST) */
> -#define TEST_LBCK BIT(4)
> +#define TEST_LBCK BIT(4)
>
> /* CC Control Register(CCCR) */
> -#define CCCR_TEST BIT(7)
> #define CCCR_CMR_MASK 0x3
> #define CCCR_CMR_SHIFT 10
> #define CCCR_CMR_CANFD 0x1
> @@ -130,21 +132,32 @@ enum m_can_mram_cfg {
> #define CCCR_CME_CAN 0
> #define CCCR_CME_CANFD 0x1
> #define CCCR_CME_CANFD_BRS 0x2
> +#define CCCR_TXP BIT(14)
^^^^^^
please use a single space here
> #define CCCR_TEST BIT(7)
> #define CCCR_MON BIT(5)
> +#define CCCR_CSR BIT(4)
> +#define CCCR_CSA BIT(3)
> +#define CCCR_ASM BIT(2)
> #define CCCR_CCE BIT(1)
> #define CCCR_INIT BIT(0)
> #define CCCR_CANFD 0x10
> -
> -/* Bit Timing & Prescaler Register (BTP) */
> -#define BTR_BRP_MASK 0x3ff
> -#define BTR_BRP_SHIFT 16
> -#define BTR_TSEG1_SHIFT 8
> -#define BTR_TSEG1_MASK (0x3f << BTR_TSEG1_SHIFT)
> -#define BTR_TSEG2_SHIFT 4
> -#define BTR_TSEG2_MASK (0xf << BTR_TSEG2_SHIFT)
> -#define BTR_SJW_SHIFT 0
> -#define BTR_SJW_MASK 0xf
> +/* for version >=3.1.x */
> +#define CCCR_EFBI BIT(13)
> +#define CCCR_PXHD BIT(12)
> +#define CCCR_BRSE BIT(9)
> +#define CCCR_FDOE BIT(8)
> +/* only for version >=3.2.x */
> +#define CCCR_NISO BIT(15)
> +
> +/* Nominal Bit Timing & Prescaler Register (NBTP) */
> +#define NBTP_NSJW_SHIFT 25
> +#define NBTP_NSJW_MASK (0x7f << NBTP_NSJW_SHIFT)
> +#define NBTP_NBRP_SHIFT 16
> +#define NBTP_NBRP_MASK (0x1ff << NBTP_NBRP_SHIFT)
> +#define NBTP_NTSEG1_SHIFT 8
> +#define NBTP_NTSEG1_MASK (0xff << NBTP_NTSEG1_SHIFT)
> +#define NBTP_NTSEG2_SHIFT 0
> +#define NBTP_NTSEG2_MASK (0x7f << NBTP_NTSEG2_SHIFT)
>
> /* Error Counter Register(ECR) */
> #define ECR_RP BIT(15)
> @@ -161,6 +174,13 @@ enum m_can_mram_cfg {
>
> /* Interrupt Register(IR) */
> #define IR_ALL_INT 0xffffffff
> +
> +/* Renamed bits for versions > 3.1.x */
> +#define IR_ARA BIT(29)
> +#define IR_PED BIT(28)
> +#define IR_PEA BIT(27)
> +
> +/* Bits for version 3.0.x */
> #define IR_STE BIT(31)
> #define IR_FOE BIT(30)
> #define IR_ACKE BIT(29)
> @@ -194,33 +214,40 @@ enum m_can_mram_cfg {
> #define IR_RF0W BIT(1)
> #define IR_RF0N BIT(0)
> #define IR_ERR_STATE (IR_BO | IR_EW | IR_EP)
> -#define IR_ERR_LEC (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
> -#define IR_ERR_BUS (IR_ERR_LEC | IR_WDI | IR_ELO | IR_BEU | \
> +
> +/* Interrupts for version 3.0.x */
> +#define IR_ERR_LEC_30X (IR_STE | IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
> +#define IR_ERR_BUS_30X (IR_ERR_LEC_30X | IR_WDI | IR_ELO | IR_BEU | \
> + IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
> + IR_RF1L | IR_RF0L)
> +#define IR_ERR_ALL_30X (IR_ERR_STATE | IR_ERR_BUS_30X)
> +/* Interrupts for version >= 3.1.x */
> +#define IR_ERR_LEC_31X (IR_PED | IR_PEA)
> +#define IR_ERR_BUS_31X (IR_ERR_LEC_31X | IR_WDI | IR_ELO | IR_BEU | \
> IR_BEC | IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | \
> IR_RF1L | IR_RF0L)
> -#define IR_ERR_ALL (IR_ERR_STATE | IR_ERR_BUS)
> +#define IR_ERR_ALL_31X (IR_ERR_STATE | IR_ERR_BUS_31X)
>
> /* Interrupt Line Select (ILS) */
> #define ILS_ALL_INT0 0x0
> #define ILS_ALL_INT1 0xFFFFFFFF
>
> /* Interrupt Line Enable (ILE) */
> -#define ILE_EINT0 BIT(0)
> #define ILE_EINT1 BIT(1)
> +#define ILE_EINT0 BIT(0)
>
> /* Rx FIFO 0/1 Configuration (RXF0C/RXF1C) */
> -#define RXFC_FWM_OFF 24
> -#define RXFC_FWM_MASK 0x7f
> -#define RXFC_FWM_1 (1 << RXFC_FWM_OFF)
> -#define RXFC_FS_OFF 16
> -#define RXFC_FS_MASK 0x7f
> +#define RXFC_FWM_SHIFT 24
> +#define RXFC_FWM_MASK (0x7f < RXFC_FWM_SHIFT)
> +#define RXFC_FS_SHIFT 16
> +#define RXFC_FS_MASK (0x7f << RXFC_FS_SHIFT)
>
> /* Rx FIFO 0/1 Status (RXF0S/RXF1S) */
> #define RXFS_RFL BIT(25)
> #define RXFS_FF BIT(24)
> -#define RXFS_FPI_OFF 16
> +#define RXFS_FPI_SHIFT 16
> #define RXFS_FPI_MASK 0x3f0000
> -#define RXFS_FGI_OFF 8
> +#define RXFS_FGI_SHIFT 8
> #define RXFS_FGI_MASK 0x3f00
> #define RXFS_FFL_MASK 0x7f
>
> @@ -229,23 +256,46 @@ enum m_can_mram_cfg {
> #define M_CAN_RXESC_64BYTES 0x777
>
> /* Tx Buffer Configuration(TXBC) */
> -#define TXBC_NDTB_OFF 16
> -#define TXBC_NDTB_MASK 0x3f
> +#define TXBC_NDTB_SHIFT 16
> +#define TXBC_NDTB_MASK (0x3f << TXBC_NDTB_SHIFT)
> +#define TXBC_TFQS_SHIFT 24
> +#define TXBC_TFQS_MASK (0x3f << TXBC_TFQS_SHIFT)
> +
> +/* Tx FIFO/Queue Status (TXFQS) */
> +#define TXFQS_TFQF BIT(21)
> +#define TXFQS_TFQPI_SHIFT 16
> +#define TXFQS_TFQPI_MASK (0x1f << TXFQS_TFQPI_SHIFT)
> +#define TXFQS_TFGI_SHIFT 8
> +#define TXFQS_TFGI_MASK (0x1f << TXFQS_TFGI_SHIFT)
> +#define TXFQS_TFFL_SHIFT 0
> +#define TXFQS_TFFL_MASK (0x3f << TXFQS_TFFL_SHIFT)
>
> /* Tx Buffer Element Size Configuration(TXESC) */
> #define TXESC_TBDS_8BYTES 0x0
> #define TXESC_TBDS_64BYTES 0x7
>
> -/* Tx Event FIFO Con.guration (TXEFC) */
> -#define TXEFC_EFS_OFF 16
> -#define TXEFC_EFS_MASK 0x3f
> +/* Tx Event FIFO Configuration (TXEFC) */
> +#define TXEFC_EFS_SHIFT 16
> +#define TXEFC_EFS_MASK (0x3f << TXEFC_EFS_SHIFT)
> +
> +/* Tx Event FIFO Status (TXEFS) */
> +#define TXEFS_TEFL BIT(25)
> +#define TXEFS_EFF BIT(24)
> +#define TXEFS_EFGI_SHIFT 8
> +#define TXEFS_EFGI_MASK (0x1f << TXEFS_EFGI_SHIFT)
singe space after #define
> +#define TXEFS_EFFL_SHIFT 0
> +#define TXEFS_EFFL_MASK (0x3f << TXEFS_EFFL_SHIFT)
> +
> +/* Tx Event FIFO Acknowledge (TXEFA) */
> +#define TXEFA_EFAI_SHIFT 0
> +#define TXEFA_EFAI_MASK (0x1f << TXEFA_EFAI_SHIFT)
>
> /* Message RAM Configuration (in bytes) */
> #define SIDF_ELEMENT_SIZE 4
> #define XIDF_ELEMENT_SIZE 8
> #define RXF0_ELEMENT_SIZE 72
> #define RXF1_ELEMENT_SIZE 72
> -#define RXB_ELEMENT_SIZE 16
> +#define RXB_ELEMENT_SIZE 72
> #define TXE_ELEMENT_SIZE 8
> #define TXB_ELEMENT_SIZE 72
>
> @@ -261,13 +311,20 @@ enum m_can_mram_cfg {
> #define RX_BUF_RTR BIT(29)
> /* R1 */
> #define RX_BUF_ANMF BIT(31)
> -#define RX_BUF_EDL BIT(21)
> +#define RX_BUF_FDF BIT(21)
> #define RX_BUF_BRS BIT(20)
>
> /* Tx Buffer Element */
> -/* R0 */
> +/* T0 */
> +#define TX_BUF_ESI BIT(31)
> #define TX_BUF_XTD BIT(30)
> #define TX_BUF_RTR BIT(29)
> +/* T1 */
> +#define TX_BUF_EFC BIT(23)
> +#define TX_BUF_FDF BIT(21)
> +#define TX_BUF_BRS BIT(20)
> +#define TX_BUF_MM_SHIFT 24
> +#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
>
> /* address offset and element number for each FIFO/Buffer in the Message RAM */
> struct mram_cfg {
> @@ -349,7 +406,8 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,
>
> static inline void m_can_enable_all_interrupts(const struct m_can_priv *priv)
> {
> - m_can_write(priv, M_CAN_ILE, ILE_EINT0 | ILE_EINT1);
> + /* Only interrupt line 0 is used in this driver */
> + m_can_write(priv, M_CAN_ILE, ILE_EINT0);
> }
>
> static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)
> @@ -367,9 +425,9 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> int i;
>
> /* calculate the fifo get index for where to read data */
> - fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
> + fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_SHIFT;
> dlc = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> - if (dlc & RX_BUF_EDL)
> + if (dlc & RX_BUF_FDF)
> skb = alloc_canfd_skb(dev, &cf);
> else
> skb = alloc_can_skb(dev, (struct can_frame **)&cf);
> @@ -378,7 +436,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> return;
> }
>
> - if (dlc & RX_BUF_EDL)
> + if (dlc & RX_BUF_FDF)
> cf->len = can_dlc2len((dlc >> 16) & 0x0F);
> else
> cf->len = get_can_dlc((dlc >> 16) & 0x0F);
> @@ -394,7 +452,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> netdev_dbg(dev, "ESI Error\n");
> }
>
> - if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
> + if (!(dlc & RX_BUF_FDF) && (id & RX_BUF_RTR)) {
> cf->can_id |= CAN_RTR_FLAG;
> } else {
> if (dlc & RX_BUF_BRS)
> @@ -532,7 +590,7 @@ static int __m_can_get_berr_counter(const struct net_device *dev,
>
> ecr = m_can_read(priv, M_CAN_ECR);
> bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
> - bec->txerr = ecr & ECR_TEC_MASK;
> + bec->txerr = (ecr & ECR_TEC_MASK) >> ECR_TEC_SHIFT;
>
> return 0;
> }
> @@ -723,7 +781,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> if (irqstatus & IR_ERR_STATE)
> work_done += m_can_handle_state_errors(dev, psr);
>
> - if (irqstatus & IR_ERR_BUS)
> + if (irqstatus & IR_ERR_BUS_30X)
> work_done += m_can_handle_bus_errors(dev, irqstatus, psr);
>
> if (irqstatus & IR_RF0N)
> @@ -758,7 +816,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> * - state change IRQ
> * - bus error IRQ and bus error reporting
> */
> - if ((ir & IR_RF0N) || (ir & IR_ERR_ALL)) {
> + if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
> priv->irqstatus = ir;
> m_can_disable_all_interrupts(priv);
> napi_schedule(&priv->napi);
> @@ -811,19 +869,19 @@ static int m_can_set_bittiming(struct net_device *dev)
> sjw = bt->sjw - 1;
> tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> tseg2 = bt->phase_seg2 - 1;
> - reg_btp = (brp << BTR_BRP_SHIFT) | (sjw << BTR_SJW_SHIFT) |
> - (tseg1 << BTR_TSEG1_SHIFT) | (tseg2 << BTR_TSEG2_SHIFT);
> - m_can_write(priv, M_CAN_BTP, reg_btp);
> + reg_btp = (brp << NBTP_NBRP_SHIFT) | (sjw << NBTP_NSJW_SHIFT) |
> + (tseg1 << NBTP_NTSEG1_SHIFT) | (tseg2 << NBTP_NTSEG2_SHIFT);
> + m_can_write(priv, M_CAN_NBTP, reg_btp);
>
> if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> brp = dbt->brp - 1;
> sjw = dbt->sjw - 1;
> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> tseg2 = dbt->phase_seg2 - 1;
> - reg_btp = (brp << FBTR_FBRP_SHIFT) | (sjw << FBTR_FSJW_SHIFT) |
> - (tseg1 << FBTR_FTSEG1_SHIFT) |
> - (tseg2 << FBTR_FTSEG2_SHIFT);
> - m_can_write(priv, M_CAN_FBTP, reg_btp);
> + reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
> + (tseg1 << DBTP_DTSEG1_SHIFT) |
> + (tseg2 << DBTP_DTSEG2_SHIFT);
> + m_can_write(priv, M_CAN_DBTP, reg_btp);
> }
>
> return 0;
> @@ -851,23 +909,23 @@ static void m_can_chip_config(struct net_device *dev)
> m_can_write(priv, M_CAN_GFC, 0x0);
>
> /* only support one Tx Buffer currently */
> - m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_OFF) |
> + m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
> priv->mcfg[MRAM_TXB].off);
>
> /* support 64 bytes payload */
> m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
>
> - m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_OFF) |
> + m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
> priv->mcfg[MRAM_TXE].off);
>
> /* rx fifo configuration, blocking mode, fifo size 1 */
> m_can_write(priv, M_CAN_RXF0C,
> - (priv->mcfg[MRAM_RXF0].num << RXFC_FS_OFF) |
> - RXFC_FWM_1 | priv->mcfg[MRAM_RXF0].off);
> + (priv->mcfg[MRAM_RXF0].num << RXFC_FS_SHIFT) |
> + priv->mcfg[MRAM_RXF0].off);
>
> m_can_write(priv, M_CAN_RXF1C,
> - (priv->mcfg[MRAM_RXF1].num << RXFC_FS_OFF) |
> - RXFC_FWM_1 | priv->mcfg[MRAM_RXF1].off);
> + (priv->mcfg[MRAM_RXF1].num << RXFC_FS_SHIFT) |
> + priv->mcfg[MRAM_RXF1].off);
>
> cccr = m_can_read(priv, M_CAN_CCCR);
> cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> @@ -892,7 +950,7 @@ static void m_can_chip_config(struct net_device *dev)
> /* enable interrupts */
> m_can_write(priv, M_CAN_IR, IR_ALL_INT);
> if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> - m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC);
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
> else
> m_can_write(priv, M_CAN_IE, IR_ALL_INT);
>
> @@ -1143,10 +1201,12 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_XIDF].num = out_val[2];
> priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
> priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF0].num = out_val[3] & RXFC_FS_MASK;
> + priv->mcfg[MRAM_RXF0].num = out_val[3] &
> + (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
> priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF1].num = out_val[4] & RXFC_FS_MASK;
> + priv->mcfg[MRAM_RXF1].num = out_val[4] &
> + (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
> priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
> priv->mcfg[MRAM_RXB].num = out_val[5];
> @@ -1155,7 +1215,8 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_TXE].num = out_val[6];
> priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
> priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXB].num = out_val[7] & TXBC_NDTB_MASK;
> + priv->mcfg[MRAM_TXB].num = out_val[7] &
> + (TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
>
> dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> priv->mram_base,
> @@ -1191,7 +1252,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
> hclk = devm_clk_get(&pdev->dev, "hclk");
> cclk = devm_clk_get(&pdev->dev, "cclk");
> if (IS_ERR(hclk) || IS_ERR(cclk)) {
> - dev_err(&pdev->dev, "no clock find\n");
> + dev_err(&pdev->dev, "no clock found\n");
> return -ENODEV;
> }
>
>
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-21 10:30 ` Marc Kleine-Budde
@ 2017-03-21 11:33 ` Oliver Hartkopp
0 siblings, 0 replies; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-21 11:33 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger; +Cc: Mario.Huettel, linux-can
On 03/21/2017 11:30 AM, Marc Kleine-Budde wrote:
> On 03/19/2017 09:33 PM, Wolfgang Grandegger wrote:
>> Am 19.03.2017 um 21:16 schrieb Oliver Hartkopp:
>>>
>>>
>>> On 03/19/2017 08:12 PM, Wolfgang Grandegger wrote:
>>>> Am 19.03.2017 um 18:43 schrieb Oliver Hartkopp:
>>>
>>>>>>>> If I understood the frame flow correctly the 'non originating' socket
>>>>>>>> receives two frames:
>>>>>>>>
>>>>>>>> 1. the frame from the completed TX operation (echo_skb / IFF_ECHO)
>>>>>>>> 2. the frame received from the RX path from the hw loopback shortcut
>>>>>>
>>>>>> Yep.
>>>>>>
>>>>>>>> I would suggest to drop the frame coming from the RX path (case 2)
>>>>>>>> when
>>>>>>>> the CAN_CTRLMODE_LOOPBACK gets enabled.
>>>>>>
>>>>>> Why? Mario, what is you motivation/use case?
>>>>>
>>>>> This was just my (Olivers) suggestion.
>>>>>
>>>>> The effect with CAN_CTRLMODE_LOOPBACK enabled would then be similar to
>>>>> using a virtual CAN interface.
>>>>>
>>>>>>> Do we have an agreement on this?
>>>>>>> Did we ever discuss this topic in this detailed manner before?
>>>>>>
>>>>>> As I see it, the CAN_CTRLMODE_LOOPBACK provides a internal hardware
>>>>>> loopback to test the send and receive software (including echo skb). It
>>>>>> has noting to to with echo skbs.
>>>>>
>>>>> Agreed.
>>>>>
>>>>> But the question is what happens to the IFF_ECHO 'mechanism' when the
>>>>> CAN controller is switched into CAN_CTRLMODE_LOOPBACK?
>>>>
>>>> Nothing special. The software does *not* known that the message was
>>>> looped back by the hardware. The TX and RX is treated a usual.
>>>>
>>>>> Do we get a virtual CAN behavior (from IFF_ECHO on driver level) or do
>>>>> we get IFF_ECHO AND a reception due to the controllers loopback?
>>>>
>>>> The latter: The echoed message when TX is done and the RX (from the
>>>> hardware loopback).
>>>
>>> But this would be different to normal operation then!
>>
>> Depends on the intended usage. For testing if TX and RX software works
>> that's just fine.
>>
>>> When you have a second CAN interface on a real CAN bus that only ACKs
>>> your frames but doesn't send own traffic, you only see your echo'ed
>>> frames in the system (due to successful TX irq notification) - but no RX
>>> frames.
>>
>> Using the loopback as a virtual CAN device came never into my mind.
>>
>>>>>> The loopback mode never got big
>>>>>> attention. I think it's rarely used and if, then just for testing
>>>>>> purposes. That's maybe the reason why this issue was never discussed.
>>>
>>> So you vote for "The echoed message when TX is done and the RX (from the
>>> hardware loopback)." - which reflects the current implementation in
>>> SJA1000 and M_CAN.
>>>
>>> But my question is:
>>>
>>> Are developers using CAN_CTRLMODE_LOOPBACK to
>>>
>>> 1. Run their CAN controller without a working CAN bus hardware
>>> 2. To test the hardware loopback mode of the controller
>>>
>>> For case 1 the RX frames should be dropped and only the echo'ed frames
>>> should be provided to the system.
>>
>> Yes, feedback would really be useful.
>>
>> In the past I used it for the following purpose. After configuring and
>> starting the controller, I'm not able to get messages out to the bus or
>> receive from it. To understand if it's a software or hardware problem, I
>> switch to loopback mode. If it still doesn't work, it's software.
>
> From my point of view, the CAN_CTRLMODE_LOOPBACK mode should behave like
> an external loopback connector, but controllable by software. Like a
> short-circuit between RX and TX for RS232.
>
> So you would receive the echo packges on TX and the "real" packages on
> "RX". If you want a virtual CAN interface, make use of a vcan interace.
Ok. Then the posted implementation from Mario remains correct.
But it's good to have a common understanding about CAN_CTRLMODE_LOOPBACK
now.
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-10 14:00 ` [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization Mario Huettel
2017-03-16 8:52 ` Oliver Hartkopp
@ 2017-03-21 14:33 ` Marc Kleine-Budde
2017-03-21 19:26 ` Oliver Hartkopp
1 sibling, 1 reply; 22+ messages in thread
From: Marc Kleine-Budde @ 2017-03-21 14:33 UTC (permalink / raw)
To: Mario Huettel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 24711 bytes --]
On 03/10/2017 03:00 PM, Mario Huettel wrote:
> This patch adapts the initialization of the M_CAN. So it can be used with
> all versions >= 3.0.x.
>
> Changes:
> * Implemented new enum that represents the supported core versions.
> * Added version element to m_can_priv structure to hold M_CAN version.
> * Renamed bittiming structs for version 3.0.x
> * Added new bittiming structs for version >= 3.1.x
> * Function alloc_m_can_dev takes 2 new arguments. The TX FIFO size and the
> base address of the module.
> * Versions >= 3.1.x use multiple echo_skb buffers. This depends on the
> TX FIFO size configured in the device tree. Because this info is needed
> by alloc_m_can_dev, parts of the M_RAM configuration are moved before
> the call of alloc_m_can_dev. This also changes the parameters of the
> m_can_of_parse_mram function.
Please move the code, that adds support for TX_FIFO into a seperate patch.
> * Chip configuaration for CAN_CTRLMODE_LOOPBACK is changed: Enabled
> CCCR_MON bit. In combination with TEST_LBCK it activates the internal
> loopback mode. Leaving CCCR_MON '0' results in external loopback mode.
> * Clocks are temporarily enabled by platform_propbe function in order to
> allow read access to the Core Release register and the Control Register.
> Registers are used to detect M_CAN version and optional Non-ISO Feature.
> Initialization of M_CAN for version >= 3.1.x:
> * TX FIFO of M_CAN is used to transmit frames. The driver does not need to
> stop the tx queue after each frame sent.
> * Initialization of TX Event FIFO is added.
> * NON-ISO is fixed for all M_CAN versions < 3.2.x. Version 3.2.x _can_ have
> the NISO (Non-ISO) bit which can switch the mode of the M_CAN to Non-ISO
> mode. This bit does not have to be writeable. Therefore it is checked.
> If it is writable Non-ISO support is added to the controllers supported
> CAN modes.
>
> New Functions:
> * Function to read TXE (Transmit Event) FIFO. Will be used later.
> * Function to check, if TXE FIFO is full.
> * Function to check the Core Release version. The read value determines the
> behaviour of the driver.
> * Function to check if the NISO bit for version >= 3.2.x is implemented.
>
> Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
> ---
> drivers/net/can/m_can/m_can.c | 418 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 342 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index f2656a3..1538944 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -107,6 +107,21 @@ enum m_can_mram_cfg {
> MRAM_CFG_NUM,
> };
>
> +enum m_can_versions {
> + M_CAN_UNKNOWN_VERSION = 0,
> + M_CAN_V30X,
> + M_CAN_V31X,
> + M_CAN_V32X
> +};
Please use a common naming scheme here:
enum m_can_version {
M_CAN_VERSION_UNKNOWN,
M_CAN_VERSION_30X,
M_CAN_VERSION_31X,
...,
};
It's probably easier for the rest of the code to remove the
M_CAN_VERSION_UNKNOWN from that enum and handle unknown versions different.
> +
> +/* Core Release Register (CREL) */
> +#define CREL_REL_SHIFT 28
> +#define CREL_REL_MASK (0xF << CREL_REL_SHIFT)
> +#define CREL_STEP_SHIFT 24
> +#define CREL_STEP_MASK (0xF << CREL_STEP_SHIFT)
> +#define CREL_SUBSTEP_SHIFT 20
> +#define CREL_SUBSTEP_MASK (0xF << CREL_SUBSTEP_SHIFT)
> +
> /* Data Bit Timing & Prescaler Register (DBTP) */
> #define DBTP_TDC BIT(23)
> #define DBTP_DBRP_SHIFT 16
> @@ -342,6 +357,7 @@ struct m_can_priv {
> struct clk *cclk;
> void __iomem *base;
> u32 irqstatus;
> + enum m_can_versions version;
>
> /* message ram configuration */
> void __iomem *mram_base;
> @@ -373,6 +389,22 @@ static inline void m_can_fifo_write(const struct m_can_priv *priv,
> fpi * TXB_ELEMENT_SIZE + offset);
> }
>
> +static inline u32 m_can_txe_fifo_read(const struct m_can_priv *priv,
> + u32 fgi,
> + u32 offset) {
int or unsigned int instead of u32, please put the fgi and the offset
into one line.
Please put the '{' into a new line.
> + return readl(priv->mram_base + priv->mcfg[MRAM_TXE].off +
> + fgi * TXE_ELEMENT_SIZE + offset);
> +}
> +
> +/* Check if TX fifo is full
> + * returns 0 if free
> + * returns 1 if occupied
just return true/false
> + */
> +static inline int m_can_tx_fifo_full(const struct m_can_priv *priv)
bool
> +{
> + return !!(m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQF);
^^ can be removed if it's returning a bool
> +}
> +
> static inline void m_can_config_endisable(const struct m_can_priv *priv,
> bool enable)
> {
> @@ -833,7 +865,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static const struct can_bittiming_const m_can_bittiming_const = {
> +static const struct can_bittiming_const m_can_30X_bittiming_const = {
Please move the version to the end of the variable, e.g.:
m_can_bittiming_cost_30X
> .name = KBUILD_MODNAME,
> .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
> .tseg1_max = 64,
> @@ -845,7 +877,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> .brp_inc = 1,
> };
>
> -static const struct can_bittiming_const m_can_data_bittiming_const = {
> +static const struct can_bittiming_const m_can_30X_data_bittiming_const = {
> .name = KBUILD_MODNAME,
> .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
> .tseg1_max = 16,
> @@ -857,6 +889,30 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> .brp_inc = 1,
> };
>
> +static const struct can_bittiming_const m_can_31X_bittiming_const = {
> + .name = KBUILD_MODNAME,
> + .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
> + .tseg1_max = 256,
> + .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
> + .tseg2_max = 128,
> + .sjw_max = 128,
> + .brp_min = 1,
> + .brp_max = 512,
> + .brp_inc = 1,
> +};
> +
> +static const struct can_bittiming_const m_can_31X_data_bittiming_const = {
> + .name = KBUILD_MODNAME,
> + .tseg1_min = 1, /* Time segment 1 = prop_seg + phase_seg1 */
> + .tseg1_max = 32,
> + .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
> + .tseg2_max = 16,
> + .sjw_max = 16,
> + .brp_min = 1,
> + .brp_max = 32,
> + .brp_inc = 1,
> +};
> +
> static int m_can_set_bittiming(struct net_device *dev)
> {
> struct m_can_priv *priv = netdev_priv(dev);
> @@ -892,6 +948,7 @@ static int m_can_set_bittiming(struct net_device *dev)
> * - configure rx fifo
> * - accept non-matching frame into fifo 0
> * - configure tx buffer
> + * - >= v3.1.x: TX FIFO is used
> * - configure mode
> * - setup bittiming
> */
> @@ -908,15 +965,32 @@ static void m_can_chip_config(struct net_device *dev)
> /* Accept Non-matching Frames Into FIFO 0 */
> m_can_write(priv, M_CAN_GFC, 0x0);
>
> - /* only support one Tx Buffer currently */
> - m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
> - priv->mcfg[MRAM_TXB].off);
Use a switch statement here:
> + if (priv->version == M_CAN_V30X) {
> + /* only support one Tx Buffer currently */
> + m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_SHIFT) |
> + priv->mcfg[MRAM_TXB].off);
> + } else if (priv->version == M_CAN_V31X ||
> + priv->version == M_CAN_V32X) {
> + /* TX FIFO is used for newer IP Core versions */
> + m_can_write(priv, M_CAN_TXBC,
> + (priv->mcfg[MRAM_TXB].num << TXBC_TFQS_SHIFT) |
> + (priv->mcfg[MRAM_TXB].off));
> + }
>
> /* support 64 bytes payload */
> m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
>
> - m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
> - priv->mcfg[MRAM_TXE].off);
> + /* TX Event FIFO */
> + if (priv->version == M_CAN_V30X) {
use switch()
> + m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_SHIFT) |
> + priv->mcfg[MRAM_TXE].off);
> + } else if (priv->version == M_CAN_V31X || priv->version == M_CAN_V32X) {
> + /* Full TX Event FIFO is used */
> + m_can_write(priv, M_CAN_TXEFC,
> + ((priv->mcfg[MRAM_TXE].num << TXEFC_EFS_SHIFT)
> + & TXEFC_EFS_MASK) |
> + priv->mcfg[MRAM_TXE].off);
> + }
>
> /* rx fifo configuration, blocking mode, fifo size 1 */
> m_can_write(priv, M_CAN_RXF0C,
> @@ -928,22 +1002,50 @@ static void m_can_chip_config(struct net_device *dev)
> priv->mcfg[MRAM_RXF1].off);
>
> cccr = m_can_read(priv, M_CAN_CCCR);
> - cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> - (CCCR_CME_MASK << CCCR_CME_SHIFT));
> test = m_can_read(priv, M_CAN_TEST);
> test &= ~TEST_LBCK;
> + if (priv->version == M_CAN_V30X) {
switch()
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> - cccr |= CCCR_MON;
> + cccr &= ~(CCCR_TEST | CCCR_MON |
> + (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> + (CCCR_CME_MASK << CCCR_CME_SHIFT));
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> + cccr |= CCCR_TEST;
> + cccr |= CCCR_MON;
> + test |= TEST_LBCK;
> + }
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> + cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
> +
> + m_can_write(priv, M_CAN_CCCR, cccr);
> + m_can_write(priv, M_CAN_TEST, test);
> +
> + } else {
> + /* Version 3.1.X or 3.2.X */
> + cccr &= ~(CCCR_TEST | CCCR_MON | CCCR_BRSE | CCCR_FDOE);
> +
> + /* Only 3.2.X has NISO Bit implemented */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
> + cccr |= CCCR_NISO;
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> + cccr |= CCCR_TEST;
> + cccr |= CCCR_MON;
> + test |= TEST_LBCK;
> + }
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> + cccr |= (CCCR_BRSE | CCCR_FDOE);
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> - cccr |= CCCR_TEST;
> - test |= TEST_LBCK;
> }
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> - cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
> + /* Enable Monitoring (all versions) */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + cccr |= CCCR_MON;
>
> + /* Write config */
> m_can_write(priv, M_CAN_CCCR, cccr);
> m_can_write(priv, M_CAN_TEST, test);
>
> @@ -957,6 +1059,8 @@ static void m_can_chip_config(struct net_device *dev)
> /* route all interrupts to INT0 */
> m_can_write(priv, M_CAN_ILS, ILS_ALL_INT0);
>
> + /* Only 3.2.X has NISO Bit implemented */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
This if () looks bogogus.
> /* set bittiming params */
> m_can_set_bittiming(dev);
>
> @@ -994,33 +1098,160 @@ static void free_m_can_dev(struct net_device *dev)
> free_candev(dev);
> }
>
> -static struct net_device *alloc_m_can_dev(void)
> +static int m_can_check_core_release(void * __iomem m_can_base)
If you integrate this into the caller, you can get rid of the need for
"M_CAN_UNKNOWN_VERSION".
> +{
> + u32 crel_reg = 0;
> + u8 rel = 0;
> + u8 step = 0;
no need init init all these as 0.
> + int res = 0;
enum
> + struct m_can_priv temp_priv;
> +
> + temp_priv.base = m_can_base;
Although this is quite hacky, this looks a bit better:
struct m_can_priv priv = {
.base = m_can_base,
};
> +
> + /* Read Core Release Version and split into version number
> + * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1;
> + */
> + crel_reg = m_can_read((const struct m_can_priv *)&temp_priv,
> + M_CAN_CREL);
cast not needed
> + rel = (u8)((crel_reg & CREL_REL_MASK) >> CREL_REL_SHIFT);
> + step = (u8)((crel_reg & CREL_STEP_MASK) >> CREL_STEP_SHIFT);
cast not needed
> +
> + if (rel == 3) {
if (rel != 3)
return;
> + switch (step) {
> + case 0:
> + /* Version 3.0.x */
> + res = M_CAN_V30X;
> + break;
> + case 1:
> + /* Version 3.1.x */
> + res = M_CAN_V31X;
> + break;
> + case 2:
> + /* Version 3.2.x */
> + res = M_CAN_V32X;
> + break;
> + default:
> + /* Unknown version */
> + res = M_CAN_UNKNOWN_VERSION;
maybe print a dev_err() with the complete core release here?
> + break;
> + }
> + } else {
> + res = M_CAN_UNKNOWN_VERSION;
> + }
> +
and a dev_dbg() with the found core release here?
> + return res;
> +}
> +
> +/* Selectable Non ISO support only in version 3.2.x
> + * This function checks if the bit is writable.
> + */
Even if you have a v3.2.x you have to check if Non ISO is supported by
the core?
> +static int m_can_check_niso_support(const struct m_can_priv *priv)
static bool m_can_niso_supported()
> +{
> + u32 cccr_reg;
> + int timeout = 10;
> + int niso_support = 0;
> +
> + m_can_config_endisable(priv, true);
> + cccr_reg = m_can_read(priv, M_CAN_CCCR);
> + cccr_reg |= CCCR_NISO;
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> +
> + while ((m_can_read(priv, M_CAN_CCCR) & (CCCR_NISO)) == 0) {
> + if (timeout == 0) {
> + /* NISO Flag could not be set NONISO not supported */
> + /* Clear NISO to prevent errors */
> + cccr_reg &= ~(CCCR_NISO);
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> + niso_support = 0;
> + goto return_niso;
> + }
> + timeout--;
> + udelay(1);
make use of readl_poll_timeout(), see:
http://lxr.free-electrons.com/source/include/linux/iopoll.h
> + }
> + /* NISO Flag is set*/
> + niso_support = 1;
> + /* Clear NISO */
> + cccr_reg &= ~(CCCR_NISO);
> + m_can_write(priv, M_CAN_CCCR, cccr_reg);
> +
> +return_niso:
> + m_can_config_endisable(priv, false);
> + return niso_support;
> +}
> +
> +static struct net_device *alloc_m_can_dev(void __iomem *addr, u32 tx_fifo_size)
hmm, what about integrating this into the probe function, I find the
separation arbitrary.
> {
> struct net_device *dev;
> struct m_can_priv *priv;
> + int m_can_version = M_CAN_UNKNOWN_VERSION;
enum
> + int niso_bit_supported = 0;
> + unsigned int loopback_buffer_count;
> +
> + m_can_version = m_can_check_core_release(addr);
> + /* return if unsupported version */
> + if (m_can_version == M_CAN_UNKNOWN_VERSION) {
> + dev = NULL;
> + goto return_dev;
> + }
>
> - dev = alloc_candev(sizeof(*priv), 1);
> - if (!dev)
> - return NULL;
> + /* If version < 3.1.x, then only one loopback buffer is used */
> + loopback_buffer_count = ((m_can_version == M_CAN_V30X)
it's not a loopback_buffer it's a tx_buffer
> + ? 1U
> + : (unsigned int)tx_fifo_size);
^^^^^^^^^^^^^^
the cast should not be needed here
please use if (m_can_version == M_CAN_V30X), or a switch() here
>
> + dev = alloc_candev(sizeof(*priv), loopback_buffer_count);
> + if (!dev) {
> + dev = NULL;
> + goto return_dev;
> + }
> priv = netdev_priv(dev);
> netif_napi_add(dev, &priv->napi, m_can_poll, M_CAN_NAPI_WEIGHT);
>
> + /* Shared properties of all M_CAN versions */
> + priv->version = m_can_version;
> priv->dev = dev;
> - priv->can.bittiming_const = &m_can_bittiming_const;
> - priv->can.data_bittiming_const = &m_can_data_bittiming_const;
> + priv->base = addr;
> priv->can.do_set_mode = m_can_set_mode;
> priv->can.do_get_berr_counter = m_can_get_berr_counter;
>
> - /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */
> - can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
>
> - /* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */
> - priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> - CAN_CTRLMODE_LISTENONLY |
> - CAN_CTRLMODE_BERR_REPORTING |
> - CAN_CTRLMODE_FD;
>
> + /* Set properties depending on M_CAN version */
> + switch (priv->version) {
> + case M_CAN_V30X:
> + priv->can.bittiming_const = &m_can_30X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_30X_data_bittiming_const;
> + /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Is it fixed with 3.0 or 3.1?
> + can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
> + break;
> + case M_CAN_V31X:
> + /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> + can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
> + priv->can.bittiming_const = &m_can_31X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_31X_data_bittiming_const;
please reorder these assignments and calls to have the same order as in
case M_CAN_V30X.
> + break;
> + case M_CAN_V32X:
> + priv->can.bittiming_const = &m_can_31X_bittiming_const;
> + priv->can.data_bittiming_const =
> + &m_can_31X_data_bittiming_const;
> + niso_bit_supported = m_can_check_niso_support(priv);
why not directly set priv->can.ctrlmode_supported here?
if (m_can_niso_suppored(priv))
priv->can.ctrlmode_supported |=
CAN_CTRLMODE_FD_NON_ISO;
> + break;
> + default:
> + /* This does not happen */
> + break;
> + }
> +
> + /* Set M_CAN supported operations */
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_BERR_REPORTING |
> + CAN_CTRLMODE_FD |
> + (niso_bit_supported == 1
> + ? CAN_CTRLMODE_FD_NON_ISO
> + : 0);
Move this to the beginning, without the niso_bit_supported.
> +return_dev:
> return dev;
> }
>
> @@ -1167,58 +1398,39 @@ static int register_m_can_dev(struct net_device *dev)
> return register_candev(dev);
> }
>
> -static int m_can_of_parse_mram(struct platform_device *pdev,
> - struct m_can_priv *priv)
> +static void m_can_of_parse_mram(void __iomem *m_ram_base,
> + const u32 *mram_config_vals,
> + struct m_can_priv *priv)
please use priv as first pointer, assign the m_ram_base outside and pass
mram_config_vals as 2nd parameter.
> {
> - struct device_node *np = pdev->dev.of_node;
> - struct resource *res;
> - void __iomem *addr;
> - u32 out_val[MRAM_CFG_LEN];
> - int i, start, end, ret;
> -
> - /* message ram could be shared */
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> - if (!res)
> - return -ENODEV;
> + int i, start, end;
>
> - addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!addr)
> - return -ENOMEM;
> -
> - /* get message ram configuration */
> - ret = of_property_read_u32_array(np, "bosch,mram-cfg",
> - out_val, sizeof(out_val) / 4);
ARRAY_SIZE instead of "sizeof(out_val) / 4"
> - if (ret) {
> - dev_err(&pdev->dev, "can not get message ram configuration\n");
> - return -ENODEV;
> - }
> -
> - priv->mram_base = addr;
> - priv->mcfg[MRAM_SIDF].off = out_val[0];
> - priv->mcfg[MRAM_SIDF].num = out_val[1];
> + priv->mram_base = m_ram_base;
> + priv->mcfg[MRAM_SIDF].off = mram_config_vals[0];
> + priv->mcfg[MRAM_SIDF].num = mram_config_vals[1];
> priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +
> priv->mcfg[MRAM_SIDF].num * SIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_XIDF].num = out_val[2];
> + priv->mcfg[MRAM_XIDF].num = mram_config_vals[2];
> priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
> priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF0].num = out_val[3] &
> + priv->mcfg[MRAM_RXF0].num = mram_config_vals[3] &
> (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
> priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXF1].num = out_val[4] &
> + priv->mcfg[MRAM_RXF1].num = mram_config_vals[4] &
> (RXFC_FS_MASK >> RXFC_FS_SHIFT);
> priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
> priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
> - priv->mcfg[MRAM_RXB].num = out_val[5];
> + priv->mcfg[MRAM_RXB].num = mram_config_vals[5];
> priv->mcfg[MRAM_TXE].off = priv->mcfg[MRAM_RXB].off +
> priv->mcfg[MRAM_RXB].num * RXB_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXE].num = out_val[6];
> + priv->mcfg[MRAM_TXE].num = mram_config_vals[6];
> priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
> priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
> - priv->mcfg[MRAM_TXB].num = out_val[7] &
> + priv->mcfg[MRAM_TXB].num = mram_config_vals[7] &
> (TXBC_NDTB_MASK >> TXBC_NDTB_SHIFT);
>
> - dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> + dev_dbg(priv->device,
> + "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> priv->mram_base,
> priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num,
> priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num,
> @@ -1237,7 +1449,6 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> for (i = start; i < end; i += 4)
> writel(0x0, priv->mram_base + i);
>
> - return 0;
> }
>
> static int m_can_plat_probe(struct platform_device *pdev)
> @@ -1246,38 +1457,85 @@ static int m_can_plat_probe(struct platform_device *pdev)
> struct m_can_priv *priv;
> struct resource *res;
> void __iomem *addr;
> + void __iomem *mram_addr;
> struct clk *hclk, *cclk;
> int irq, ret;
> + struct device_node *np;
> + u32 mram_config_vals[MRAM_CFG_LEN];
> + u32 tx_fifo_size;
> +
> + np = pdev->dev.of_node;
>
> hclk = devm_clk_get(&pdev->dev, "hclk");
> cclk = devm_clk_get(&pdev->dev, "cclk");
> +
> if (IS_ERR(hclk) || IS_ERR(cclk)) {
> dev_err(&pdev->dev, "no clock found\n");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto failed_ret;
> }
>
> + /* Enable clocks. Necessary to read Core Release in order to determine
> + * M_CAN version
> + */
> + ret = clk_prepare_enable(hclk);
> + if (ret)
> + goto disable_hclk_ret;
> +
> + ret = clk_prepare_enable(cclk);
> + if (ret)
> + goto disable_cclk_ret;
> +
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
> addr = devm_ioremap_resource(&pdev->dev, res);
> irq = platform_get_irq_byname(pdev, "int0");
> - if (IS_ERR(addr) || irq < 0)
> - return -EINVAL;
>
> - /* allocate the m_can device */
> - dev = alloc_m_can_dev();
> - if (!dev)
> - return -ENOMEM;
> + if (IS_ERR(addr) || irq < 0) {
> + ret = -EINVAL;
> + goto disable_cclk_ret;
> + }
> +
> + /* message ram could be shared */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> + if (!res) {
> + ret = -ENODEV;
> + goto disable_cclk_ret;
> + }
> +
> + mram_addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!mram_addr) {
> + ret = -ENOMEM;
> + goto disable_cclk_ret;
> + }
>
> + /* get message ram configuration */
> + ret = of_property_read_u32_array(np, "bosch,mram-cfg",
> + mram_config_vals,
> + sizeof(mram_config_vals) / 4);
> + if (ret) {
> + dev_err(&pdev->dev, "Could not get Message RAM configuration.");
> + goto disable_cclk_ret;
> + }
> +
> + /* Get TX FIFO size
> + * Defines the total amount of echo buffers for loopback
> + */
> + tx_fifo_size = mram_config_vals[7];
> +
> + /* allocate the m_can device */
> + dev = alloc_m_can_dev(addr, tx_fifo_size);
> + if (!dev) {
> + ret = -ENOMEM;
> + goto disable_cclk_ret;
> + }
> priv = netdev_priv(dev);
> dev->irq = irq;
> - priv->base = addr;
> priv->device = &pdev->dev;
> priv->hclk = hclk;
> priv->cclk = cclk;
> priv->can.clock.freq = clk_get_rate(cclk);
>
> - ret = m_can_of_parse_mram(pdev, priv);
> - if (ret)
> - goto failed_free_dev;
> + m_can_of_parse_mram(mram_addr, mram_config_vals, priv);
>
> platform_set_drvdata(pdev, dev);
> SET_NETDEV_DEV(dev, &pdev->dev);
> @@ -1294,10 +1552,18 @@ static int m_can_plat_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> KBUILD_MODNAME, priv->base, dev->irq);
>
> - return 0;
> + /* Probe finished
> + * Stop clocks. They will be reactivated once the M_CAN device is opened
> + */
> + goto disable_cclk_ret;
>
> failed_free_dev:
> free_m_can_dev(dev);
> +disable_cclk_ret:
> + clk_disable_unprepare(cclk);
> +disable_hclk_ret:
> + clk_disable_unprepare(hclk);
> +failed_ret:
> return ret;
> }
>
>
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
2017-03-10 14:00 ` [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x Mario Huettel
2017-03-16 9:03 ` Oliver Hartkopp
@ 2017-03-21 14:50 ` Marc Kleine-Budde
1 sibling, 0 replies; 22+ messages in thread
From: Marc Kleine-Budde @ 2017-03-21 14:50 UTC (permalink / raw)
To: Mario Huettel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 11403 bytes --]
On 03/10/2017 03:00 PM, Mario Huettel wrote:
> * Added defines for TX Event FIFO Element
> * Adapted ndo_start_xmit function.
> For versions >= v3.1.x it uses the TX FIFO to optimize the data
> throughput. It stores the echo skb at the same index as in the
> M_CAN's TX FIFO. The frame's message marker is set to this index.
> This message marker is received in the TX Event FIFO after
> the message was successfully transmitted. It is used to echo the
> correct echo skb back to the network stack.
> * Added m_can_echo_tx_event function. It reads all received
> message markers in the TX Event FIFO and loops back the
> corresponding echo skbs.
Does the driver echo the CAN frames in the order you put them into the
FIFO and the HW sends them onto the wire?
> * ISR checks for new TX Event Entry interrupt for version >= 3.1.x.
> * Added spinlock to prevent concurrent accesses on the TX queue.
>
> Signed-off-by: Mario Huettel <mario.huettel@de.bosch.com>
> ---
> drivers/net/can/m_can/m_can.c | 198 +++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 174 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 1538944..1a30d0e 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -23,6 +23,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/spinlock.h>
>
> #include <linux/can/dev.h>
>
> @@ -341,6 +342,11 @@ enum m_can_versions {
> #define TX_BUF_MM_SHIFT 24
> #define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)
>
> +/* Tx event FIFO Element */
> +/* E1 */
> +#define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT
> +#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT)
> +
> /* address offset and element number for each FIFO/Buffer in the Message RAM */
> struct mram_cfg {
> u16 off;
> @@ -358,6 +364,8 @@ struct m_can_priv {
> void __iomem *base;
> u32 irqstatus;
> enum m_can_versions version;
> + /* Spinlock to protect TX queue */
> + spinlock_t xmit_lock;
>
> /* message ram configuration */
> void __iomem *mram_base;
> @@ -828,6 +836,44 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> return work_done;
> }
>
> +static void m_can_echo_tx_event(struct net_device *dev)
> +{
> + u32 txe_count = 0;
> + u32 m_can_txefs;
> + u32 fgi = 0;
> + int i = 0;
no need to init as 0
> + unsigned int msg_mark;
> +
no blank line here
> + struct m_can_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> +
> + /* read tx event fifo status */
> + m_can_txefs = m_can_read(priv, M_CAN_TXEFS);
> +
> + /* Get Tx Event fifo element count */
> + txe_count = (m_can_txefs & TXEFS_EFFL_MASK)
> + >> TXEFS_EFFL_SHIFT;
> +
> + /* Get and process all sent elements */
> + for (i = 0; i < txe_count; i++) {
Is this a true hardware fifo, so that you echo the elements in the order
you sent them?
> + /* retrieve get index */
> + fgi = (m_can_read(priv, M_CAN_TXEFS) & TXEFS_EFGI_MASK)
> + >> TXEFS_EFGI_SHIFT;
> +
> + /* get message marker */
> + msg_mark = (m_can_txe_fifo_read(priv, fgi, 4) &
> + TX_EVENT_MM_MASK) >> TX_EVENT_MM_SHIFT;
> +
> + /* ack txe element */
> + m_can_write(priv, M_CAN_TXEFA, (TXEFA_EFAI_MASK &
> + (fgi << TXEFA_EFAI_SHIFT)));
> +
> + /* update stats */
> + stats->tx_bytes += can_get_echo_skb(dev, msg_mark);
> + stats->tx_packets++;
> + }
> +}
> +
> static irqreturn_t m_can_isr(int irq, void *dev_id)
> {
> struct net_device *dev = (struct net_device *)dev_id;
> @@ -855,11 +901,19 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> }
>
> /* transmission complete interrupt */
make it a switch(priv->version) on the first level
> - if (ir & IR_TC) {
> + if ((ir & IR_TC) && priv->version == M_CAN_V30X) {
> stats->tx_bytes += can_get_echo_skb(dev, 0);
> stats->tx_packets++;
> can_led_event(dev, CAN_LED_EVENT_TX);
> netif_wake_queue(dev);
> + } else if ((ir & IR_TEFN) &&
> + (priv->version == M_CAN_V31X ||
> + priv->version == M_CAN_V32X)) {
> + /* New TX FIFO Element arrived */
> + m_can_echo_tx_event(dev);
> + can_led_event(dev, CAN_LED_EVENT_TX);
> + if (netif_queue_stopped(dev) && !m_can_tx_fifo_full(priv))
> + netif_wake_queue(dev);
> }
>
> return IRQ_HANDLED;
> @@ -1052,7 +1106,12 @@ static void m_can_chip_config(struct net_device *dev)
> /* enable interrupts */
> m_can_write(priv, M_CAN_IR, IR_ALL_INT);
> if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> - m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC_30X);
> + if (priv->version == M_CAN_V30X)
switch()
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT &
> + ~(IR_ERR_LEC_30X));
> + else
> + m_can_write(priv, M_CAN_IE, IR_ALL_INT &
> + ~(IR_ERR_LEC_31X));
> else
> m_can_write(priv, M_CAN_IE, IR_ALL_INT);
>
> @@ -1329,19 +1388,35 @@ static int m_can_close(struct net_device *dev)
> return 0;
> }
>
> +static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
> +{
> + struct m_can_priv *priv = netdev_priv(dev);
> + /*get wrap around for loopback skb index */
^^ space
> + unsigned int wrap = priv->can.echo_skb_max;
> + int next_idx;
> +
> + /* calculate next index */
> + next_idx = (++putidx >= wrap ? 0 : putidx);
> +
> + /* check if occupied */
> + return !!priv->can.echo_skb[next_idx];
> +}
> +
> static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> struct m_can_priv *priv = netdev_priv(dev);
> struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> - u32 id, cccr;
> + u32 id, cccr, fdflags;
> int i;
> + int putidx;
> + unsigned long irqsave;
>
> if (can_dropped_invalid_skb(dev, skb))
> return NETDEV_TX_OK;
>
> - netif_stop_queue(dev);
> -
> + /* Generate ID field for TX buffer Element */
> + /* Common to all supported M_CAN versions */
> if (cf->can_id & CAN_EFF_FLAG) {
> id = cf->can_id & CAN_EFF_MASK;
> id |= TX_BUF_XTD;
> @@ -1352,33 +1427,105 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> if (cf->can_id & CAN_RTR_FLAG)
> id |= TX_BUF_RTR;
>
> - /* message ram configuration */
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, can_len2dlc(cf->len) << 16);
> + if (priv->version == M_CAN_V30X) {
switch()
> + netif_stop_queue(dev);
>
> - for (i = 0; i < cf->len; i += 4)
> - m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
> - *(u32 *)(cf->data + i));
> + /* message ram configuration */
> + m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
> + m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC,
> + can_len2dlc(cf->len) << 16);
>
> - can_put_echo_skb(skb, dev, 0);
> + for (i = 0; i < cf->len; i += 4)
> + m_can_fifo_write(priv, 0,
> + M_CAN_FIFO_DATA(i / 4),
> + *(u32 *)(cf->data + i));
> +
> + can_put_echo_skb(skb, dev, 0);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> + cccr = m_can_read(priv, M_CAN_CCCR);
> + cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
> + if (can_is_canfd_skb(skb)) {
> + if (cf->flags & CANFD_BRS)
> + cccr |= CCCR_CMR_CANFD_BRS <<
> + CCCR_CMR_SHIFT;
> + else
> + cccr |= CCCR_CMR_CANFD <<
> + CCCR_CMR_SHIFT;
> + } else {
> + cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
> + }
> + m_can_write(priv, M_CAN_CCCR, cccr);
> + }
> + m_can_write(priv, M_CAN_TXBTIE, 0x1);
> + m_can_write(priv, M_CAN_TXBAR, 0x1);
> + /* End of xmit function for version 3.0.x */
maybe put this into a subfunction....
> + } else if (priv->version == M_CAN_V31X ||
> + priv->version == M_CAN_V32X) {
> + /* Check if FIFO full */
> + if (m_can_tx_fifo_full(priv)) {
> + /* This shouldn't happen */
> + netif_stop_queue(dev);
> + netdev_warn(dev,
> + "TX queue active although FIFO is full.");
> + return NETDEV_TX_BUSY;
> + }
>
> - if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> - cccr = m_can_read(priv, M_CAN_CCCR);
> - cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
> + /* get put index for frame */
> + putidx = ((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
> + >> TXFQS_TFQPI_SHIFT);
> + /* Write ID Field to FIFO Element */
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_ID, id);
> +
> + /* get CAN FD configuration of frame */
> + fdflags = 0;
> if (can_is_canfd_skb(skb)) {
> + fdflags |= TX_BUF_FDF;
> if (cf->flags & CANFD_BRS)
> - cccr |= CCCR_CMR_CANFD_BRS << CCCR_CMR_SHIFT;
> - else
> - cccr |= CCCR_CMR_CANFD << CCCR_CMR_SHIFT;
> - } else {
> - cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
> + fdflags |= TX_BUF_BRS;
> }
> - m_can_write(priv, M_CAN_CCCR, cccr);
> +
> + /* Construct DLC Field. Also contains CAN-FD configuration
> + * use put index of fifo as message marker
> + * it is used in TX interrupt for
> + * sending the correct echo frame
> + */
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_DLC,
> + ((putidx << TX_BUF_MM_SHIFT) &
> + TX_BUF_MM_MASK) |
> + (can_len2dlc(cf->len) << 16) |
> + fdflags | TX_BUF_EFC);
> +
> + for (i = 0; i < cf->len; i += 4)
> + m_can_fifo_write(priv, putidx, M_CAN_FIFO_DATA(i / 4),
> + *(u32 *)(cf->data + i));
> +
> + /* Push loopback echo.
> + * Will be looped back on TX interrupt based on message marker
> + */
> + can_put_echo_skb(skb, dev, putidx);
> +
> + /* Enable TX FIFO element to start transfer */
> + m_can_write(priv, M_CAN_TXBAR, (1 << putidx));
> +
> + /* wait until put index has changed */
> + while (((m_can_read(priv, M_CAN_TXFQS) & TXFQS_TFQPI_MASK)
> + >> TXFQS_TFQPI_SHIFT) == putidx)
> + ;
why is this needed? what does this do?
> +
> + /* Lock spinlock for TX to prevent concurrent
> + * access on the network queue
> + */
> + spin_lock_irqsave(&priv->xmit_lock, irqsave);
What does this lock protect? It's only used in the m_can_start_xmit()
function, which is only entered by a single instance of the networking
stack.
> +
> + /* stop network queue if fifo full */
> + if (m_can_tx_fifo_full(priv) ||
> + m_can_next_echo_skb_occupied(dev, putidx))
> + netif_stop_queue(dev);
> +
> + spin_unlock_irqrestore(&priv->xmit_lock, irqsave);
> }
>
> - /* enable first TX buffer to start transfer */
> - m_can_write(priv, M_CAN_TXBTIE, 0x1);
> - m_can_write(priv, M_CAN_TXBAR, 0x1);
>
> return NETDEV_TX_OK;
> }
> @@ -1552,9 +1699,12 @@ static int m_can_plat_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> KBUILD_MODNAME, priv->base, dev->irq);
>
> + spin_lock_init(&priv->xmit_lock);
> +
> /* Probe finished
> * Stop clocks. They will be reactivated once the M_CAN device is opened
> */
> +
> goto disable_cclk_ret;
>
> failed_free_dev:
>
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization
2017-03-21 14:33 ` Marc Kleine-Budde
@ 2017-03-21 19:26 ` Oliver Hartkopp
0 siblings, 0 replies; 22+ messages in thread
From: Oliver Hartkopp @ 2017-03-21 19:26 UTC (permalink / raw)
To: Marc Kleine-Budde, Mario Huettel, linux-can
On 03/21/2017 03:33 PM, Marc Kleine-Budde wrote:
>> +enum m_can_versions {
>> + M_CAN_UNKNOWN_VERSION = 0,
>> + M_CAN_V30X,
>> + M_CAN_V31X,
>> + M_CAN_V32X
>> +};
>
> Please use a common naming scheme here:
>
> enum m_can_version {
> M_CAN_VERSION_UNKNOWN,
> M_CAN_VERSION_30X,
> M_CAN_VERSION_31X,
> ...,
> };
Your should review the v2 patches %-)
Regards,
Oliver
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-03-21 19:27 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 14:00 [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > v3.0.x Mario Huettel
2017-03-10 14:00 ` [PATCH 0/3] can: m_can: Add driver support for M_CAN versions > 3.0.x Mario Huettel
2017-03-10 14:00 ` [PATCH 1/3] can: m_can: Support M_CAN IP versions > v3.0.x Mario Huettel
2017-03-16 8:41 ` Oliver Hartkopp
2017-03-21 10:36 ` Marc Kleine-Budde
2017-03-10 14:00 ` [PATCH 2/3] can: m_can: Enable M_CAN IP version dependent initialization Mario Huettel
2017-03-16 8:52 ` Oliver Hartkopp
[not found] ` <c3d26079-554b-104d-4836-825a8496b66f@de.bosch.com>
2017-03-17 17:52 ` Oliver Hartkopp
2017-03-18 13:27 ` Oliver Hartkopp
2017-03-19 13:59 ` Wolfgang Grandegger
2017-03-19 14:44 ` Wolfgang Grandegger
2017-03-19 17:43 ` Oliver Hartkopp
2017-03-19 19:12 ` Wolfgang Grandegger
2017-03-19 20:16 ` Oliver Hartkopp
2017-03-19 20:33 ` Wolfgang Grandegger
2017-03-21 10:30 ` Marc Kleine-Budde
2017-03-21 11:33 ` Oliver Hartkopp
2017-03-21 14:33 ` Marc Kleine-Budde
2017-03-21 19:26 ` Oliver Hartkopp
2017-03-10 14:00 ` [PATCH 3/3] can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x Mario Huettel
2017-03-16 9:03 ` Oliver Hartkopp
2017-03-21 14:50 ` Marc Kleine-Budde
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.