Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A - Marc Kleine-Budde

From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Pankaj Bansal <pankaj.bansal@nxp.com>,
	wg@grandegger.com, linux-can@vger.kernel.org
Cc: V.Sethi@nxp.com, poonam.aggrwal@nxp.com,
	Bhupesh Sharma <bhupesh.sharma@freescale.com>
Subject: Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
Date: Tue, 14 Nov 2017 13:59:08 +0100	[thread overview]
Message-ID: <828fba89-66dc-a2c5-e095-8554b9dce5d5@pengutronix.de> (raw)
In-Reply-To: <1510660589-16125-2-git-send-email-pankaj.bansal@nxp.com>

[-- Attachment #1.1: Type: text/plain, Size: 2706 bytes --]

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.

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]

> +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/drivers/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   |

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]