Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: 18 March 2025 11:50
> Subject: Re: RE: [PATCH v5 12/16] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info
>
> On 18.03.2025 11:26:54, Biju Das wrote:
> > 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 */
>
> Another option is to evaluate, if you can use priv->can.bittiming_const and priv-
> >can.data_bittiming_const instead.
>
Looks that is possible, as we just need to pick the max vals from below gpv->info->*_bittiming
variables.
struct rcar_canfd_hw_info {
+ const struct can_bittiming_const *nom_bittiming;
+ const struct can_bittiming_const *data_bittiming;
static const struct can_bittiming_const rcar_canfd_nom_bittiming_const = {
+static const struct can_bittiming_const rcar_canfd_gen3_nom_bittiming_const = {
static const struct can_bittiming_const rcar_canfd_gen4_nom_bittiming_const = {
+ .name = RCANFD_DRV_NAME,
+ .tseg1_min = 2,
+ .tseg1_max = 256,
+ .tseg2_min = 2,
+ .tseg2_max = 128,
+ .sjw_max = 128,
+ .brp_min = 1,
+ .brp_max = 1024,
+ .brp_inc = 1,
+};
+
static const struct can_bittiming_const rcar_canfd_data_bittiming_const = {
+static const struct can_bittiming_const rcar_canfd_gen3_data_bittiming_const = {
+static const struct can_bittiming_const rcar_canfd_gen4_data_bittiming_const = {
+ .name = RCANFD_DRV_NAME,
+ .tseg1_min = 2,
+ .tseg1_max = 32,
+ .tseg2_min = 2,
+ .tseg2_max = 16,
+ .sjw_max = 16,
+ .brp_min = 1,
+ .brp_max = 256,
+ .brp_inc = 1,
+};
+
+ priv->can.bittiming_const = gpriv->info->nom_bittiming;
+ priv->can.data_bittiming_const = gpriv->info->data_bittiming;
Cheers,
Biju
