Hi Geert and Marc,
> -----Original Message-----
> From: Biju Das
> Sent: 17 March 2025 16:24
> Subject: RE: [PATCH v5 12/16] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info
>
> Hi Geert,
>
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Sent: 17 March 2025 15:04
> > Subject: Re: [PATCH v5 12/16] can: rcar_canfd: Add mask table to
> > struct rcar_canfd_hw_info
> >
> > Hi Biju,
> >
> > On Mon, 17 Mar 2025 at 15:46, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > > Sent: 17 March 2025 14:13
> > > > Subject: Re: [PATCH v5 12/16] can: rcar_canfd: Add mask table to
> > > > struct rcar_canfd_hw_info
> > > >
> > > > On Mon, 17 Mar 2025 at 13:37, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > R-Car Gen3 and Gen4 have some differences in the mask bits. Add
> > > > > a mask table to handle these differences.
> > > > >
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > v4->v5:
> > > > > * Improved commit description by replacing has->have.
> > > > > * Collected tag.
> > > > > * Dropped RCANFD_EEF_MASK and RCANFD_RNC_MASK as it is taken
> > > > > care by gpriv->channels_mask and info->num_supported_rules.
> > > >
> > > > Thanks for the update!
> > > >
> > > > All mask values are just the maximum values of various parameters.
> > > > Hence they could be replaced by the latter, like you already did for the RNC mask.
> > >
> > > But this will increase memory size, right? Currently we have
> > > rcar-gen3 and gen4 tables
> > > 2 tables used by 4 hardware info variants.
> > >
> > > If we drop tables and use variable with max values like RNC MASK,
> > > then this will be like 4 tables for 4 hardware info variants, right?
> > >
> > > Please correct me if my understanding is wrong.
> >
> > It depends where you store the parameters: in the (two) tables, or in
> > the (four) hardware info structures...
>
> OK, you mean replace mask_table->max_val_table, rcar_gen{3,4}_mask_table->rcar_gen{3,4}_max_val_table
> and *_MASK->*_MAX_VAL??
The below parameters[1] has mask = max_parameter_val - 1.
I will update the table as below. Please let me know if there is any issue.
struct rcar_canfd_hw_info {
- const u32 *mask_table;
+ const u32 *params;
};
-enum rcar_canfd_mask_id {
- RCANFD_NTSEG2_MASK, /* Nominal Bit Rate Time Segment 2 Control */
- RCANFD_NTSEG1_MASK, /* Nominal Bit Rate Time Segment 1 Control */
- RCANFD_NSJW_MASK, /* Nominal Bit Rate Resynchronization Jump Width Control */
- RCANFD_DSJW_MASK, /* Data Bit Rate Resynchronization Jump Width Control */
- RCANFD_DTSEG2_MASK, /* Data Bit Rate Time Segment 2 Control */
- RCANFD_DTSEG1_MASK, /* Data Bit Rate Time Segment 1 Control */
- RCANFD_CFTML_MASK, /* Common FIFO TX Message Buffer Link */
+enum rcar_canfd_parameter_id {
+ RCANFD_NTSEG2, /* Nominal Bit Rate Time Segment 2 Control */
+ RCANFD_NTSEG1, /* Nominal Bit Rate Time Segment 1 Control */
+ RCANFD_NSJW, /* Nominal Bit Rate Resynchronization Jump Width Control */
+ RCANFD_DSJW, /* Data Bit Rate Resynchronization Jump Width Control */
+ RCANFD_DTSEG2, /* Data Bit Rate Time Segment 2 Control */
+ RCANFD_DTSEG1, /* Data Bit Rate Time Segment 1 Control */
+ RCANFD_CFTML, /* Common FIFO TX Message Buffer Link */
};
-static const u32 rcar_gen3_mask_table[] = {
- [RCANFD_AFLPN_MASK] = 0x1f,
- [RCANFD_NTSEG2_MASK] = 0x1f,
- [RCANFD_NTSEG1_MASK] = 0x7f,
- [RCANFD_NSJW_MASK] = 0x1f,
- [RCANFD_DSJW_MASK] = 0x7,
- [RCANFD_DTSEG2_MASK] = 0x7,
- [RCANFD_DTSEG1_MASK] = 0xf,
- [RCANFD_CFTML_MASK] = 0xf,
+static const u32 rcar_gen3_params[] = {
+ [RCANFD_NTSEG2] = 32,
+ [RCANFD_NTSEG1] = 128,
+ [RCANFD_NSJW] = 32,
+ [RCANFD_DSJW] = 8,
+ [RCANFD_DTSEG2] = 8,
+ [RCANFD_DTSEG1] = 16,
+ [RCANFD_CFTML] = 15,
};
-static const u32 rcar_gen4_mask_table[] = {
- [RCANFD_AFLPN_MASK] = 0x7f,
- [RCANFD_NTSEG2_MASK] = 0x7f,
- [RCANFD_NTSEG1_MASK] = 0xff,
- [RCANFD_NSJW_MASK] = 0x7f,
- [RCANFD_DSJW_MASK] = 0xf,
- [RCANFD_DTSEG2_MASK] = 0xf,
- [RCANFD_DTSEG1_MASK] = 0x1f,
- [RCANFD_CFTML_MASK] = 0x1f,
+static const u32 rcar_gen4_params[] = {
+ [RCANFD_NTSEG2] = 128,
+ [RCANFD_NTSEG1] = 256,
+ [RCANFD_NSJW] = 128,
+ [RCANFD_DSJW] = 16,
+ [RCANFD_DTSEG2] = 16,
+ [RCANFD_DTSEG1] = 32,
+ [RCANFD_CFTML] = 31,
};
[1]
DSJW[3:0] 0h Data Bit Rate Resynchronization Jump Width Control
Fh: 16 Tq
DTSEG2[3:0] 0h Data Bit Rate Time Segment 2 Control
Fh: 16 Tq
DTSEG1[4:0] 0h Data Bit Rate Time Segment 1 Control
1Fh: 32 Tq
NTSEG2[6:0] 0h Nominal Bit Rate Time Segment 2 Control
7Fh: 128 Tq
NTSEG1[7:0] 0h Nominal Bit Rate Time Segment 1 Control
FFh: 256 Tq
NSJW[6:0] 0h Nominal Bit Rate Resynchronization Jump Width Control
7Fh: 128 Tq
For CFTML, I will use the direct value.
CFTML[4:0] 0h Common FIFO TX Message Buffer Link
Transmission scan link position of the corresponding channel
Cheers,
Biju
