[PATCH] can: rcar_canfd: Describe channel-specific FD registers using C struct

[PATCH] can: rcar_canfd: Describe channel-specific FD registers using C struct

[Geert Uytterhoeven]

The rcar_canfd_f_*() inline functions to obtain channel-specific CAN-FD
register offsets really describe a memory layout.  Hence replace them by
a C structure, to simplify the code, and reduce kernel size.

This also gets rid of warnings about unused rcar_canfd_f_*() inline
functions, which are reported by recent versions of clang.

Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reported-by: Jakub Kicinski <kuba@kernel.org>
Closes: https://lore.kernel.org/20250618183827.5bebca8f@kernel.org
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Fast-tracked because of the clang warnings.

Changes compared to Vincent's original suggestion
(https://lore.kernel.org/420d37b1-5648-4209-8d6f-1ac9d780eea2@wanadoo.fr):
  - Move rcar_canfd_f to the old RCANFD_F_*() location,
  - Update RSCFDnCFDCmXXX comment.
  - Rename struct rcar_canfd_f to struct rcar_canfd_f,
  - Rename cbase to fcbase,
  - Drop static_assert(),
  - Drop unused car_canfd_*_reg() functions.
  - Drop simple wrappers around {read,write}l(),
---
 drivers/net/can/rcar/rcar_canfd.c | 64 +++++++++++++------------------
 1 file changed, 27 insertions(+), 37 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 3dcc977affe89df1..8a9f288187e0e8b6 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -439,6 +439,16 @@
 
 /* CAN FD mode specific register map */
 
+/* RSCFDnCFDCmXXX -> gpriv->fcbase[m].xxx */
+struct rcar_canfd_f_c {
+	u32 dcfg;
+	u32 cfdcfg;
+	u32 cfdctr;
+	u32 cfdsts;
+	u32 cfdcrc;
+	u32 pad[3];
+};
+
 /* RSCFDnCFDGAFLXXXj offset */
 #define RCANFD_F_GAFL_OFFSET		(0x1000)
 
@@ -564,6 +574,7 @@ struct rcar_canfd_channel {
 struct rcar_canfd_global {
 	struct rcar_canfd_channel *ch[RCANFD_NUM_CHANNELS];
 	void __iomem *base;		/* Register base address */
+	struct rcar_canfd_f_c __iomem *fcbase;
 	struct platform_device *pdev;	/* Respective platform device */
 	struct clk *clkp;		/* Peripheral clock */
 	struct clk *can_clk;		/* fCAN clock */
@@ -803,6 +814,16 @@ static void rcar_canfd_update_bit(void __iomem *base, u32 reg,
 	rcar_canfd_update(mask, val, base + reg);
 }
 
+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 +846,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 +873,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->fcbase[ch].cfdcfg, val);
 	} else {
 		if (gpriv->fdmode)
 			rcar_canfd_set_bit(gpriv->base, RCANFD_GRMCFG,
@@ -1533,7 +1522,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);
+	writel(cfg, &gpriv->fcbase[ch].dcfg);
 
 	/* Transceiver Delay Compensation */
 	if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_AUTO) {
@@ -1546,8 +1535,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->fcbase[ch].cfdcfg, mask,
+				  tdcmode | FIELD_PREP(RCANFD_FDCFG_TDCO, tdco));
 }
 
 static int rcar_canfd_start(struct net_device *ndev)
@@ -1861,7 +1850,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 = readl(&gpriv->fcbase[ch].cfdsts);
 	u32 tdcr = FIELD_GET(RCANFD_FDSTS_TDCR, sts);
 
 	return tdcr & (gpriv->info->tdc_const->tdcv_max - 1);
@@ -2170,6 +2159,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		goto fail_dev;
 	}
 	gpriv->base = addr;
+	gpriv->fcbase = addr + gpriv->info->regs->coffset;
 
 	/* Request IRQ that's common for both channels */
 	if (info->shared_global_irqs) {
-- 
2.43.0

Re: [PATCH] can: rcar_canfd: Describe channel-specific FD registers using C struct

[Vincent Mailhol]

Hi Geert,

Thanks for the patch.

On 19/06/2025 at 19:13, Geert Uytterhoeven wrote:
> The rcar_canfd_f_*() inline functions to obtain channel-specific CAN-FD
> register offsets really describe a memory layout.  Hence replace them by
> a C structure, to simplify the code, and reduce kernel size.
> 
> This also gets rid of warnings about unused rcar_canfd_f_*() inline
> functions, which are reported by recent versions of clang.
> 
> Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Closes: https://lore.kernel.org/20250618183827.5bebca8f@kernel.org
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Fast-tracked because of the clang warnings.

Make sense.

@Jakub, OK for me if you want to directly pick this patch without going through
the linux-can tree.

> Changes compared to Vincent's original suggestion
> (https://lore.kernel.org/420d37b1-5648-4209-8d6f-1ac9d780eea2@wanadoo.fr):
>   - Move rcar_canfd_f to the old RCANFD_F_*() location,
>   - Update RSCFDnCFDCmXXX comment.
>   - Rename struct rcar_canfd_f to struct rcar_canfd_f,
                                           ^^^^^^^^^^^^
rcar_canfd_f_c ;)

>   - Rename cbase to fcbase,
>   - Drop static_assert(),
>   - Drop unused car_canfd_*_reg() functions.
>   - Drop simple wrappers around {read,write}l(),

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


Yours sincerely,
Vincent Mailhol

Re: [PATCH] can: rcar_canfd: Describe channel-specific FD registers using C struct

[Geert Uytterhoeven]

Hi Vincent,

On Thu, 19 Jun 2025 at 13:12, Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
> On 19/06/2025 at 19:13, Geert Uytterhoeven wrote:
> > The rcar_canfd_f_*() inline functions to obtain channel-specific CAN-FD
> > register offsets really describe a memory layout.  Hence replace them by
> > a C structure, to simplify the code, and reduce kernel size.
> >
> > This also gets rid of warnings about unused rcar_canfd_f_*() inline
> > functions, which are reported by recent versions of clang.
> >
> > Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > Reported-by: Jakub Kicinski <kuba@kernel.org>
> > Closes: https://lore.kernel.org/20250618183827.5bebca8f@kernel.org
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Fast-tracked because of the clang warnings.
>
> Make sense.
>
> @Jakub, OK for me if you want to directly pick this patch without going through
> the linux-can tree.
>
> > Changes compared to Vincent's original suggestion
> > (https://lore.kernel.org/420d37b1-5648-4209-8d6f-1ac9d780eea2@wanadoo.fr):
> >   - Move rcar_canfd_f to the old RCANFD_F_*() location,
> >   - Update RSCFDnCFDCmXXX comment.
> >   - Rename struct rcar_canfd_f to struct rcar_canfd_f,
>                                            ^^^^^^^^^^^^
> rcar_canfd_f_c ;)

Oops, right...

> >   - Rename cbase to fcbase,
> >   - Drop static_assert(),
> >   - Drop unused car_canfd_*_reg() functions.
> >   - Drop simple wrappers around {read,write}l(),
>
> Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

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] can: rcar_canfd: Describe channel-specific FD registers using C struct

[Jakub Kicinski]

On Thu, 19 Jun 2025 20:12:41 +0900 Vincent Mailhol wrote:
> @Jakub, OK for me if you want to directly pick this patch without going through
> the linux-can tree.

Ah, missed this before replying to the other thread.

We'll take it direct then, thank you!

Re: [PATCH] can: rcar_canfd: Describe channel-specific FD registers using C struct

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 19 Jun 2025 12:13:17 +0200 you wrote:
> The rcar_canfd_f_*() inline functions to obtain channel-specific CAN-FD
> register offsets really describe a memory layout.  Hence replace them by
> a C structure, to simplify the code, and reduce kernel size.
> 
> This also gets rid of warnings about unused rcar_canfd_f_*() inline
> functions, which are reported by recent versions of clang.
> 
> [...]

Here is the summary with links:
  - can: rcar_canfd: Describe channel-specific FD registers using C struct
    https://git.kernel.org/netdev/net-next/c/ab2aa5453bb8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html