Subject: [PATCH v2 00/11] can: m_can: Optimizations for tcan and peripheral chips

Hi Marc and everyone,

as requested I split the series into two parts. This is the first parts
with simple improvements to reduce the number of SPI transfers. The
second part will be the rest with coalescing support and more complex
optimizations.

Changes in v2:
- Fixed register ranges
- Added fixes: tag for two patches

Sorry that I am one day later than promised.

Best,
Markus

Markus Schneider-Pargmann (11):
can: m_can: Eliminate double read of TXFQS in tx_handler
can: m_can: Avoid reading irqstatus twice
can: m_can: Read register PSR only on error
can: m_can: Count TXE FIFO getidx in the driver
can: m_can: Count read getindex in the driver
can: m_can: Batch acknowledge transmit events
can: m_can: Batch acknowledge rx fifo
can: tcan4x5x: Remove invalid write in clear_interrupts
can: tcan4x5x: Fix use of register error status mask
can: tcan4x5x: Fix register range of first two blocks
can: tcan4x5x: Specify separate read/write ranges

drivers/net/can/m_can/m_can.c | 90 +++++++++++++++----------
drivers/net/can/m_can/tcan4x5x-core.c | 19 ++----
drivers/net/can/m_can/tcan4x5x-regmap.c | 47 ++++++++++---
3 files changed, 100 insertions(+), 56 deletions(-)


base-commit: 76dcd734eca23168cb008912c0f69ff408905235
--
2.38.1


Subject: [PATCH v2 02/11] can: m_can: Avoid reading irqstatus twice

For peripheral devices the m_can_rx_handler is called directly after
setting cdev->irqstatus. This means we don't have to read the irqstatus
again in m_can_rx_handler. Avoid this by adding a parameter that is
false for direct calls.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 0cc0abde9b1d..d30afbb3503b 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -895,14 +895,13 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
return work_done;
}

-static int m_can_rx_handler(struct net_device *dev, int quota)
+static int m_can_rx_handler(struct net_device *dev, int quota, u32 irqstatus)
{
struct m_can_classdev *cdev = netdev_priv(dev);
int rx_work_or_err;
int work_done = 0;
- u32 irqstatus, psr;
+ u32 psr;

- irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
if (!irqstatus)
goto end;

@@ -946,12 +945,12 @@ static int m_can_rx_handler(struct net_device *dev, int quota)
return work_done;
}

-static int m_can_rx_peripheral(struct net_device *dev)
+static int m_can_rx_peripheral(struct net_device *dev, u32 irqstatus)
{
struct m_can_classdev *cdev = netdev_priv(dev);
int work_done;

- work_done = m_can_rx_handler(dev, NAPI_POLL_WEIGHT);
+ work_done = m_can_rx_handler(dev, NAPI_POLL_WEIGHT, irqstatus);

/* Don't re-enable interrupts if the driver had a fatal error
* (e.g., FIFO read failure).
@@ -967,8 +966,11 @@ static int m_can_poll(struct napi_struct *napi, int quota)
struct net_device *dev = napi->dev;
struct m_can_classdev *cdev = netdev_priv(dev);
int work_done;
+ u32 irqstatus;
+
+ irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);

- work_done = m_can_rx_handler(dev, quota);
+ work_done = m_can_rx_handler(dev, quota, irqstatus);

/* Don't re-enable interrupts if the driver had a fatal error
* (e.g., FIFO read failure).
@@ -1078,7 +1080,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
m_can_disable_all_interrupts(cdev);
if (!cdev->is_peripheral)
napi_schedule(&cdev->napi);
- else if (m_can_rx_peripheral(dev) < 0)
+ else if (m_can_rx_peripheral(dev, ir) < 0)
goto out_fail;
}

--
2.38.1

Subject: [PATCH v2 01/11] can: m_can: Eliminate double read of TXFQS in tx_handler

The TXFQS register is read first to check if the fifo is full and then
immediately again to get the putidx. This is unnecessary and adds
significant overhead if read requests are done over a slow bus, for
example SPI with tcan4x5x.

Add a variable to store the value of the register. Split the
m_can_tx_fifo_full function into two to avoid the hidden m_can_read call
if not needed.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index e5575d2755e4..0cc0abde9b1d 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -368,9 +368,14 @@ m_can_txe_fifo_read(struct m_can_classdev *cdev, u32 fgi, u32 offset, u32 *val)
return cdev->ops->read_fifo(cdev, addr_offset, val, 1);
}

+static inline bool _m_can_tx_fifo_full(u32 txfqs)
+{
+ return !!(txfqs & TXFQS_TFQF);
+}
+
static inline bool m_can_tx_fifo_full(struct m_can_classdev *cdev)
{
- return !!(m_can_read(cdev, M_CAN_TXFQS) & TXFQS_TFQF);
+ return _m_can_tx_fifo_full(m_can_read(cdev, M_CAN_TXFQS));
}

static void m_can_config_endisable(struct m_can_classdev *cdev, bool enable)
@@ -1585,6 +1590,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
struct sk_buff *skb = cdev->tx_skb;
struct id_and_dlc fifo_header;
u32 cccr, fdflags;
+ u32 txfqs;
int err;
int putidx;

@@ -1641,8 +1647,10 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
} else {
/* Transmit routine for version >= v3.1.x */

+ txfqs = m_can_read(cdev, M_CAN_TXFQS);
+
/* Check if FIFO full */
- if (m_can_tx_fifo_full(cdev)) {
+ if (_m_can_tx_fifo_full(txfqs)) {
/* This shouldn't happen */
netif_stop_queue(dev);
netdev_warn(dev,
@@ -1658,8 +1666,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
}

/* get put index for frame */
- putidx = FIELD_GET(TXFQS_TFQPI_MASK,
- m_can_read(cdev, M_CAN_TXFQS));
+ putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);

/* Construct DLC Field, with CAN-FD configuration.
* Use the put index of the fifo as the message marker,
--
2.38.1

Subject: [PATCH v2 06/11] can: m_can: Batch acknowledge transmit events

Transmit events from the txe fifo can be batch acknowledged by
acknowledging the last read txe fifo item. This will save txe_count
writes which is important for peripheral chips.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a0ae543d418c..5572a6b3b94c 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1013,7 +1013,9 @@ static int m_can_echo_tx_event(struct net_device *dev)
u32 txe_count = 0;
u32 m_can_txefs;
u32 fgi = 0;
+ int ack_fgi = -1;
int i = 0;
+ int err = 0;
unsigned int msg_mark;

struct m_can_classdev *cdev = netdev_priv(dev);
@@ -1028,28 +1030,29 @@ static int m_can_echo_tx_event(struct net_device *dev)
/* Get and process all sent elements */
for (i = 0; i < txe_count; i++) {
u32 txe, timestamp = 0;
- int err;

/* get message marker, timestamp */
err = m_can_txe_fifo_read(cdev, fgi, 4, &txe);
if (err) {
netdev_err(dev, "TXE FIFO read returned %d\n", err);
- return err;
+ break;
}

msg_mark = FIELD_GET(TX_EVENT_MM_MASK, txe);
timestamp = FIELD_GET(TX_EVENT_TXTS_MASK, txe) << 16;

- /* ack txe element */
- m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
- fgi));
+ ack_fgi = fgi;
fgi = (++fgi >= cdev->mcfg[MRAM_TXE].num ? 0 : fgi);

/* update stats */
m_can_tx_update_stats(cdev, msg_mark, timestamp);
}

- return 0;
+ if (ack_fgi != -1)
+ m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
+ ack_fgi));
+
+ return err;
}

static irqreturn_t m_can_isr(int irq, void *dev_id)
--
2.38.1

Subject: [PATCH v2 04/11] can: m_can: Count TXE FIFO getidx in the driver

The getindex simply increases by one for every iteration. There is no
need to get the current getidx every time from a register. Instead we
can just count and wrap if necessary.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 45c718413abf..a133f15fb90a 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1020,15 +1020,13 @@ static int m_can_echo_tx_event(struct net_device *dev)

/* Get Tx Event fifo element count */
txe_count = FIELD_GET(TXEFS_EFFL_MASK, m_can_txefs);
+ fgi = FIELD_GET(TXEFS_EFGI_MASK, m_can_txefs);

/* Get and process all sent elements */
for (i = 0; i < txe_count; i++) {
u32 txe, timestamp = 0;
int err;

- /* retrieve get index */
- fgi = FIELD_GET(TXEFS_EFGI_MASK, m_can_read(cdev, M_CAN_TXEFS));
-
/* get message marker, timestamp */
err = m_can_txe_fifo_read(cdev, fgi, 4, &txe);
if (err) {
@@ -1042,6 +1040,7 @@ static int m_can_echo_tx_event(struct net_device *dev)
/* ack txe element */
m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
fgi));
+ fgi = (++fgi >= cdev->mcfg[MRAM_TXE].num ? 0 : fgi);

/* update stats */
m_can_tx_update_stats(cdev, msg_mark, timestamp);
--
2.38.1

Subject: [PATCH v2 03/11] can: m_can: Read register PSR only on error

Only read register PSR if there is an error indicated in irqstatus.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index d30afbb3503b..45c718413abf 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -900,7 +900,6 @@ static int m_can_rx_handler(struct net_device *dev, int quota, u32 irqstatus)
struct m_can_classdev *cdev = netdev_priv(dev);
int rx_work_or_err;
int work_done = 0;
- u32 psr;

if (!irqstatus)
goto end;
@@ -926,13 +925,13 @@ static int m_can_rx_handler(struct net_device *dev, int quota, u32 irqstatus)
}
}

- psr = m_can_read(cdev, M_CAN_PSR);
-
if (irqstatus & IR_ERR_STATE)
- work_done += m_can_handle_state_errors(dev, psr);
+ work_done += m_can_handle_state_errors(dev,
+ m_can_read(cdev, M_CAN_PSR));

if (irqstatus & IR_ERR_BUS_30X)
- work_done += m_can_handle_bus_errors(dev, irqstatus, psr);
+ work_done += m_can_handle_bus_errors(dev, irqstatus,
+ m_can_read(cdev, M_CAN_PSR));

if (irqstatus & IR_RF0N) {
rx_work_or_err = m_can_do_rx_poll(dev, (quota - work_done));
--
2.38.1

Subject: [PATCH v2 08/11] can: tcan4x5x: Remove invalid write in clear_interrupts

Register 0x824 TCAN4X5X_MCAN_INT_REG is a read-only register. Any writes
to this register do not have any effect.

Remove this write. The m_can driver aldready clears the interrupts in
m_can_isr() by writing to M_CAN_IR which is translated to register
0x1050 which is a writable version of this register.

Fixes: 5443c226ba91 ("can: tcan4x5x: Add tcan4x5x driver to the kernel")
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---

Notes:
v2:
- Add fixes tag

drivers/net/can/m_can/tcan4x5x-core.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 41645a24384c..1fec394b3517 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -204,11 +204,6 @@ static int tcan4x5x_clear_interrupts(struct m_can_classdev *cdev)
if (ret)
return ret;

- ret = tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_MCAN_INT_REG,
- TCAN4X5X_ENABLE_MCAN_INT);
- if (ret)
- return ret;
-
ret = tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_INT_FLAGS,
TCAN4X5X_CLEAR_ALL_INT);
if (ret)
--
2.38.1

Subject: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

Specify exactly which registers are read/writeable in the chip. This
is supposed to help detect any violations in the future.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
index 33aed989e42a..2b218ce04e9f 100644
--- a/drivers/net/can/m_can/tcan4x5x-regmap.c
+++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
@@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
return 0;
}

-static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
+static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
+ /* Device ID and SPI Registers */
+ regmap_reg_range(0x000c, 0x0010),
+ /* Device configuration registers and Interrupt Flags*/
+ regmap_reg_range(0x0800, 0x080c),
+ regmap_reg_range(0x0814, 0x0814),
+ regmap_reg_range(0x0820, 0x0820),
+ regmap_reg_range(0x0830, 0x0830),
+ /* M_CAN */
+ regmap_reg_range(0x100c, 0x102c),
+ regmap_reg_range(0x1048, 0x1048),
+ regmap_reg_range(0x1050, 0x105c),
+ regmap_reg_range(0x1080, 0x1088),
+ regmap_reg_range(0x1090, 0x1090),
+ regmap_reg_range(0x1098, 0x10a0),
+ regmap_reg_range(0x10a8, 0x10b0),
+ regmap_reg_range(0x10b8, 0x10c0),
+ regmap_reg_range(0x10c8, 0x10c8),
+ regmap_reg_range(0x10d0, 0x10d4),
+ regmap_reg_range(0x10e0, 0x10e4),
+ regmap_reg_range(0x10f0, 0x10f0),
+ regmap_reg_range(0x10f8, 0x10f8),
+ /* MRAM */
+ regmap_reg_range(0x8000, 0x87fc),
+};
+
+static const struct regmap_range tcan4x5x_reg_table_rd_range[] = {
regmap_reg_range(0x0000, 0x0010), /* Device ID and SPI Registers */
regmap_reg_range(0x0800, 0x0830), /* Device configuration registers and Interrupt Flags*/
regmap_reg_range(0x1000, 0x10fc), /* M_CAN */
regmap_reg_range(0x8000, 0x87fc), /* MRAM */
};

-static const struct regmap_access_table tcan4x5x_reg_table = {
- .yes_ranges = tcan4x5x_reg_table_yes_range,
- .n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_yes_range),
+static const struct regmap_access_table tcan4x5x_reg_table_wr = {
+ .yes_ranges = tcan4x5x_reg_table_wr_range,
+ .n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_wr_range),
+};
+
+static const struct regmap_access_table tcan4x5x_reg_table_rd = {
+ .yes_ranges = tcan4x5x_reg_table_rd_range,
+ .n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_rd_range),
};

static const struct regmap_config tcan4x5x_regmap = {
@@ -107,8 +138,8 @@ static const struct regmap_config tcan4x5x_regmap = {
.reg_stride = 4,
.pad_bits = 8,
.val_bits = 32,
- .wr_table = &tcan4x5x_reg_table,
- .rd_table = &tcan4x5x_reg_table,
+ .wr_table = &tcan4x5x_reg_table_wr,
+ .rd_table = &tcan4x5x_reg_table_rd,
.max_register = TCAN4X5X_MAX_REGISTER,
.cache_type = REGCACHE_NONE,
.read_flag_mask = (__force unsigned long)
--
2.38.1

Subject: [PATCH v2 09/11] can: tcan4x5x: Fix use of register error status mask

TCAN4X5X_ERROR_STATUS is not a status register that needs clearing
during interrupt handling. Instead this is a masking register that masks
error interrupts. Writing TCAN4X5X_CLEAR_ALL_INT to this register
effectively masks everything.

Rename the register and mask all error interrupts only once by writing
to the register in tcan4x5x_init.

Fixes: 5443c226ba91 ("can: tcan4x5x: Add tcan4x5x driver to the kernel")
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---

Notes:
v2:
- Add fixes tag

drivers/net/can/m_can/tcan4x5x-core.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 1fec394b3517..efa2381bf85b 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -10,7 +10,7 @@
#define TCAN4X5X_DEV_ID1 0x04
#define TCAN4X5X_REV 0x08
#define TCAN4X5X_STATUS 0x0C
-#define TCAN4X5X_ERROR_STATUS 0x10
+#define TCAN4X5X_ERROR_STATUS_MASK 0x10
#define TCAN4X5X_CONTROL 0x14

#define TCAN4X5X_CONFIG 0x800
@@ -204,12 +204,7 @@ static int tcan4x5x_clear_interrupts(struct m_can_classdev *cdev)
if (ret)
return ret;

- ret = tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_INT_FLAGS,
- TCAN4X5X_CLEAR_ALL_INT);
- if (ret)
- return ret;
-
- return tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_ERROR_STATUS,
+ return tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_INT_FLAGS,
TCAN4X5X_CLEAR_ALL_INT);
}

@@ -229,6 +224,11 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)
if (ret)
return ret;

+ ret = tcan4x5x_write_tcan_reg(cdev, TCAN4X5X_ERROR_STATUS_MASK,
+ TCAN4X5X_CLEAR_ALL_INT);
+ if (ret)
+ return ret;
+
/* Zero out the MCAN buffers */
ret = m_can_init_ram(cdev);
if (ret)
--
2.38.1

Subject: [PATCH v2 10/11] can: tcan4x5x: Fix register range of first two blocks

According to the datasheet 0x10 is the last register in the first block,
not register 0x2c.

The datasheet lists the last register of the second block as 0x830, not
0x83c.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---

Notes:
v2:
- Fix end of first range, was 0x1c, is now 0x10
- Add fix for the end of the second range, was 0x3c, is now 0x30.

drivers/net/can/m_can/tcan4x5x-regmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
index 26e212b8ca7a..33aed989e42a 100644
--- a/drivers/net/can/m_can/tcan4x5x-regmap.c
+++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
@@ -91,8 +91,8 @@ static int tcan4x5x_regmap_read(void *context,
}

static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
- regmap_reg_range(0x0000, 0x002c), /* Device ID and SPI Registers */
- regmap_reg_range(0x0800, 0x083c), /* Device configuration registers and Interrupt Flags*/
+ regmap_reg_range(0x0000, 0x0010), /* Device ID and SPI Registers */
+ regmap_reg_range(0x0800, 0x0830), /* Device configuration registers and Interrupt Flags*/
regmap_reg_range(0x1000, 0x10fc), /* M_CAN */
regmap_reg_range(0x8000, 0x87fc), /* MRAM */
};
--
2.38.1

Subject: [PATCH v2 07/11] can: m_can: Batch acknowledge rx fifo

Instead of acknowledging every item of the fifo, only acknowledge the
last item read. This behavior is documented in the datasheet. The new
getindex will be the acknowledged item + 1.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 5572a6b3b94c..56f07f2023dd 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -529,9 +529,6 @@ static int m_can_read_fifo(struct net_device *dev, u32 fgi)
}
stats->rx_packets++;

- /* acknowledge rx fifo 0 */
- m_can_write(cdev, M_CAN_RXF0A, fgi);
-
timestamp = FIELD_GET(RX_BUF_RXTS_MASK, fifo_header.dlc) << 16;

m_can_receive_skb(cdev, skb, timestamp);
@@ -552,8 +549,9 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
u32 rxfs;
u32 rx_count;
u32 fgi;
+ int ack_fgi = -1;
int i;
- int err;
+ int err = 0;

rxfs = m_can_read(cdev, M_CAN_RXF0S);
if (!(rxfs & RXFS_FFL_MASK)) {
@@ -567,13 +565,20 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
for (i = 0; i < rx_count && quota > 0; ++i) {
err = m_can_read_fifo(dev, fgi);
if (err)
- return err;
+ break;

quota--;
pkts++;
+ ack_fgi = fgi;
fgi = (++fgi >= cdev->mcfg[MRAM_RXF0].num ? 0 : fgi);
}

+ if (ack_fgi != -1)
+ m_can_write(cdev, M_CAN_RXF0A, ack_fgi);
+
+ if (err)
+ return err;
+
return pkts;
}

--
2.38.1

Subject: [PATCH v2 05/11] can: m_can: Count read getindex in the driver

The getindex gets increased by one every time. We can calculate the
correct getindex in the driver and avoid the additional reads of rxfs.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a133f15fb90a..a0ae543d418c 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -476,19 +476,16 @@ static void m_can_receive_skb(struct m_can_classdev *cdev,
}
}

-static int m_can_read_fifo(struct net_device *dev, u32 rxfs)
+static int m_can_read_fifo(struct net_device *dev, u32 fgi)
{
struct net_device_stats *stats = &dev->stats;
struct m_can_classdev *cdev = netdev_priv(dev);
struct canfd_frame *cf;
struct sk_buff *skb;
struct id_and_dlc fifo_header;
- u32 fgi;
u32 timestamp = 0;
int err;

- /* calculate the fifo get index for where to read data */
- fgi = FIELD_GET(RXFS_FGI_MASK, rxfs);
err = m_can_fifo_read(cdev, fgi, M_CAN_FIFO_ID, &fifo_header, 2);
if (err)
goto out_fail;
@@ -553,6 +550,9 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
struct m_can_classdev *cdev = netdev_priv(dev);
u32 pkts = 0;
u32 rxfs;
+ u32 rx_count;
+ u32 fgi;
+ int i;
int err;

rxfs = m_can_read(cdev, M_CAN_RXF0S);
@@ -561,14 +561,17 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
return 0;
}

- while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
- err = m_can_read_fifo(dev, rxfs);
+ rx_count = FIELD_GET(RXFS_FFL_MASK, rxfs);
+ fgi = FIELD_GET(RXFS_FGI_MASK, rxfs);
+
+ for (i = 0; i < rx_count && quota > 0; ++i) {
+ err = m_can_read_fifo(dev, fgi);
if (err)
return err;

quota--;
pkts++;
- rxfs = m_can_read(cdev, M_CAN_RXF0S);
+ fgi = (++fgi >= cdev->mcfg[MRAM_RXF0].num ? 0 : fgi);
}

return pkts;
--
2.38.1

2022-12-06 17:02:41

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> Specify exactly which registers are read/writeable in the chip. This
> is supposed to help detect any violations in the future.
>
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> 1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> index 33aed989e42a..2b218ce04e9f 100644
> --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> return 0;
> }
>
> -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> + /* Device ID and SPI Registers */
> + regmap_reg_range(0x000c, 0x0010),

According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
:/.

> + /* Device configuration registers and Interrupt Flags*/
> + regmap_reg_range(0x0800, 0x080c),
> + regmap_reg_range(0x0814, 0x0814),

0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
2022"?

Marc

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

Attachments:
(No filename) (1.57 kB)
 signature.asc (499.00 B)
 Download all attachments

2022-12-12 11:34:06

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > Specify exactly which registers are read/writeable in the chip. This
> > is supposed to help detect any violations in the future.
> >
> > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > ---
> > drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> > 1 file changed, 37 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > index 33aed989e42a..2b218ce04e9f 100644
> > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> > return 0;
> > }
> >
> > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > + /* Device ID and SPI Registers */
> > + regmap_reg_range(0x000c, 0x0010),
>
> According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> :/.
>
> > + /* Device configuration registers and Interrupt Flags*/
> > + regmap_reg_range(0x0800, 0x080c),
> > + regmap_reg_range(0x0814, 0x0814),
>
> 0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
> 2022"?

I'll take the series as is, that can be fixed later.

regards,
Marc

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

Attachments:
(No filename) (1.75 kB)
 signature.asc (499.00 B)
 Download all attachments
Subject: Re: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

Hi Marc,

sorry for the delay.

On Mon, Dec 12, 2022 at 11:54:44AM +0100, Marc Kleine-Budde wrote:
> On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> > On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > > Specify exactly which registers are read/writeable in the chip. This
> > > is supposed to help detect any violations in the future.
> > >
> > > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > > ---
> > > drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> > > 1 file changed, 37 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > index 33aed989e42a..2b218ce04e9f 100644
> > > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> > > return 0;
> > > }
> > >
> > > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > > + /* Device ID and SPI Registers */
> > > + regmap_reg_range(0x000c, 0x0010),
> >
> > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > :/.

I am trying to clarify this. I guess table 8-8 is not correct, but we
will see.

> >
> > > + /* Device configuration registers and Interrupt Flags*/
> > > + regmap_reg_range(0x0800, 0x080c),
> > > + regmap_reg_range(0x0814, 0x0814),
> >
> > 0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
> > 2022"?
>

Yes that's a mistake, sorry. I will add a fixup for the upcoming series.

> I'll take the series as is, that can be fixed later.

Thank you!.

Best,
Markus

2022-12-13 19:26:28

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

On 13.12.2022 20:10:25, Markus Schneider-Pargmann wrote:
> > > > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > > > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > > > :/.
> >
> > I am trying to clarify this. I guess table 8-8 is not correct, but we
> > will see.
>
> So it is indeed a typo in table 8-8. The register is R/W.

Do you have a contact to TI to fix this?

Marc

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

Attachments:
(No filename) (716.00 B)
 signature.asc (499.00 B)
 Download all attachments
Subject: Re: [PATCH v2 11/11] can: tcan4x5x: Specify separate read/write ranges

Hi Marc,

On Tue, Dec 13, 2022 at 06:10:36PM +0100, Markus Schneider-Pargmann wrote:
> Hi Marc,
>
> sorry for the delay.
>
> On Mon, Dec 12, 2022 at 11:54:44AM +0100, Marc Kleine-Budde wrote:
> > On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> > > On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > > > Specify exactly which registers are read/writeable in the chip. This
> > > > is supposed to help detect any violations in the future.
> > > >
> > > > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > > > ---
> > > > drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> > > > 1 file changed, 37 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > index 33aed989e42a..2b218ce04e9f 100644
> > > > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> > > > return 0;
> > > > }
> > > >
> > > > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > > > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > > > + /* Device ID and SPI Registers */
> > > > + regmap_reg_range(0x000c, 0x0010),
> > >
> > > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > > :/.
>
> I am trying to clarify this. I guess table 8-8 is not correct, but we
> will see.

So it is indeed a typo in table 8-8. The register is R/W.

Best,
Markus