On 02/06/2025 at 20:54, 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.
>
> This fixes the addresses 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 RCANFD_F_CFDCFG().
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> drivers/net/can/rcar/rcar_canfd.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 0cad3c198e58e494..7a9a88fa5fb1a521 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -425,18 +425,16 @@
> #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)))
> +#define RCANFD_F_DCFG(gpriv, m) ((gpriv)->info->regs->coffset + 0x00 + (0x20 * (m)))
> +#define RCANFD_F_CFDCFG(gpriv, m) ((gpriv)->info->regs->coffset + 0x04 + (0x20 * (m)))
> +#define RCANFD_F_CFDCTR(gpriv, m) ((gpriv)->info->regs->coffset + 0x08 + (0x20 * (m)))
> +#define RCANFD_F_CFDSTS(gpriv, m) ((gpriv)->info->regs->coffset + 0x0c + (0x20 * (m)))
> +#define RCANFD_F_CFDCRC(gpriv, m) ((gpriv)->info->regs->coffset + 0x10 + (0x20 * (m)))
I really start to dislike all those function like macros in the rcar_canfd
driver. The only benefits of a function like macro is either to have type
polymorphism or to generate integer constant expression or to work with context
specific info (e.g. __func__ or __LINE__).
Can you just change these five function like macros to static functions?
And from now on, each time there is a need to modify one of the rcar_canfd, I
would like this to become an opportunity to little by little clean up that macro
madness.
> /* RSCFDnCFDGAFLXXXj offset */
> #define RCANFD_F_GAFL_OFFSET (0x1000)
> @@ -510,7 +508,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 +639,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 +649,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,
> };
> @@ -827,8 +825,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,
> + RCANFD_F_CFDCFG(gpriv, ch), val);
> } else {
> if (gpriv->fdmode)
> rcar_canfd_set_bit(gpriv->base, RCANFD_GRMCFG,
Yours sincerely,
Vincent Mailhol
