[PATCH V2 0/2] xilinx_can: Update on xilinx can

[PATCH V2 0/2] xilinx_can: Update on xilinx can

[Srinivas Neeli]

This patch series addresses
1) Reverts the limiting CANFD brp_min to 2.
2) Adds TDC support for Xilinx can driver.

Srinivas Neeli (2):
  Revert "can: xilinx_can: Limit CANFD brp to 2"
  can: xilinx_can: Add Transmitter delay compensation (TDC) feature
    support

 drivers/net/can/xilinx_can.c | 50 ++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH V2 1/2] Revert "can: xilinx_can: Limit CANFD brp to 2"

[Srinivas Neeli]

This reverts commit 05ca14fdb6fe65614e0652d03e44b02748d25af7.

On early silicon engineering samples observed
bit shrinking issue when we use brp as 1.
Hence updated brp_min as 2. As in production
silicon this issue is fixed,so reverting the patch.

Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 8a3b7b103ca4..e179d311aa28 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -258,7 +258,7 @@ static const struct can_bittiming_const xcan_bittiming_const_canfd2 = {
 	.tseg2_min = 1,
 	.tseg2_max = 128,
 	.sjw_max = 128,
-	.brp_min = 2,
+	.brp_min = 1,
 	.brp_max = 256,
 	.brp_inc = 1,
 };
@@ -271,7 +271,7 @@ static const struct can_bittiming_const xcan_data_bittiming_const_canfd2 = {
 	.tseg2_min = 1,
 	.tseg2_max = 16,
 	.sjw_max = 16,
-	.brp_min = 2,
+	.brp_min = 1,
 	.brp_max = 256,
 	.brp_inc = 1,
 };
-- 
2.25.1

[PATCH V2 2/2] can: xilinx_can: Add Transmitter delay compensation (TDC) feature support

[Srinivas Neeli]

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 a controller,
will compensate for the measure loop delay in the receive path.

Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 46 +++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index e179d311aa28..d0edd1bca33c 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
  *
@@ -99,6 +99,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		0x007F0000 /* 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 +133,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		8  /* Transmitter Delay Compensation Offset */
+#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 */
@@ -140,6 +143,7 @@ enum xcan_reg {
 #define XCAN_IDR_ID2_SHIFT		1  /* Extended Message Identifier */
 #define XCAN_DLCR_DLC_SHIFT		28 /* Data length code */
 #define XCAN_ESR_REC_SHIFT		8  /* Rx Error Count */
+#define XCAN_SR_TDCV_SHIFT		16 /* TDCV Value */
 
 /* CAN frame length constants */
 #define XCAN_FRAME_MAX_DATA_LEN		8
@@ -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
@@ -424,6 +438,11 @@ static int xcan_set_bittiming(struct net_device *ndev)
 	    priv->devtype.cantype == XAXI_CANFD_2_0) {
 		/* Setting Baud Rate prescalar value in F_BRPR Register */
 		btr0 = dbt->brp - 1;
+		if (can_tdc_is_enabled(&priv->can)) {
+			btr0 = btr0 |
+			priv->can.tdc.tdco << XCAN_BRPR_TDCO_SHIFT |
+			XCAN_BRPR_TDC_ENABLE;
+		}
 
 		/* Setting Time Segment 1 in BTR Register */
 		btr1 = dbt->prop_seg + dbt->phase_seg1 - 1;
@@ -1483,6 +1502,23 @@ 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 = (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TDCV_MASK) >>
+		 XCAN_SR_TDCV_SHIFT;
+
+	return 0;
+}
+
 static const struct net_device_ops xcan_netdev_ops = {
 	.ndo_open	= xcan_open,
 	.ndo_stop	= xcan_close,
@@ -1734,18 +1770,22 @@ static int xcan_probe(struct platform_device *pdev)
 	priv->can.do_get_berr_counter = xcan_get_berr_counter;
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
 					CAN_CTRLMODE_BERR_REPORTING;
+	priv->can.do_get_auto_tdcv = xcan_get_auto_tdcv;
 
 	if (devtype->cantype == XAXI_CANFD)
 		priv->can.data_bittiming_const =
 			&xcan_data_bittiming_const_canfd;
 
-	if (devtype->cantype == XAXI_CANFD_2_0)
+	if (devtype->cantype == XAXI_CANFD_2_0) {
 		priv->can.data_bittiming_const =
 			&xcan_data_bittiming_const_canfd2;
+		priv->can.tdc_const = &xcan_tdc_const;
+	}
 
 	if (devtype->cantype == XAXI_CANFD ||
 	    devtype->cantype == XAXI_CANFD_2_0)
-		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
+		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
+						CAN_CTRLMODE_TDC_AUTO;
 
 	priv->reg_base = addr;
 	priv->tx_max = tx_max;
-- 
2.25.1

Re: [PATCH V2 1/2] Revert "can: xilinx_can: Limit CANFD brp to 2"

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

On 07.06.2022 14:26:53, Srinivas Neeli wrote:
> This reverts commit 05ca14fdb6fe65614e0652d03e44b02748d25af7.
> 
> On early silicon engineering samples observed
> bit shrinking issue when we use brp as 1.
> Hence updated brp_min as 2. As in production
> silicon this issue is fixed,so reverting the patch.
> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>

Should this be applied to -stable?

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 |

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

Re: [PATCH V2 2/2] can: xilinx_can: Add Transmitter delay compensation (TDC) feature support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 5901 bytes --]

Hello Srinivas Neeli,

thanks for your patch!

On 07.06.2022 14:26:54, 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 a controller,
> will compensate for the measure loop delay in the receive path.

Wich controllers support TDC?

XAXI_CANFD doesn't have do_get_auto_tdc assigned, but
CAN_CTRLMODE_TDC_AUTO is set.

> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> ---
>  drivers/net/can/xilinx_can.c | 46 +++++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index e179d311aa28..d0edd1bca33c 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
>   *
> @@ -99,6 +99,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		0x007F0000 /* 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 +133,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		8  /* Transmitter Delay Compensation Offset */
> +#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 */
> @@ -140,6 +143,7 @@ enum xcan_reg {
>  #define XCAN_IDR_ID2_SHIFT		1  /* Extended Message Identifier */
>  #define XCAN_DLCR_DLC_SHIFT		28 /* Data length code */
>  #define XCAN_ESR_REC_SHIFT		8  /* Rx Error Count */
> +#define XCAN_SR_TDCV_SHIFT		16 /* TDCV Value */
>  
>  /* CAN frame length constants */
>  #define XCAN_FRAME_MAX_DATA_LEN		8
> @@ -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
> @@ -424,6 +438,11 @@ static int xcan_set_bittiming(struct net_device *ndev)
>  	    priv->devtype.cantype == XAXI_CANFD_2_0) {
>  		/* Setting Baud Rate prescalar value in F_BRPR Register */
>  		btr0 = dbt->brp - 1;
> +		if (can_tdc_is_enabled(&priv->can)) {
> +			btr0 = btr0 |

Make use of "|=" and properly indent.

> +			priv->can.tdc.tdco << XCAN_BRPR_TDCO_SHIFT |

Please include <linux/bitfield.h> and make use of "FIELD_PREP".

> +			XCAN_BRPR_TDC_ENABLE;
> +		}
>  
>  		/* Setting Time Segment 1 in BTR Register */
>  		btr1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> @@ -1483,6 +1502,23 @@ 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 = (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TDCV_MASK) >>
> +		 XCAN_SR_TDCV_SHIFT;

Please use FIELD_GET.

> +
> +	return 0;
> +}
> +
>  static const struct net_device_ops xcan_netdev_ops = {
>  	.ndo_open	= xcan_open,
>  	.ndo_stop	= xcan_close,
> @@ -1734,18 +1770,22 @@ static int xcan_probe(struct platform_device *pdev)
>  	priv->can.do_get_berr_counter = xcan_get_berr_counter;
>  	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>  					CAN_CTRLMODE_BERR_REPORTING;
> +	priv->can.do_get_auto_tdcv = xcan_get_auto_tdcv;

I'm not sure, if it has any side effects, if you assign do_get_auto_tdc
for all controllers, even the ones that don't support it. Vincent can
probably clarify this.

>  
>  	if (devtype->cantype == XAXI_CANFD)
>  		priv->can.data_bittiming_const =
>  			&xcan_data_bittiming_const_canfd;
>  
> -	if (devtype->cantype == XAXI_CANFD_2_0)
> +	if (devtype->cantype == XAXI_CANFD_2_0) {
>  		priv->can.data_bittiming_const =
>  			&xcan_data_bittiming_const_canfd2;
> +		priv->can.tdc_const = &xcan_tdc_const;
> +	}
>  
>  	if (devtype->cantype == XAXI_CANFD ||
>  	    devtype->cantype == XAXI_CANFD_2_0)
> -		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
> +						CAN_CTRLMODE_TDC_AUTO;
>  
>  	priv->reg_base = addr;
>  	priv->tx_max = tx_max;

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

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

Re: [PATCH V2 2/2] can: xilinx_can: Add Transmitter delay compensation (TDC) feature support

[Vincent MAILHOL]

 On Tue. 7 Jun. 2022 at 18:19, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> Hello Srinivas Neeli,
>
> thanks for your patch!
>
> On 07.06.2022 14:26:54, 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 a controller,
> > will compensate for the measure loop delay in the receive path.
>
> Wich controllers support TDC?
>
> XAXI_CANFD doesn't have do_get_auto_tdc assigned, but
> CAN_CTRLMODE_TDC_AUTO is set.
>
> > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> > ---
> >  drivers/net/can/xilinx_can.c | 46 +++++++++++++++++++++++++++++++++---
> >  1 file changed, 43 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> > index e179d311aa28..d0edd1bca33c 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
> >   *
> > @@ -99,6 +99,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            0x007F0000 /* 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 +133,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         8  /* Transmitter Delay Compensation Offset */
> > +#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 */
> > @@ -140,6 +143,7 @@ enum xcan_reg {
> >  #define XCAN_IDR_ID2_SHIFT           1  /* Extended Message Identifier */
> >  #define XCAN_DLCR_DLC_SHIFT          28 /* Data length code */
> >  #define XCAN_ESR_REC_SHIFT           8  /* Rx Error Count */
> > +#define XCAN_SR_TDCV_SHIFT           16 /* TDCV Value */
> >
> >  /* CAN frame length constants */
> >  #define XCAN_FRAME_MAX_DATA_LEN              8
> > @@ -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
> > @@ -424,6 +438,11 @@ static int xcan_set_bittiming(struct net_device *ndev)
> >           priv->devtype.cantype == XAXI_CANFD_2_0) {> >               /* Setting Baud Rate prescalar value in F_BRPR Register */
                                       ^^^^^^^^^
Not related to this patch, but this is a typo. prescalar -> prescaler.

> >               btr0 = dbt->brp - 1;
> > +             if (can_tdc_is_enabled(&priv->can)) {
> > +                     btr0 = btr0 |
>
> Make use of "|=" and properly indent.
>
> > +                     priv->can.tdc.tdco << XCAN_BRPR_TDCO_SHIFT |
>
> Please include <linux/bitfield.h> and make use of "FIELD_PREP".
>
> > +                     XCAN_BRPR_TDC_ENABLE;
> > +             }
> >
> >               /* Setting Time Segment 1 in BTR Register */
> >               btr1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> > @@ -1483,6 +1502,23 @@ 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 = (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TDCV_MASK) >>
> > +              XCAN_SR_TDCV_SHIFT;
>
> Please use FIELD_GET.
>
> > +
> > +     return 0;
> > +}
> > +
> >  static const struct net_device_ops xcan_netdev_ops = {
> >       .ndo_open       = xcan_open,
> >       .ndo_stop       = xcan_close,
> > @@ -1734,18 +1770,22 @@ static int xcan_probe(struct platform_device *pdev)
> >       priv->can.do_get_berr_counter = xcan_get_berr_counter;
> >       priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> >                                       CAN_CTRLMODE_BERR_REPORTING;
> > +     priv->can.do_get_auto_tdcv = xcan_get_auto_tdcv;
>
> I'm not sure, if it has any side effects, if you assign do_get_auto_tdc
> for all controllers, even the ones that don't support it. Vincent can
> probably clarify this.

It should be fine. can_priv::do_get_auto_tdcv() gets called in one
single location in can_tdc_fill_info(). c.f.
https://elixir.bootlin.com/linux/v5.18/source/drivers/net/can/dev/netlink.c#L464

can_tdc_fill_info() returns if can_priv::tdc_const is not populated.
https://elixir.bootlin.com/linux/v5.18/source/drivers/net/can/dev/netlink.c#L438

Below, you only assign tdc_const only for XAXI_CANFD_2_0, so this
should be safe. *BUT*, I do not think this helps for the code
readability so I encourage you to only populate
can_priv::do_get_auto_tdcv() for the controllers which do support TDC.

> >
> >       if (devtype->cantype == XAXI_CANFD)
> >               priv->can.data_bittiming_const =
> >                       &xcan_data_bittiming_const_canfd;
> >
> > -     if (devtype->cantype == XAXI_CANFD_2_0)
> > +     if (devtype->cantype == XAXI_CANFD_2_0) {
> >               priv->can.data_bittiming_const =
> >                       &xcan_data_bittiming_const_canfd2;
> > +             priv->can.tdc_const = &xcan_tdc_const;

Here, you only populate tdc_const for XAXI_CANFD_2_0...

> > +     }
> >
> >       if (devtype->cantype == XAXI_CANFD ||
> >           devtype->cantype == XAXI_CANFD_2_0)
> > -             priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
> > +             priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
> > +                                             CAN_CTRLMODE_TDC_AUTO;

...but here, you set the CAN_CTRLMODE_TDC_AUTO for both XAXI_CANFD and
XAXI_CANFD_2_0. Isn’t this a mismatch?

I suggest to have a single if block in which you populate all the TDC
fields at once:
|         if (devtype->cantype == XAXI_CANFD_2_0) {
|                 priv->can.data_bittiming_const =
|                         &xcan_data_bittiming_const_canfd2;
|                 priv->can.tdc_const = &xcan_tdc_const;
|                 priv->can.do_get_auto_tdcv = xcan_get_auto_tdcv;
|                 priv->can.ctrlmode_supported |= CAN_CTRLMODE_TDC_AUTO;
|         }

> >       priv->reg_base = addr;
> >       priv->tx_max = tx_max;


Yours sincerely,
Vincent Mailhol