RE: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A

[Pankaj Bansal <pankaj.bansal@nxp.com>]

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Tuesday, November 14, 2017 6:29 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; wg@grandegger.com; linux-
> can@vger.kernel.org
> Cc: Varun Sethi <V.Sethi@nxp.com>; Poonam Aggrwal
> <poonam.aggrwal@nxp.com>; Bhupesh Sharma
> <bhupesh.sharma@freescale.com>
> Subject: Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for
> LS1021A
> 
> On 11/14/2017 12:56 PM, Pankaj Bansal wrote:
> > This patch adds platform specific details for NXP SOC LS1021A to the
> > flexcan driver code.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Reviewed-by: Zhengxiong Jin <Jason.Jin@freescale.com>
> > Reviewed-by: Poonam Aggrwal <poonam.aggrwal@nxp.com>
> > ---
> > Changes in v2:
> >   - No change.
> >
> >  drivers/net/can/flexcan.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 100b451..fa2d4c9 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -304,6 +304,11 @@ static const struct flexcan_devtype_data
> fsl_vf610_devtype_data = {
> >  		FLEXCAN_QUIRK_DISABLE_MECR |
> FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,  };
> >
> > +/* LS1021A-Rev2 has functional RX-FIFO mode  */
> 
> How big is the RX-FIFO? If the normal mailboxes can receive RTR messages,
> we definitely want to use FLEXCAN_QUIRK_USE_OFF_TIMESTAMP. As you
> can buffer > 60 CAN frames compared to only 6 in FIFO mode.
> 

The FIFO in LS1021A is 6 frames.
I saw this quirk. Basically we are creating a FIFO in software which is sorted based
on the receive time stamp value. IMO LS1021A should be able to support that.
BUT I have not tested this quirk yet. Right now I won't be able to test this on LS1021A.
If you can accept this platform with FIFO mode now, then later on I can add this quirk after testing.

> Please add your IP core to the "FLEXCAN hardware feature flags" table [1] in
> the driver. Also please test the transition interrupts for changes in the CAN
> error state, see the commits of ZHU Yi
> (ST-FIR/ENG1-Zhu) <Yi.Zhu5@cn.bosch.com> on the driver. [2]
> 

I did not understand this quirk (FLEXCAN_QUIRK_BROKEN_PERR_STATE).
What does this quirk do ? how do I check if my IP supports it or not ?
How do I test this ?

Can you please help me to understand ?

> > +static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
> > +	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_DISABLE_MECR,
> > +};
> > +
> >  static const struct can_bittiming_const flexcan_bittiming_const = {
> >  	.name = DRV_NAME,
> >  	.tseg1_min = 4,
> > @@ -1245,6 +1250,8 @@ static const struct of_device_id
> flexcan_of_match[] = {
> >  	{ .compatible = "fsl,imx28-flexcan", .data =
> &fsl_imx28_devtype_data, },
> >  	{ .compatible = "fsl,p1010-flexcan", .data =
> &fsl_p1010_devtype_data, },
> >  	{ .compatible = "fsl,vf610-flexcan", .data =
> > &fsl_vf610_devtype_data, },
> > +	{ .compatible = "fsl,ls1021ar2-flexcan",
> > +	  .data = &fsl_ls1021a_r2_devtype_data, },
> 
> Please keep in in one line, even if it's more than 80 chars.
> 
> >  	{ /* sentinel */ },
> >  };
> >  MODULE_DEVICE_TABLE(of, flexcan_of_match);
> >
> 
> Marc
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driver
> s/net/can/flexcan.c#n182
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers
> /net/can/flexcan.c
> 
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |