[PATCH v2 00/10] can: rcar_canfd: Add support for Transceiver Delay Compensation

[PATCH v2 00/10] can: rcar_canfd: Add support for Transceiver Delay Compensation

[Geert Uytterhoeven]

	Hi all,

This patch series adds CAN-FD Transceiver Delay Compensation support to
the R-Car CAN-FD driver, after the customary cleanups and refactorings.

Changes compared to v1:
  - Dropped patch "can: rcar_canfd: Use ndev parameter in
    rcar_canfd_set_bittiming()",
  - New patch "[PATCH v2 02/10] can: rcar_canfd: Remove bittiming debug
    prints",
  - New patch "[PATCH v2 07/10] can: rcar_canfd: Rename
    rcar_canfd_setrnc() to rcar_canfd_set_rnc()",
  - Add Reviewed-by,
  - Replace function-like RCANFD_F_*() macros by rcar_canfd_f_*()
    inline functions,
  - Replace function-like macro RCANFD_FDSTS_TDCR() by bitmask
    RCANFD_FDSTS_TDCR and helper function rcar_canfd_get_tdcr(),
  - Replace function-like macro RCANFD_FDSTS_TDCVF() by two bit
    definitions,
  - Drop debug print of tdc mode and tdco value.

This has been tested on R-Car V4H (White Hawk), V4M (Gray Hawk Single),
and E3 (Ebisu-4D[2]), using various data bit rates.  Without proper TDC
configuration, transmitting at 8 Mbps makes the CAN-FD controller enter
BUS-OFF state.  The TDCV value as measured by the CAN-FD controller is 4
on all boards tested (base clock 40 MHz, i.e. 25 ns period), and ca. 90
ns as measured by a logic analyzer on Gray Hawk Single.

Note that the BSP (predating upstream TDC support), uses a much simpler
method: for transfer rates >= 5 Mbps on R-Car Gen4, it enables TDC with
a hardcoded (hardware) TDCO value of 2 (i.e. actual 3), which matches
the behavior of this series at 8 Mbps.

Thanks for your comments!

[1] "[PATCH 0/9] can: rcar_canfd: Add support for Transceiver Delay Compensation"
    https://lore.kernel.org/cover.1748863848.git.geert+renesas@glider.be

[2] r8a77990.dtsi configures the CANFD core clock to 40 MHz, limiting
    transfer rates to 4 Mbps.  Enable support for 8 Mbps by adding to
    ebisu.dtsi:

	&canfd {
		assigned-clock-rates = <80000000>;
	}

    I plan to send patches to update this on all R-Car Gen3 and RZ/G2
    SoCs once this series has reached upstream.

Geert Uytterhoeven (10):
  can: rcar_canfd: Consistently use ndev for net_device pointers
  can: rcar_canfd: Remove bittiming debug prints
  can: rcar_canfd: Add helper variable ndev to rcar_canfd_rx_pkt()
  can: rcar_canfd: Add helper variable dev to
    rcar_canfd_reset_controller()
  can: rcar_canfd: Simplify data access in rcar_canfd_{ge,pu}t_data()
  can: rcar_canfd: Repurpose f_dcfg base for other registers
  can: rcar_canfd: Rename rcar_canfd_setrnc() to rcar_canfd_set_rnc()
  can: rcar_canfd: Share config code in rcar_canfd_set_bittiming()
  can: rcar_canfd: Return early in rcar_canfd_set_bittiming() when not
    FD
  can: rcar_canfd: Add support for Transceiver Delay Compensation

 drivers/net/can/rcar/rcar_canfd.c | 228 ++++++++++++++++++++----------
 1 file changed, 156 insertions(+), 72 deletions(-)

-- 
2.43.0

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH v2 01/10] can: rcar_canfd: Consistently use ndev for net_device pointers

[Geert Uytterhoeven]

Most net_device pointers are named "ndev", but some are called "dev".
Increase uniformity by always using "ndev".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by.
---
 drivers/net/can/rcar/rcar_canfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 7f10213738e5cee7..2174c9667cabce54 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1436,9 +1436,9 @@ static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void rcar_canfd_set_bittiming(struct net_device *dev)
+static void rcar_canfd_set_bittiming(struct net_device *ndev)
 {
-	struct rcar_canfd_channel *priv = netdev_priv(dev);
+	struct rcar_canfd_channel *priv = netdev_priv(ndev);
 	struct rcar_canfd_global *gpriv = priv->gpriv;
 	const struct can_bittiming *bt = &priv->can.bittiming;
 	const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -1818,10 +1818,10 @@ static int rcar_canfd_do_set_mode(struct net_device *ndev, enum can_mode mode)
 	}
 }
 
-static int rcar_canfd_get_berr_counter(const struct net_device *dev,
+static int rcar_canfd_get_berr_counter(const struct net_device *ndev,
 				       struct can_berr_counter *bec)
 {
-	struct rcar_canfd_channel *priv = netdev_priv(dev);
+	struct rcar_canfd_channel *priv = netdev_priv(ndev);
 	u32 val, ch = priv->channel;
 
 	/* Peripheral clock is already enabled in probe */
-- 
2.43.0

[PATCH v2 02/10] can: rcar_canfd: Remove bittiming debug prints

[Geert Uytterhoeven]

There is no need to have debug code to print the bittiming values, as
the user can get all values through the netlink interface.

Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/net/can/rcar/rcar_canfd.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 2174c9667cabce54..b353168f75f28565 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1458,8 +1458,6 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		       RCANFD_NCFG_NSJW(gpriv, sjw) | RCANFD_NCFG_NTSEG2(gpriv, tseg2));
 
 		rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
-		netdev_dbg(priv->ndev, "nrate: brp %u, sjw %u, tseg1 %u, tseg2 %u\n",
-			   brp, sjw, tseg1, tseg2);
 
 		/* Data bit timing settings */
 		brp = dbt->brp - 1;
@@ -1471,8 +1469,6 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
 
 		rcar_canfd_write(priv->base, RCANFD_F_DCFG(gpriv, ch), cfg);
-		netdev_dbg(priv->ndev, "drate: brp %u, sjw %u, tseg1 %u, tseg2 %u\n",
-			   brp, sjw, tseg1, tseg2);
 	} else {
 		/* Classical CAN only mode */
 		if (gpriv->info->shared_can_regs) {
@@ -1488,9 +1484,6 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		}
 
 		rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
-		netdev_dbg(priv->ndev,
-			   "rate: brp %u, sjw %u, tseg1 %u, tseg2 %u\n",
-			   brp, sjw, tseg1, tseg2);
 	}
 }
 
-- 
2.43.0

[PATCH v2 03/10] can: rcar_canfd: Add helper variable ndev to rcar_canfd_rx_pkt()

[Geert Uytterhoeven]

rcar_canfd_rx_pkt() has many users of "priv->ndev".  Introduce a
shorthand to simplify the code.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by.
---
 drivers/net/can/rcar/rcar_canfd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index b353168f75f28565..ddf3b91d3d2bba97 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1684,7 +1684,8 @@ static netdev_tx_t rcar_canfd_start_xmit(struct sk_buff *skb,
 
 static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
 {
-	struct net_device_stats *stats = &priv->ndev->stats;
+	struct net_device *ndev = priv->ndev;
+	struct net_device_stats *stats = &ndev->stats;
 	struct rcar_canfd_global *gpriv = priv->gpriv;
 	struct canfd_frame *cf;
 	struct sk_buff *skb;
@@ -1700,14 +1701,13 @@ static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
 
 		if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) &&
 		    sts & RCANFD_RFFDSTS_RFFDF)
-			skb = alloc_canfd_skb(priv->ndev, &cf);
+			skb = alloc_canfd_skb(ndev, &cf);
 		else
-			skb = alloc_can_skb(priv->ndev,
-					    (struct can_frame **)&cf);
+			skb = alloc_can_skb(ndev, (struct can_frame **)&cf);
 	} else {
 		id = rcar_canfd_read(priv->base, RCANFD_C_RFID(ridx));
 		dlc = rcar_canfd_read(priv->base, RCANFD_C_RFPTR(ridx));
-		skb = alloc_can_skb(priv->ndev, (struct can_frame **)&cf);
+		skb = alloc_can_skb(ndev, (struct can_frame **)&cf);
 	}
 
 	if (!skb) {
@@ -1728,7 +1728,7 @@ static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
 
 		if (sts & RCANFD_RFFDSTS_RFESI) {
 			cf->flags |= CANFD_ESI;
-			netdev_dbg(priv->ndev, "ESI Error\n");
+			netdev_dbg(ndev, "ESI Error\n");
 		}
 
 		if (!(sts & RCANFD_RFFDSTS_RFFDF) && (id & RCANFD_RFID_RFRTR)) {
-- 
2.43.0

[PATCH v2 04/10] can: rcar_canfd: Add helper variable dev to rcar_canfd_reset_controller()

[Geert Uytterhoeven]

rcar_canfd_reset_controller() has many users of "pdev->dev".  Introduce
a shorthand to simplify the code.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by.
---
 drivers/net/can/rcar/rcar_canfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index ddf3b91d3d2bba97..3244584a6ee5d67b 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -841,6 +841,7 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
 
 static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 {
+	struct device *dev = &gpriv->pdev->dev;
 	u32 sts, ch;
 	int err;
 
@@ -850,7 +851,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 	err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
 				 !(sts & RCANFD_GSTS_GRAMINIT), 2, 500000);
 	if (err) {
-		dev_dbg(&gpriv->pdev->dev, "global raminit failed\n");
+		dev_dbg(dev, "global raminit failed\n");
 		return err;
 	}
 
@@ -863,7 +864,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 	err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
 				 (sts & RCANFD_GSTS_GRSTSTS), 2, 500000);
 	if (err) {
-		dev_dbg(&gpriv->pdev->dev, "global reset failed\n");
+		dev_dbg(dev, "global reset failed\n");
 		return err;
 	}
 
@@ -887,8 +888,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 					 (sts & RCANFD_CSTS_CRSTSTS),
 					 2, 500000);
 		if (err) {
-			dev_dbg(&gpriv->pdev->dev,
-				"channel %u reset failed\n", ch);
+			dev_dbg(dev, "channel %u reset failed\n", ch);
 			return err;
 		}
 	}
-- 
2.43.0

[PATCH v2 05/10] can: rcar_canfd: Simplify data access in rcar_canfd_{ge,pu}t_data()

[Geert Uytterhoeven]

Replace the repeated casts, pointer additions, and pointer dereferences
by array accesses to improve readability.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by.
---
 drivers/net/can/rcar/rcar_canfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 3244584a6ee5d67b..dded509793bb93ec 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -781,23 +781,23 @@ static void rcar_canfd_update_bit(void __iomem *base, u32 reg,
 static void rcar_canfd_get_data(struct rcar_canfd_channel *priv,
 				struct canfd_frame *cf, u32 off)
 {
+	u32 *data = (u32 *)cf->data;
 	u32 i, lwords;
 
 	lwords = DIV_ROUND_UP(cf->len, sizeof(u32));
 	for (i = 0; i < lwords; i++)
-		*((u32 *)cf->data + i) =
-			rcar_canfd_read(priv->base, off + i * sizeof(u32));
+		data[i] = rcar_canfd_read(priv->base, off + i * sizeof(u32));
 }
 
 static void rcar_canfd_put_data(struct rcar_canfd_channel *priv,
 				struct canfd_frame *cf, u32 off)
 {
+	const u32 *data = (u32 *)cf->data;
 	u32 i, lwords;
 
 	lwords = DIV_ROUND_UP(cf->len, sizeof(u32));
 	for (i = 0; i < lwords; i++)
-		rcar_canfd_write(priv->base, off + i * sizeof(u32),
-				 *((u32 *)cf->data + i));
+		rcar_canfd_write(priv->base, off + i * sizeof(u32), data[i]);
 }
 
 static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
-- 
2.43.0

[PATCH v2 06/10] can: rcar_canfd: Repurpose f_dcfg base for other registers

[Geert Uytterhoeven]

Reuse the existing Channel Data Bitrate Configuration Register offset
member in the register configuration as the base offset for all related
channel-specific registers.
Rename the member and update the (incorrect) comment to reflect this.
Replace the function-like channel-specific register offset macros by
inline functions.

This fixes the offsets of all other (currently unused) channel-specific
registers on R-Car Gen4 and RZ/G3E, and allows us to replace
RCANFD_GEN4_FDCFG() by the more generic rcar_canfd_f_cfdcfg().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by.
  - Replace function-like macros by inline functions,
  - s/addresses/offsets/.
---
 drivers/net/can/rcar/rcar_canfd.c | 52 ++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index dded509793bb93ec..8baf8a928da757f2 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -425,19 +425,10 @@
 #define RCANFD_C_RPGACC(r)		(0x1900 + (0x04 * (r)))
 
 /* R-Car Gen4 Classical and CAN FD mode specific register map */
-#define RCANFD_GEN4_FDCFG(m)		(0x1404 + (0x20 * (m)))
-
 #define RCANFD_GEN4_GAFL_OFFSET		(0x1800)
 
 /* CAN FD mode specific register map */
 
-/* RSCFDnCFDCmXXX -> RCANFD_F_XXX(m) */
-#define RCANFD_F_DCFG(gpriv, m)		((gpriv)->info->regs->f_dcfg + (0x20 * (m)))
-#define RCANFD_F_CFDCFG(m)		(0x0504 + (0x20 * (m)))
-#define RCANFD_F_CFDCTR(m)		(0x0508 + (0x20 * (m)))
-#define RCANFD_F_CFDSTS(m)		(0x050c + (0x20 * (m)))
-#define RCANFD_F_CFDCRC(m)		(0x0510 + (0x20 * (m)))
-
 /* RSCFDnCFDGAFLXXXj offset */
 #define RCANFD_F_GAFL_OFFSET		(0x1000)
 
@@ -510,7 +501,7 @@ struct rcar_canfd_regs {
 	u16 cfcc;	/* Common FIFO Configuration/Control Register */
 	u16 cfsts;	/* Common FIFO Status Register */
 	u16 cfpctr;	/* Common FIFO Pointer Control Register */
-	u16 f_dcfg;	/* Global FD Configuration Register */
+	u16 coffset;	/* Channel Data Bitrate Configuration Register */
 	u16 rfoffset;	/* Receive FIFO buffer access ID register */
 	u16 cfoffset;	/* Transmit/receive FIFO buffer access ID register */
 };
@@ -641,7 +632,7 @@ static const struct rcar_canfd_regs rcar_gen3_regs = {
 	.cfcc = 0x0118,
 	.cfsts = 0x0178,
 	.cfpctr = 0x01d8,
-	.f_dcfg = 0x0500,
+	.coffset = 0x0500,
 	.rfoffset = 0x3000,
 	.cfoffset = 0x3400,
 };
@@ -651,7 +642,7 @@ static const struct rcar_canfd_regs rcar_gen4_regs = {
 	.cfcc = 0x0120,
 	.cfsts = 0x01e0,
 	.cfpctr = 0x0240,
-	.f_dcfg = 0x1400,
+	.coffset = 0x1400,
 	.rfoffset = 0x6000,
 	.cfoffset = 0x6400,
 };
@@ -800,6 +791,37 @@ static void rcar_canfd_put_data(struct rcar_canfd_channel *priv,
 		rcar_canfd_write(priv->base, off + i * sizeof(u32), data[i]);
 }
 
+/* RSCFDnCFDCmXXX -> rcar_canfd_f_xxx(gpriv, ch) */
+static inline unsigned int rcar_canfd_f_dcfg(struct rcar_canfd_global *gpriv,
+					     unsigned int ch)
+{
+	return gpriv->info->regs->coffset + 0x00 + 0x20 * ch;
+}
+
+static inline unsigned int rcar_canfd_f_cfdcfg(struct rcar_canfd_global *gpriv,
+					       unsigned int ch)
+{
+	return gpriv->info->regs->coffset + 0x04 + 0x20 * ch;
+}
+
+static inline unsigned int rcar_canfd_f_cfdctr(struct rcar_canfd_global *gpriv,
+					       unsigned int ch)
+{
+	return gpriv->info->regs->coffset + 0x08 + 0x20 * ch;
+}
+
+static inline unsigned int rcar_canfd_f_cfdsts(struct rcar_canfd_global *gpriv,
+					       unsigned int ch)
+{
+	return gpriv->info->regs->coffset + 0x0c + 0x20 * ch;
+}
+
+static inline unsigned int rcar_canfd_f_cfdcrc(struct rcar_canfd_global *gpriv,
+					       unsigned int ch)
+{
+	return gpriv->info->regs->coffset + 0x10 + 0x20 * ch;
+}
+
 static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
 {
 	u32 i;
@@ -827,8 +849,8 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
 
 		for_each_set_bit(ch, &gpriv->channels_mask,
 				 gpriv->info->max_channels)
-			rcar_canfd_set_bit(gpriv->base, RCANFD_GEN4_FDCFG(ch),
-					   val);
+			rcar_canfd_set_bit(gpriv->base,
+					   rcar_canfd_f_cfdcfg(gpriv, ch), val);
 	} else {
 		if (gpriv->fdmode)
 			rcar_canfd_set_bit(gpriv->base, RCANFD_GRMCFG,
@@ -1468,7 +1490,7 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		cfg = (RCANFD_DCFG_DTSEG1(gpriv, tseg1) | RCANFD_DCFG_DBRP(brp) |
 		       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
 
-		rcar_canfd_write(priv->base, RCANFD_F_DCFG(gpriv, ch), cfg);
+		rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
 	} else {
 		/* Classical CAN only mode */
 		if (gpriv->info->shared_can_regs) {
-- 
2.43.0

[PATCH v2 07/10] can: rcar_canfd: Rename rcar_canfd_setrnc() to rcar_canfd_set_rnc()

[Geert Uytterhoeven]

Insert an underscore in the function's name, for consistency with other
getter and setter helper functions.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/net/can/rcar/rcar_canfd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 8baf8a928da757f2..c292694ae4d27ac4 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -830,8 +830,8 @@ static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
 		can_free_echo_skb(ndev, i, NULL);
 }
 
-static void rcar_canfd_setrnc(struct rcar_canfd_global *gpriv, unsigned int ch,
-			      unsigned int num_rules)
+static void rcar_canfd_set_rnc(struct rcar_canfd_global *gpriv, unsigned int ch,
+			       unsigned int num_rules)
 {
 	unsigned int rnc_stride = 32 / gpriv->info->rnc_field_width;
 	unsigned int shift = 32 - (ch % rnc_stride + 1) * gpriv->info->rnc_field_width;
@@ -960,7 +960,7 @@ static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv,
 			    RCANFD_GAFLECTR_AFLDAE));
 
 	/* Write number of rules for channel */
-	rcar_canfd_setrnc(gpriv, ch, num_rules);
+	rcar_canfd_set_rnc(gpriv, ch, num_rules);
 	if (gpriv->info->shared_can_regs)
 		offset = RCANFD_GEN4_GAFL_OFFSET;
 	else if (gpriv->fdmode)
-- 
2.43.0

[PATCH v2 08/10] can: rcar_canfd: Share config code in rcar_canfd_set_bittiming()

[Geert Uytterhoeven]

The configuration register format for nominal bit timings in CAN-FD mode
and the format for bit timings in CAN mode on CAN-FD controllers with
shared Classical CAN registers are the same.

Restructure the code to make this clear, also reducing kernel size by 80
bytes.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by,
  - Rebase.
---
 drivers/net/can/rcar/rcar_canfd.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index c292694ae4d27ac4..9ee49ef57e4f9c8e 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1474,13 +1474,17 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 	tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
 	tseg2 = bt->phase_seg2 - 1;
 
-	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
-		/* CAN FD only mode */
+	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) || gpriv->info->shared_can_regs) {
 		cfg = (RCANFD_NCFG_NTSEG1(gpriv, tseg1) | RCANFD_NCFG_NBRP(brp) |
 		       RCANFD_NCFG_NSJW(gpriv, sjw) | RCANFD_NCFG_NTSEG2(gpriv, tseg2));
+	} else {
+		cfg = (RCANFD_CFG_TSEG1(tseg1) | RCANFD_CFG_BRP(brp) |
+		       RCANFD_CFG_SJW(sjw) | RCANFD_CFG_TSEG2(tseg2));
+	}
 
-		rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
+	rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
 
+	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
 		/* Data bit timing settings */
 		brp = dbt->brp - 1;
 		sjw = dbt->sjw - 1;
@@ -1491,21 +1495,6 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
 
 		rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
-	} else {
-		/* Classical CAN only mode */
-		if (gpriv->info->shared_can_regs) {
-			cfg = (RCANFD_NCFG_NTSEG1(gpriv, tseg1) |
-			       RCANFD_NCFG_NBRP(brp) |
-			       RCANFD_NCFG_NSJW(gpriv, sjw) |
-			       RCANFD_NCFG_NTSEG2(gpriv, tseg2));
-		} else {
-			cfg = (RCANFD_CFG_TSEG1(tseg1) |
-			       RCANFD_CFG_BRP(brp) |
-			       RCANFD_CFG_SJW(sjw) |
-			       RCANFD_CFG_TSEG2(tseg2));
-		}
-
-		rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
 	}
 }
 
-- 
2.43.0

[PATCH v2 09/10] can: rcar_canfd: Return early in rcar_canfd_set_bittiming() when not FD

[Geert Uytterhoeven]

Return early after completing all setup for non-FD mode in
rcar_canfd_set_bittiming(), to prepare for the advent of more FD-only
setup.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by,
  - Rebase.
---
 drivers/net/can/rcar/rcar_canfd.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 9ee49ef57e4f9c8e..3340ae75bbecdb5b 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1484,18 +1484,19 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 
 	rcar_canfd_write(priv->base, RCANFD_CCFG(ch), cfg);
 
-	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
-		/* Data bit timing settings */
-		brp = dbt->brp - 1;
-		sjw = dbt->sjw - 1;
-		tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
-		tseg2 = dbt->phase_seg2 - 1;
+	if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD))
+		return;
 
-		cfg = (RCANFD_DCFG_DTSEG1(gpriv, tseg1) | RCANFD_DCFG_DBRP(brp) |
-		       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
+	/* Data bit timing settings */
+	brp = dbt->brp - 1;
+	sjw = dbt->sjw - 1;
+	tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
+	tseg2 = dbt->phase_seg2 - 1;
 
-		rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
-	}
+	cfg = (RCANFD_DCFG_DTSEG1(gpriv, tseg1) | RCANFD_DCFG_DBRP(brp) |
+	       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
+
+	rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
 }
 
 static int rcar_canfd_start(struct net_device *ndev)
-- 
2.43.0

[PATCH v2 10/10] can: rcar_canfd: Add support for Transceiver Delay Compensation

[Geert Uytterhoeven]

The Renesas CAN-FD hardware block supports configuring Transceiver Delay
Compensation, and reading back the Transceiver Delay Compensation
Result, which is needed to support high transfer rates like 8 Mbps.
The Secondary Sample Point is either the measured delay plus the
configured offset, or just the configured offset.

Fix the existing RCANFD_FDCFG_TDCO() macro for the intended use case
(writing instead of reading the field).  Add register definition bits
for the Channel n CAN-FD Status Register.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
v2:
  - Add Reviewed-by,
  - Rebase,
  - Replace function-like macro RCANFD_FDSTS_TDCR() by bitmask
    RCANFD_FDSTS_TDCR and helper function rcar_canfd_get_tdcr(),
  - Replace function-like macro RCANFD_FDSTS_TDCVF() by two bit
    definitions,
  - Drop debug print of tdc mode and tdco value.
---
 drivers/net/can/rcar/rcar_canfd.c | 85 +++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 3340ae75bbecdb5b..1e559c0ff0389a5b 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -191,9 +191,19 @@
 /* RSCFDnCFDCmFDCFG */
 #define RCANFD_GEN4_FDCFG_CLOE		BIT(30)
 #define RCANFD_GEN4_FDCFG_FDOE		BIT(28)
+#define RCANFD_FDCFG_TDCO		GENMASK(23, 16)
 #define RCANFD_FDCFG_TDCE		BIT(9)
 #define RCANFD_FDCFG_TDCOC		BIT(8)
-#define RCANFD_FDCFG_TDCO(x)		(((x) & 0x7f) >> 16)
+
+/* RSCFDnCFDCmFDSTS */
+#define RCANFD_FDSTS_SOC		GENMASK(31, 24)
+#define RCANFD_FDSTS_EOC		GENMASK(23, 16)
+#define RCANFD_GEN4_FDSTS_TDCVF		BIT(15)
+#define RCANFD_GEN4_FDSTS_PNSTS		GENMASK(13, 12)
+#define RCANFD_FDSTS_SOCO		BIT(9)
+#define RCANFD_FDSTS_EOCO		BIT(8)
+#define RCANFD_FDSTS_TDCVF		BIT(7)
+#define RCANFD_FDSTS_TDCR		GENMASK(7, 0)
 
 /* RSCFDnCFDRFCCx */
 #define RCANFD_RFCC_RFIM		BIT(12)
@@ -520,6 +530,7 @@ struct rcar_canfd_shift_data {
 struct rcar_canfd_hw_info {
 	const struct can_bittiming_const *nom_bittiming;
 	const struct can_bittiming_const *data_bittiming;
+	const struct can_tdc_const *tdc_const;
 	const struct rcar_canfd_regs *regs;
 	const struct rcar_canfd_shift_data *sh;
 	u8 rnc_field_width;
@@ -627,6 +638,25 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 	.brp_inc = 1,
 };
 
+/* CAN FD Transmission Delay Compensation constants */
+static const struct can_tdc_const rcar_canfd_gen3_tdc_const = {
+	.tdcv_min = 1,
+	.tdcv_max = 128,
+	.tdco_min = 1,
+	.tdco_max = 128,
+	.tdcf_min = 0,	/* Filter window not supported */
+	.tdcf_max = 0,
+};
+
+static const struct can_tdc_const rcar_canfd_gen4_tdc_const = {
+	.tdcv_min = 1,
+	.tdcv_max = 256,
+	.tdco_min = 1,
+	.tdco_max = 256,
+	.tdcf_min = 0,	/* Filter window not supported */
+	.tdcf_max = 0,
+};
+
 static const struct rcar_canfd_regs rcar_gen3_regs = {
 	.rfcc = 0x00b8,
 	.cfcc = 0x0118,
@@ -672,6 +702,7 @@ static const struct rcar_canfd_shift_data rcar_gen4_shift_data = {
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.nom_bittiming = &rcar_canfd_gen3_nom_bittiming_const,
 	.data_bittiming = &rcar_canfd_gen3_data_bittiming_const,
+	.tdc_const = &rcar_canfd_gen3_tdc_const,
 	.regs = &rcar_gen3_regs,
 	.sh = &rcar_gen3_shift_data,
 	.rnc_field_width = 8,
@@ -688,6 +719,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
 	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
+	.tdc_const = &rcar_canfd_gen4_tdc_const,
 	.regs = &rcar_gen4_regs,
 	.sh = &rcar_gen4_shift_data,
 	.rnc_field_width = 16,
@@ -704,6 +736,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.nom_bittiming = &rcar_canfd_gen3_nom_bittiming_const,
 	.data_bittiming = &rcar_canfd_gen3_data_bittiming_const,
+	.tdc_const = &rcar_canfd_gen3_tdc_const,
 	.regs = &rcar_gen3_regs,
 	.sh = &rcar_gen3_shift_data,
 	.rnc_field_width = 8,
@@ -720,6 +753,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
 	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
 	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
+	.tdc_const = &rcar_canfd_gen4_tdc_const,
 	.regs = &rcar_gen4_regs,
 	.sh = &rcar_gen4_shift_data,
 	.rnc_field_width = 16,
@@ -1460,12 +1494,15 @@ static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
 
 static void rcar_canfd_set_bittiming(struct net_device *ndev)
 {
+	u32 mask = RCANFD_FDCFG_TDCO | RCANFD_FDCFG_TDCE | RCANFD_FDCFG_TDCOC;
 	struct rcar_canfd_channel *priv = netdev_priv(ndev);
 	struct rcar_canfd_global *gpriv = priv->gpriv;
 	const struct can_bittiming *bt = &priv->can.bittiming;
 	const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
+	const struct can_tdc_const *tdc_const = priv->can.fd.tdc_const;
+	const struct can_tdc *tdc = &priv->can.fd.tdc;
+	u32 cfg, tdcmode = 0, tdco = 0;
 	u16 brp, sjw, tseg1, tseg2;
-	u32 cfg;
 	u32 ch = priv->channel;
 
 	/* Nominal bit timing settings */
@@ -1497,6 +1534,20 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 	       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
 
 	rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
+
+	/* Transceiver Delay Compensation */
+	if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_AUTO) {
+		/* TDC enabled, measured + offset */
+		tdcmode = RCANFD_FDCFG_TDCE;
+		tdco = tdc->tdco - 1;
+	} else if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_MANUAL) {
+		/* TDC enabled, offset only */
+		tdcmode = RCANFD_FDCFG_TDCE | RCANFD_FDCFG_TDCOC;
+		tdco = min(tdc->tdcv + tdc->tdco, tdc_const->tdco_max) - 1;
+	}
+
+	rcar_canfd_update_bit(gpriv->base, rcar_canfd_f_cfdcfg(gpriv, ch), mask,
+			      tdcmode | FIELD_PREP(RCANFD_FDCFG_TDCO, tdco));
 }
 
 static int rcar_canfd_start(struct net_device *ndev)
@@ -1807,6 +1858,29 @@ static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota)
 	return num_pkts;
 }
 
+static unsigned int rcar_canfd_get_tdcr(struct rcar_canfd_global *gpriv,
+					unsigned int ch)
+{
+	u32 sts = rcar_canfd_read(gpriv->base, rcar_canfd_f_cfdsts(gpriv, ch));
+	u32 tdcr = FIELD_GET(RCANFD_FDSTS_TDCR, sts);
+
+	return tdcr & (gpriv->info->tdc_const->tdcv_max - 1);
+}
+
+static int rcar_canfd_get_auto_tdcv(const struct net_device *ndev, u32 *tdcv)
+{
+	struct rcar_canfd_channel *priv = netdev_priv(ndev);
+	u32 tdco = priv->can.fd.tdc.tdco;
+	u32 tdcr;
+
+	/* Transceiver Delay Compensation Result */
+	tdcr = rcar_canfd_get_tdcr(priv->gpriv, priv->channel) + 1;
+
+	*tdcv = tdcr < tdco ? 0 : tdcr - tdco;
+
+	return 0;
+}
+
 static int rcar_canfd_do_set_mode(struct net_device *ndev, enum can_mode mode)
 {
 	int err;
@@ -1929,12 +2003,17 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	if (gpriv->fdmode) {
 		priv->can.bittiming_const = gpriv->info->nom_bittiming;
 		priv->can.fd.data_bittiming_const = gpriv->info->data_bittiming;
+		priv->can.fd.tdc_const = gpriv->info->tdc_const;
 
 		/* Controller starts in CAN FD only mode */
 		err = can_set_static_ctrlmode(ndev, CAN_CTRLMODE_FD);
 		if (err)
 			goto fail;
-		priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING;
+
+		priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
+					       CAN_CTRLMODE_TDC_AUTO |
+					       CAN_CTRLMODE_TDC_MANUAL;
+		priv->can.fd.do_get_auto_tdcv = rcar_canfd_get_auto_tdcv;
 	} else {
 		/* Controller starts in Classical CAN only mode */
 		priv->can.bittiming_const = &rcar_canfd_bittiming_const;
-- 
2.43.0

Re: [PATCH v2 06/10] can: rcar_canfd: Repurpose f_dcfg base for other registers

[Vincent Mailhol]

On 12/06/2025 at 00:37, Geert Uytterhoeven wrote:
> Reuse the existing Channel Data Bitrate Configuration Register offset
> member in the register configuration as the base offset for all related
> channel-specific registers.
> Rename the member and update the (incorrect) comment to reflect this.
> Replace the function-like channel-specific register offset macros by
> inline functions.
> 
> This fixes the offsets of all other (currently unused) channel-specific
> registers on R-Car Gen4 and RZ/G3E, and allows us to replace
> RCANFD_GEN4_FDCFG() by the more generic rcar_canfd_f_cfdcfg().
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> v2:
>   - Add Reviewed-by.
>   - Replace function-like macros by inline functions,

Thanks!

>   - s/addresses/offsets/.
> ---
>  drivers/net/can/rcar/rcar_canfd.c | 52 ++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index dded509793bb93ec..8baf8a928da757f2 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -425,19 +425,10 @@
>  #define RCANFD_C_RPGACC(r)		(0x1900 + (0x04 * (r)))
>  
>  /* R-Car Gen4 Classical and CAN FD mode specific register map */
> -#define RCANFD_GEN4_FDCFG(m)		(0x1404 + (0x20 * (m)))
> -
>  #define RCANFD_GEN4_GAFL_OFFSET		(0x1800)
>  
>  /* CAN FD mode specific register map */
>  
> -/* RSCFDnCFDCmXXX -> RCANFD_F_XXX(m) */
> -#define RCANFD_F_DCFG(gpriv, m)		((gpriv)->info->regs->f_dcfg + (0x20 * (m)))
> -#define RCANFD_F_CFDCFG(m)		(0x0504 + (0x20 * (m)))
> -#define RCANFD_F_CFDCTR(m)		(0x0508 + (0x20 * (m)))
> -#define RCANFD_F_CFDSTS(m)		(0x050c + (0x20 * (m)))
> -#define RCANFD_F_CFDCRC(m)		(0x0510 + (0x20 * (m)))
> -
>  /* RSCFDnCFDGAFLXXXj offset */
>  #define RCANFD_F_GAFL_OFFSET		(0x1000)
>  
> @@ -510,7 +501,7 @@ struct rcar_canfd_regs {
>  	u16 cfcc;	/* Common FIFO Configuration/Control Register */
>  	u16 cfsts;	/* Common FIFO Status Register */
>  	u16 cfpctr;	/* Common FIFO Pointer Control Register */
> -	u16 f_dcfg;	/* Global FD Configuration Register */
> +	u16 coffset;	/* Channel Data Bitrate Configuration Register */
>  	u16 rfoffset;	/* Receive FIFO buffer access ID register */
>  	u16 cfoffset;	/* Transmit/receive FIFO buffer access ID register */
>  };
> @@ -641,7 +632,7 @@ static const struct rcar_canfd_regs rcar_gen3_regs = {
>  	.cfcc = 0x0118,
>  	.cfsts = 0x0178,
>  	.cfpctr = 0x01d8,
> -	.f_dcfg = 0x0500,
> +	.coffset = 0x0500,
>  	.rfoffset = 0x3000,
>  	.cfoffset = 0x3400,
>  };
> @@ -651,7 +642,7 @@ static const struct rcar_canfd_regs rcar_gen4_regs = {
>  	.cfcc = 0x0120,
>  	.cfsts = 0x01e0,
>  	.cfpctr = 0x0240,
> -	.f_dcfg = 0x1400,
> +	.coffset = 0x1400,
>  	.rfoffset = 0x6000,
>  	.cfoffset = 0x6400,
>  };
> @@ -800,6 +791,37 @@ static void rcar_canfd_put_data(struct rcar_canfd_channel *priv,
>  		rcar_canfd_write(priv->base, off + i * sizeof(u32), data[i]);
>  }
>  
> +/* RSCFDnCFDCmXXX -> rcar_canfd_f_xxx(gpriv, ch) */
> +static inline unsigned int rcar_canfd_f_dcfg(struct rcar_canfd_global *gpriv,
> +					     unsigned int ch)
> +{
> +	return gpriv->info->regs->coffset + 0x00 + 0x20 * ch;
> +}
> +
> +static inline unsigned int rcar_canfd_f_cfdcfg(struct rcar_canfd_global *gpriv,
> +					       unsigned int ch)
> +{
> +	return gpriv->info->regs->coffset + 0x04 + 0x20 * ch;
> +}
> +
> +static inline unsigned int rcar_canfd_f_cfdctr(struct rcar_canfd_global *gpriv,
> +					       unsigned int ch)
> +{
> +	return gpriv->info->regs->coffset + 0x08 + 0x20 * ch;
> +}
> +
> +static inline unsigned int rcar_canfd_f_cfdsts(struct rcar_canfd_global *gpriv,
> +					       unsigned int ch)
> +{
> +	return gpriv->info->regs->coffset + 0x0c + 0x20 * ch;
> +}
> +
> +static inline unsigned int rcar_canfd_f_cfdcrc(struct rcar_canfd_global *gpriv,
> +					       unsigned int ch)
> +{
> +	return gpriv->info->regs->coffset + 0x10 + 0x20 * ch;
> +}
> +
>  static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
>  {
>  	u32 i;
> @@ -827,8 +849,8 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
>  
>  		for_each_set_bit(ch, &gpriv->channels_mask,
>  				 gpriv->info->max_channels)
> -			rcar_canfd_set_bit(gpriv->base, RCANFD_GEN4_FDCFG(ch),
> -					   val);
> +			rcar_canfd_set_bit(gpriv->base,
> +					   rcar_canfd_f_cfdcfg(gpriv, ch), val);
>  	} else {
>  		if (gpriv->fdmode)
>  			rcar_canfd_set_bit(gpriv->base, RCANFD_GRMCFG,
> @@ -1468,7 +1490,7 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
>  		cfg = (RCANFD_DCFG_DTSEG1(gpriv, tseg1) | RCANFD_DCFG_DBRP(brp) |
>  		       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
>  
> -		rcar_canfd_write(priv->base, RCANFD_F_DCFG(gpriv, ch), cfg);
> +		rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
>  	} else {
>  		/* Classical CAN only mode */
>  		if (gpriv->info->shared_can_regs) {

Thinking of your code, you are still using some magic numbers, e.g.

  0x04 + 0x20 * ch

to access your registers. But at the end those magic numbers are just describing
a memory layout.

I think this can be describe as a C structure. This is what I have in mind:

--------------8<--------------
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 1e559c0ff038..487f40320c20 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -560,10 +560,21 @@ struct rcar_canfd_channel {
 	spinlock_t tx_lock;			/* To protect tx path */
 };
 
+struct rcar_canfd_f {
+	u32 dcfg;
+	u32 cfdcfg;
+	u32 cfdctr;
+	u32 cfdsts;
+	u32 cfdcrc;
+	u32 padding[3];
+};
+static_assert(sizeof(struct rcar_canfd_f) == 0x20);
+
 /* Global priv data */
 struct rcar_canfd_global {
 	struct rcar_canfd_channel *ch[RCANFD_NUM_CHANNELS];
 	void __iomem *base;		/* Register base address */
+	struct rcar_canfd_f __iomem *cbase;
 	struct platform_device *pdev;	/* Respective platform device */
 	struct clk *clkp;		/* Peripheral clock */
 	struct clk *can_clk;		/* fCAN clock */
@@ -803,6 +814,26 @@ static void rcar_canfd_update_bit(void __iomem *base, u32 reg,
 	rcar_canfd_update(mask, val, base + reg);
 }
 
+static inline u32 rcar_canfd_read_reg(void __iomem *addr)
+{
+	return readl(addr);
+}
+
+static inline void rcar_canfd_write_reg(void __iomem *addr, u32 val)
+{
+	writel(val, addr);
+}
+
+static void rcar_canfd_set_bit_reg(void __iomem *addr, u32 val)
+{
+	rcar_canfd_update(val, val, addr);
+}
+
+static void rcar_canfd_update_bit_reg(void __iomem *addr, u32 mask, u32 val)
+{
+	rcar_canfd_update(mask, val, addr);
+}
+
 static void rcar_canfd_get_data(struct rcar_canfd_channel *priv,
 				struct canfd_frame *cf, u32 off)
 {
@@ -825,37 +856,6 @@ static void rcar_canfd_put_data(struct rcar_canfd_channel *priv,
 		rcar_canfd_write(priv->base, off + i * sizeof(u32), data[i]);
 }
 
-/* RSCFDnCFDCmXXX -> rcar_canfd_f_xxx(gpriv, ch) */
-static inline unsigned int rcar_canfd_f_dcfg(struct rcar_canfd_global *gpriv,
-					     unsigned int ch)
-{
-	return gpriv->info->regs->coffset + 0x00 + 0x20 * ch;
-}
-
-static inline unsigned int rcar_canfd_f_cfdcfg(struct rcar_canfd_global *gpriv,
-					       unsigned int ch)
-{
-	return gpriv->info->regs->coffset + 0x04 + 0x20 * ch;
-}
-
-static inline unsigned int rcar_canfd_f_cfdctr(struct rcar_canfd_global *gpriv,
-					       unsigned int ch)
-{
-	return gpriv->info->regs->coffset + 0x08 + 0x20 * ch;
-}
-
-static inline unsigned int rcar_canfd_f_cfdsts(struct rcar_canfd_global *gpriv,
-					       unsigned int ch)
-{
-	return gpriv->info->regs->coffset + 0x0c + 0x20 * ch;
-}
-
-static inline unsigned int rcar_canfd_f_cfdcrc(struct rcar_canfd_global *gpriv,
-					       unsigned int ch)
-{
-	return gpriv->info->regs->coffset + 0x10 + 0x20 * ch;
-}
-
 static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
 {
 	u32 i;
@@ -883,8 +883,7 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
 
 		for_each_set_bit(ch, &gpriv->channels_mask,
 				 gpriv->info->max_channels)
-			rcar_canfd_set_bit(gpriv->base,
-					   rcar_canfd_f_cfdcfg(gpriv, ch), val);
+			rcar_canfd_set_bit_reg(&gpriv->cbase[ch].cfdcfg, val);
 	} else {
 		if (gpriv->fdmode)
 			rcar_canfd_set_bit(gpriv->base, RCANFD_GRMCFG,
@@ -1533,7 +1532,7 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 	cfg = (RCANFD_DCFG_DTSEG1(gpriv, tseg1) | RCANFD_DCFG_DBRP(brp) |
 	       RCANFD_DCFG_DSJW(gpriv, sjw) | RCANFD_DCFG_DTSEG2(gpriv, tseg2));
 
-	rcar_canfd_write(priv->base, rcar_canfd_f_dcfg(gpriv, ch), cfg);
+	rcar_canfd_write_reg(&gpriv->cbase[ch].dcfg, cfg);
 
 	/* Transceiver Delay Compensation */
 	if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_AUTO) {
@@ -1546,8 +1545,8 @@ static void rcar_canfd_set_bittiming(struct net_device *ndev)
 		tdco = min(tdc->tdcv + tdc->tdco, tdc_const->tdco_max) - 1;
 	}
 
-	rcar_canfd_update_bit(gpriv->base, rcar_canfd_f_cfdcfg(gpriv, ch), mask,
-			      tdcmode | FIELD_PREP(RCANFD_FDCFG_TDCO, tdco));
+	rcar_canfd_update_bit_reg(&gpriv->cbase[ch].cfdcfg, mask,
+				  tdcmode | FIELD_PREP(RCANFD_FDCFG_TDCO, tdco));
 }
 
 static int rcar_canfd_start(struct net_device *ndev)
@@ -1861,7 +1860,7 @@ static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota)
 static unsigned int rcar_canfd_get_tdcr(struct rcar_canfd_global *gpriv,
 					unsigned int ch)
 {
-	u32 sts = rcar_canfd_read(gpriv->base, rcar_canfd_f_cfdsts(gpriv, ch));
+	u32 sts = rcar_canfd_read_reg(&gpriv->cbase[ch].cfdsts);
 	u32 tdcr = FIELD_GET(RCANFD_FDSTS_TDCR, sts);
 
 	return tdcr & (gpriv->info->tdc_const->tdcv_max - 1);
@@ -2170,6 +2169,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		goto fail_dev;
 	}
 	gpriv->base = addr;
+	gpriv->cbase = gpriv->base + gpriv->info->regs->coffset;
 
 	/* Request IRQ that's common for both channels */
 	if (info->shared_global_irqs) {
-------------->8--------------

(this applies on top of the last patch of your series)


To be honnest, I am happy to accept your patch as it is now, but what
do you think of the above? I think that this approach works with your
other macro as well.


Yours sincerely,
Vincent Mailhol

Re: [PATCH v2 02/10] can: rcar_canfd: Remove bittiming debug prints

[Vincent Mailhol]

On 12/06/2025 at 00:37, Geert Uytterhoeven wrote:
> There is no need to have debug code to print the bittiming values, as
> the user can get all values through the netlink interface.
> 
> Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Yours sincerely,
Vincent Mailhol

Re: [PATCH v2 07/10] can: rcar_canfd: Rename rcar_canfd_setrnc() to rcar_canfd_set_rnc()

[Vincent Mailhol]

On 12/06/2025 at 00:37, Geert Uytterhoeven wrote:
> Insert an underscore in the function's name, for consistency with other
> getter and setter helper functions.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Yours sincerely,
Vincent Mailhol

Re: [PATCH v2 06/10] can: rcar_canfd: Repurpose f_dcfg base for other registers

[Geert Uytterhoeven]

Hi Vincent,

On Thu, 12 Jun 2025 at 06:00, Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
> On 12/06/2025 at 00:37, Geert Uytterhoeven wrote:
> > Reuse the existing Channel Data Bitrate Configuration Register offset
> > member in the register configuration as the base offset for all related
> > channel-specific registers.
> > Rename the member and update the (incorrect) comment to reflect this.
> > Replace the function-like channel-specific register offset macros by
> > inline functions.
> >
> > This fixes the offsets of all other (currently unused) channel-specific
> > registers on R-Car Gen4 and RZ/G3E, and allows us to replace
> > RCANFD_GEN4_FDCFG() by the more generic rcar_canfd_f_cfdcfg().
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > ---
> > v2:
> >   - Add Reviewed-by.
> >   - Replace function-like macros by inline functions,
>
> Thanks!

> Thinking of your code, you are still using some magic numbers, e.g.
>
>   0x04 + 0x20 * ch
>
> to access your registers. But at the end those magic numbers are just describing
> a memory layout.
>
> I think this can be describe as a C structure. This is what I have in mind:
>
> --------------8<--------------
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 1e559c0ff038..487f40320c20 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -560,10 +560,21 @@ struct rcar_canfd_channel {
>         spinlock_t tx_lock;                     /* To protect tx path */
>  };
>
> +struct rcar_canfd_f {
> +       u32 dcfg;
> +       u32 cfdcfg;
> +       u32 cfdctr;
> +       u32 cfdsts;
> +       u32 cfdcrc;
> +       u32 padding[3];
> +};
> +static_assert(sizeof(struct rcar_canfd_f) == 0x20);

Is that really needed?

> @@ -883,8 +883,7 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
>
>                 for_each_set_bit(ch, &gpriv->channels_mask,
>                                  gpriv->info->max_channels)
> -                       rcar_canfd_set_bit(gpriv->base,
> -                                          rcar_canfd_f_cfdcfg(gpriv, ch), val);
> +                       rcar_canfd_set_bit_reg(&gpriv->cbase[ch].cfdcfg, val);

Nice!

> To be honnest, I am happy to accept your patch as it is now, but what
> do you think of the above? I think that this approach works with your
> other macro as well.

Please take this as-is, so we can move forward.
I will create a proper patch (with your Suggested-by) later,
I have more CAN-FD items on my TODO list...

Thanks!

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 06/10] can: rcar_canfd: Repurpose f_dcfg base for other registers

[Vincent Mailhol]

On 12/06/2025 at 20:33, Geert Uytterhoeven wrote:
> Hi Vincent,
> 
> On Thu, 12 Jun 2025 at 06:00, Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
>> On 12/06/2025 at 00:37, Geert Uytterhoeven wrote:
>>> Reuse the existing Channel Data Bitrate Configuration Register offset
>>> member in the register configuration as the base offset for all related
>>> channel-specific registers.
>>> Rename the member and update the (incorrect) comment to reflect this.
>>> Replace the function-like channel-specific register offset macros by
>>> inline functions.
>>>
>>> This fixes the offsets of all other (currently unused) channel-specific
>>> registers on R-Car Gen4 and RZ/G3E, and allows us to replace
>>> RCANFD_GEN4_FDCFG() by the more generic rcar_canfd_f_cfdcfg().
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>>> ---
>>> v2:
>>>   - Add Reviewed-by.
>>>   - Replace function-like macros by inline functions,
>>
>> Thanks!
> 
>> Thinking of your code, you are still using some magic numbers, e.g.
>>
>>   0x04 + 0x20 * ch
>>
>> to access your registers. But at the end those magic numbers are just describing
>> a memory layout.
>>
>> I think this can be describe as a C structure. This is what I have in mind:
>>
>> --------------8<--------------
>> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
>> index 1e559c0ff038..487f40320c20 100644
>> --- a/drivers/net/can/rcar/rcar_canfd.c
>> +++ b/drivers/net/can/rcar/rcar_canfd.c
>> @@ -560,10 +560,21 @@ struct rcar_canfd_channel {
>>         spinlock_t tx_lock;                     /* To protect tx path */
>>  };
>>
>> +struct rcar_canfd_f {
>> +       u32 dcfg;
>> +       u32 cfdcfg;
>> +       u32 cfdctr;
>> +       u32 cfdsts;
>> +       u32 cfdcrc;
>> +       u32 padding[3];
>> +};
>> +static_assert(sizeof(struct rcar_canfd_f) == 0x20);
> 
> Is that really needed?

It is needed to counterbalance my lack of confidence on whether I am able to
count up to 8 :)

It is just for debug, you can remove. Similarly, feel free to adjust the names.
I just wanted to convey my idea, and a piece of code was easier than a long
paragraph for that.

>> @@ -883,8 +883,7 @@ static void rcar_canfd_set_mode(struct rcar_canfd_global *gpriv)
>>
>>                 for_each_set_bit(ch, &gpriv->channels_mask,
>>                                  gpriv->info->max_channels)
>> -                       rcar_canfd_set_bit(gpriv->base,
>> -                                          rcar_canfd_f_cfdcfg(gpriv, ch), val);
>> +                       rcar_canfd_set_bit_reg(&gpriv->cbase[ch].cfdcfg, val);

I was wondering if there was some kind of helper function to do that? If not,
maybe adding a

  io_set_bitl(void __iomem *addr, val);

and so on into linux/io.h could be a good idea?

> Nice!
> 
>> To be honnest, I am happy to accept your patch as it is now, but what
>> do you think of the above? I think that this approach works with your
>> other macro as well.
> 
> Please take this as-is, so we can move forward.

@Marc, review is done, the series is ready for pick-up.

> I will create a proper patch (with your Suggested-by) later,
> I have more CAN-FD items on my TODO list...

No problem, take your time! My only wish is to not see more complexity added the
the function-like macros. Any other refactor is just a bonus!


Yours sincerely,
Vincent Mailhol

Re: [PATCH v2 00/10] can: rcar_canfd: Add support for Transceiver Delay Compensation

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

On 11.06.2025 17:37:29, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series adds CAN-FD Transceiver Delay Compensation support to
> the R-Car CAN-FD driver, after the customary cleanups and refactorings.
> 

There's no reference to footnote [1], as it refers to the v1, I'll add
it here.

> Changes compared to v1:

Changes compared to v1 [1]:

>   - Dropped patch "can: rcar_canfd: Use ndev parameter in
>     rcar_canfd_set_bittiming()",
>   - New patch "[PATCH v2 02/10] can: rcar_canfd: Remove bittiming debug
>     prints",
>   - New patch "[PATCH v2 07/10] can: rcar_canfd: Rename
>     rcar_canfd_setrnc() to rcar_canfd_set_rnc()",
>   - Add Reviewed-by,
>   - Replace function-like RCANFD_F_*() macros by rcar_canfd_f_*()
>     inline functions,
>   - Replace function-like macro RCANFD_FDSTS_TDCR() by bitmask
>     RCANFD_FDSTS_TDCR and helper function rcar_canfd_get_tdcr(),
>   - Replace function-like macro RCANFD_FDSTS_TDCVF() by two bit
>     definitions,
>   - Drop debug print of tdc mode and tdco value.
> 
> This has been tested on R-Car V4H (White Hawk), V4M (Gray Hawk Single),
> and E3 (Ebisu-4D[2]), using various data bit rates.  Without proper TDC
> configuration, transmitting at 8 Mbps makes the CAN-FD controller enter
> BUS-OFF state.  The TDCV value as measured by the CAN-FD controller is 4
> on all boards tested (base clock 40 MHz, i.e. 25 ns period), and ca. 90
> ns as measured by a logic analyzer on Gray Hawk Single.
> 
> Note that the BSP (predating upstream TDC support), uses a much simpler
> method: for transfer rates >= 5 Mbps on R-Car Gen4, it enables TDC with
> a hardcoded (hardware) TDCO value of 2 (i.e. actual 3), which matches
> the behavior of this series at 8 Mbps.
> 
> Thanks for your comments!
> 
> [1] "[PATCH 0/9] can: rcar_canfd: Add support for Transceiver Delay Compensation"
>     https://lore.kernel.org/cover.1748863848.git.geert+renesas@glider.be

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 00/10] can: rcar_canfd: Add support for Transceiver Delay Compensation

[Geert Uytterhoeven]

Hi Marc,

On Fri, 13 Jun 2025 at 13:42, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 11.06.2025 17:37:29, Geert Uytterhoeven wrote:
> > This patch series adds CAN-FD Transceiver Delay Compensation support to
> > the R-Car CAN-FD driver, after the customary cleanups and refactorings.
>
> There's no reference to footnote [1], as it refers to the v1, I'll add
> it here.

Sorry, my mistake.

> > Changes compared to v1:
>
> Changes compared to v1 [1]:

That was my intention, thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds