linux-kernel - Re: [PATCH V3 2/2] can: xilinx_can: Add Transmitter delay compensation (TDC) feature support

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220609083139.sx2adt4raptu2jif@pengutronix.de>
Date:   Thu, 9 Jun 2022 10:31:39 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Srinivas Neeli <srinivas.neeli@...inx.com>
Cc:     wg@...ndegger.com, davem@...emloft.net, edumazet@...gle.com,
        srinivas.neeli@....com, neelisrinivas18@...il.com,
        appana.durga.rao@...inx.com, sgoud@...inx.com,
        michal.simek@...inx.com, kuba@...nel.org, pabeni@...hat.com,
        linux-can@...r.kernel.org, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        git@...inx.com
Subject: Re: [PATCH V3 2/2] can: xilinx_can: Add Transmitter delay
 compensation (TDC) feature support

On 09.06.2022 13:54:33, Srinivas Neeli wrote:
> Added Transmitter delay compensation (TDC) feature support.
> In the case of higher measured loop delay with higher baud rates,
> observed bit stuff errors. By enabling the TDC feature in
> CANFD controllers, will compensate for the measure loop delay in
> the receive path.
> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@...inx.com>
> ---
> Changes in V3:
> -Implemented GENMASK,FIELD_PERP & FIELD_GET Calls.
> -Implemented TDC feature for all Xilinx CANFD controllers.
> -corrected prescalar to prescaler(typo).
> Changes in V2:
> -Created two patchs one for revert another for TDC support.
> ---
>  drivers/net/can/xilinx_can.c | 48 ++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index e179d311aa28..288be69c0aed 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /* Xilinx CAN device driver
>   *
> - * Copyright (C) 2012 - 2014 Xilinx, Inc.
> + * Copyright (C) 2012 - 2022 Xilinx, Inc.
>   * Copyright (C) 2009 PetaLogix. All rights reserved.
>   * Copyright (C) 2017 - 2018 Sandvik Mining and Construction Oy
>   *
> @@ -9,6 +9,7 @@
>   * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> @@ -99,6 +100,7 @@ enum xcan_reg {
>  #define XCAN_ESR_STER_MASK		0x00000004 /* Stuff error */
>  #define XCAN_ESR_FMER_MASK		0x00000002 /* Form error */
>  #define XCAN_ESR_CRCER_MASK		0x00000001 /* CRC error */
> +#define XCAN_SR_TDCV_MASK		GENMASK(22, 16) /* TDCV Value */
>  #define XCAN_SR_TXFLL_MASK		0x00000400 /* TX FIFO is full */
>  #define XCAN_SR_ESTAT_MASK		0x00000180 /* Error status */
>  #define XCAN_SR_ERRWRN_MASK		0x00000040 /* Error warning */
> @@ -132,6 +134,8 @@ enum xcan_reg {
>  #define XCAN_DLCR_BRS_MASK		0x04000000 /* BRS Mask in DLC */
>  
>  /* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
> +#define XCAN_BRPR_TDCO_SHIFT		GENMASK(13, 8)  /* Transmitter Delay Compensation Offset */
                          ^^^^^
This is a MASK.

> +#define XCAN_BRPR_TDC_ENABLE		BIT(16) /* Transmitter Delay Compensation (TDC) Enable */
>  #define XCAN_BTR_SJW_SHIFT		7  /* Synchronous jump width */
>  #define XCAN_BTR_TS2_SHIFT		4  /* Time segment 2 */
>  #define XCAN_BTR_SJW_SHIFT_CANFD	16 /* Synchronous jump width */
> @@ -276,6 +280,16 @@ static const struct can_bittiming_const xcan_data_bittiming_const_canfd2 = {
>  	.brp_inc = 1,
>  };
>  
> +/* Transmission Delay Compensation constants for CANFD2.0 and Versal  */
> +static const struct can_tdc_const xcan_tdc_const = {
> +	.tdcv_min = 0,
> +	.tdcv_max = 0, /* Manual mode not supported. */
> +	.tdco_min = 0,
> +	.tdco_max = 64,
> +	.tdcf_min = 0, /* Filter window not supported */
> +	.tdcf_max = 0,
> +};
> +
>  /**
>   * xcan_write_reg_le - Write a value to the device register little endian
>   * @priv:	Driver private data structure
> @@ -405,7 +419,7 @@ static int xcan_set_bittiming(struct net_device *ndev)
>  		return -EPERM;
>  	}
>  
> -	/* Setting Baud Rate prescalar value in BRPR Register */
> +	/* Setting Baud Rate prescaler value in BRPR Register */

unrelated change, please make it a separate patch

>  	btr0 = (bt->brp - 1);
>  
>  	/* Setting Time Segment 1 in BTR Register */
> @@ -422,8 +436,12 @@ static int xcan_set_bittiming(struct net_device *ndev)
>  
>  	if (priv->devtype.cantype == XAXI_CANFD ||
>  	    priv->devtype.cantype == XAXI_CANFD_2_0) {
> -		/* Setting Baud Rate prescalar value in F_BRPR Register */
> +		/* Setting Baud Rate prescaler value in F_BRPR Register */

same

>  		btr0 = dbt->brp - 1;
> +		if (can_tdc_is_enabled(&priv->can))
> +			btr0 |=
> +			FIELD_PREP(XCAN_BRPR_TDCO_SHIFT, priv->can.tdc.tdco) |
> +			XCAN_BRPR_TDC_ENABLE;
>  
>  		/* Setting Time Segment 1 in BTR Register */
>  		btr1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> @@ -1483,6 +1501,22 @@ static int xcan_get_berr_counter(const struct net_device *ndev,
>  	return 0;
>  }
>  
> +/**
> + * xcan_get_auto_tdcv - Get Transmitter Delay Compensation Value
> + * @ndev:	Pointer to net_device structure
> + * @tdcv:	Pointer to TDCV value
> + *
> + * Return: 0 on success
> + */
> +static int xcan_get_auto_tdcv(const struct net_device *ndev, u32 *tdcv)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	*tdcv = FIELD_GET(XCAN_SR_TDCV_MASK, priv->read_reg(priv, XCAN_SR_OFFSET));
> +
> +	return 0;
> +}
> +
>  static const struct net_device_ops xcan_netdev_ops = {
>  	.ndo_open	= xcan_open,
>  	.ndo_stop	= xcan_close,
> @@ -1744,8 +1778,12 @@ static int xcan_probe(struct platform_device *pdev)
>  			&xcan_data_bittiming_const_canfd2;
>  
>  	if (devtype->cantype == XAXI_CANFD ||
> -	    devtype->cantype == XAXI_CANFD_2_0)
> -		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
> +	    devtype->cantype == XAXI_CANFD_2_0) {
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
> +						CAN_CTRLMODE_TDC_AUTO;
> +		priv->can.do_get_auto_tdcv = xcan_get_auto_tdcv;
> +		priv->can.tdc_const = &xcan_tdc_const;
> +	}
>  
>  	priv->reg_base = addr;
>  	priv->tx_max = tx_max;
> -- 
> 2.25.1
> 
> 

Otherwise looks good.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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