lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <53C38FD9.8070403@pengutronix.de>
Date:	Mon, 14 Jul 2014 10:07:53 +0200
From:	Marc Kleine-Budde <mkl@...gutronix.de>
To:	Stefan Agner <stefan@...er.ch>, shawn.guo@...escale.com
CC:	kernel@...gutronix.de, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/14/2014 09:48 AM, Stefan Agner wrote:
> Extend FlexCAN driver to support Vybrid. Vybrids variant of the IP
> has ECC support which is controlled through the memory error
> control register (MECR). There is also an errata which leads to
> false positive error detections (ID e5295). This patch disables
> the memory error detection completely.
> 
> Signed-off-by: Stefan Agner <stefan@...er.ch>

Looks good, small nitpick inline.

> ---
>  drivers/net/can/flexcan.c | 73 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 4c598c9..a0fc53e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -92,6 +92,27 @@
>  #define FLEXCAN_CTRL_ERR_ALL \
>  	(FLEXCAN_CTRL_ERR_BUS | FLEXCAN_CTRL_ERR_STATE)
>  
> +/* FLEXCAN control register 2 (CTRL2) bits */
> +#define FLEXCAN_CRL2_ECRWRE		BIT(29)
> +#define FLEXCAN_CRL2_WRMFRZ		BIT(28)
> +#define FLEXCAN_CRL2_RFFN(x)		(((x) & 0x0f) << 24)
> +#define FLEXCAN_CRL2_TASD(x)		(((x) & 0x1f) << 19)
> +#define FLEXCAN_CRL2_MRP		BIT(18)
> +#define FLEXCAN_CRL2_RRS		BIT(17)
> +#define FLEXCAN_CRL2_EACEN		BIT(16)
> +
> +/* FLEXCAN memory error control register (MECR) bits */
> +#define FLEXCAN_MECR_ECRWRDIS		BIT(31)
> +#define FLEXCAN_MECR_HANCEI_MSK		BIT(19)
> +#define FLEXCAN_MECR_FANCEI_MSK		BIT(18)
> +#define FLEXCAN_MECR_CEI_MSK		BIT(16)
> +#define FLEXCAN_MECR_HAERRIE		BIT(15)
> +#define FLEXCAN_MECR_FAERRIE		BIT(14)
> +#define FLEXCAN_MECR_EXTERRIE		BIT(13)
> +#define FLEXCAN_MECR_RERRDIS		BIT(9)
> +#define FLEXCAN_MECR_ECCDIS		BIT(8)
> +#define FLEXCAN_MECR_NCEFAFRZ		BIT(7)
> +
>  /* FLEXCAN error and status register (ESR) bits */
>  #define FLEXCAN_ESR_TWRN_INT		BIT(17)
>  #define FLEXCAN_ESR_RWRN_INT		BIT(16)
> @@ -150,18 +171,20 @@
>   * FLEXCAN hardware feature flags
>   *
>   * Below is some version info we got:
> - *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT
> - *                                Filter?   connected?
> - *   MX25  FlexCAN2  03.00.00.00     no         no
> - *   MX28  FlexCAN2  03.00.04.00    yes        yes
> - *   MX35  FlexCAN2  03.00.00.00     no         no
> - *   MX53  FlexCAN2  03.00.00.00    yes         no
> - *   MX6s  FlexCAN3  10.00.12.00    yes        yes
> + *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT  Memory err
> + *                                Filter?   connected?  detection
> + *   MX25  FlexCAN2  03.00.00.00     no         no         no
> + *   MX28  FlexCAN2  03.00.04.00    yes        yes         no
> + *   MX35  FlexCAN2  03.00.00.00     no         no         no
> + *   MX53  FlexCAN2  03.00.00.00    yes         no         no
> + *   MX6s  FlexCAN3  10.00.12.00    yes        yes         no
> + *   VF610 FlexCAN3  ?               no         no        yes
>   *
>   * Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
>   */
>  #define FLEXCAN_HAS_V10_FEATURES	BIT(1) /* For core version >= 10 */
>  #define FLEXCAN_HAS_BROKEN_ERR_STATE	BIT(2) /* [TR]WRN_INT not connected */
> +#define FLEXCAN_HAS_MECR_FEATURES	BIT(3) /* Memory error detection */
>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -192,8 +215,11 @@ struct flexcan_regs {
>  	u32 crcr;		/* 0x44 */
>  	u32 rxfgmask;		/* 0x48 */
>  	u32 rxfir;		/* 0x4c */
> -	u32 _reserved3[12];
> +	u32 _reserved3[12];	/* 0x50 */
>  	struct flexcan_mb cantxfg[64];
> +	u32 _reserved4[408];
> +	u32 mecr;		/* 0xae0 */
> +	u32 erriar;		/* 0xae4 */
>  };
>  
>  struct flexcan_devtype_data {
> @@ -223,6 +249,9 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data;
>  static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  	.features = FLEXCAN_HAS_V10_FEATURES,
>  };
> +static struct flexcan_devtype_data fsl_vf610_devtype_data = {
> +	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
> +};
>  
>  static const struct can_bittiming_const flexcan_bittiming_const = {
>  	.name = DRV_NAME,
> @@ -807,7 +836,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	int err;
> -	u32 reg_mcr, reg_ctrl;
> +	u32 reg_mcr, reg_ctrl, reg_crl2, reg_mecr;
>  
>  	/* enable module */
>  	err = flexcan_chip_enable(priv);
> @@ -884,6 +913,31 @@ static int flexcan_chip_start(struct net_device *dev)
>  	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
>  		flexcan_write(0x0, &regs->rxfgmask);
>  
> +	/*
> +	 * On Vybrid, disable memory error detection interrupts
> +	 * and freeze mode.
> +	 * This also works around errata e5295 which generates
> +	 * false positive memory errors and put the device in
> +	 * freeze mode.
> +	 */
> +	if (priv->devtype_data->features & FLEXCAN_HAS_MECR_FEATURES) {
> +		/*
> +		 * Follow the protocol as described in "Detection
> +		 * and Correction of Memory Errors" to write to
> +		 * MECR register
> +		 */


One nitpick regarding the comment. In driver/net, the preferred
multi-line comment style is

/* comment starts here
 * comment continues
 */

> +		reg_crl2 = flexcan_read(&regs->crl2);
> +		reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> +		flexcan_write(reg_crl2, &regs->crl2);
> +
> +		reg_mecr = flexcan_read(&regs->mecr);
> +		reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> +		flexcan_write(reg_mecr, &regs->mecr);
> +		reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
> +				FLEXCAN_MECR_FANCEI_MSK);
> +		flexcan_write(reg_mecr, &regs->mecr);
> +	}
> +
>  	err = flexcan_transceiver_enable(priv);
>  	if (err)
>  		goto out_chip_disable;
> @@ -1094,6 +1148,7 @@ static const struct of_device_id flexcan_of_match[] = {
>  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>  	{ .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, },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, flexcan_of_match);
> 

Marc

-- 
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   |


Download attachment "signature.asc" of type "application/pgp-signature" (243 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ