Re: [PATCH v3 11/16] can: m_can: Cache tx putidx

[Simon Horman]

On Wed, Mar 15, 2023 at 12:05:41PM +0100, Markus Schneider-Pargmann wrote:
> m_can_tx_handler is the only place where data is written to the tx fifo.
> We can calculate the putidx in the driver code here to avoid the
> dependency on the txfqs register.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx>
> ---
>  drivers/net/can/m_can/m_can.c | 8 +++++++-
>  drivers/net/can/m_can/m_can.h | 3 +++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 4e794166664a..d5bcce948d2c 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1475,6 +1475,10 @@ static int m_can_start(struct net_device *dev)
>  
>  	m_can_enable_all_interrupts(cdev);
>  
> +	if (cdev->version > 30)
> +		cdev->tx_fifo_putidx = FIELD_GET(TXFQS_TFQPI_MASK,
> +						 m_can_read(cdev, M_CAN_TXFQS));
> +
>  	return 0;
>  }
>  
> @@ -1765,7 +1769,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
>  		}
>  
>  		/* get put index for frame */
> -		putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);
> +		putidx = cdev->tx_fifo_putidx;
>  
>  		/* Construct DLC Field, with CAN-FD configuration.
>  		 * Use the put index of the fifo as the message marker,
> @@ -1798,6 +1802,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
>  
>  		/* Enable TX FIFO element to start transfer  */
>  		m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
> +		cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
> +					0 : cdev->tx_fifo_putidx);
>  
>  		/* stop network queue if fifo full */
>  		if (m_can_tx_fifo_full(cdev) ||
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index d0c21eddb6ec..548ae908ac4e 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -102,6 +102,9 @@ struct m_can_classdev {
>  	u32 tx_max_coalesced_frames_irq;
>  	u32 tx_coalesce_usecs_irq;
>  
> +	// Store this internally to avoid fetch delays on peripheral chips
> +	int tx_fifo_putidx;

nit: it might be slightly nicer to do a pass over the code
     and make putidx unsigned - assuming it is an unsigned value.

> +
>  	struct mram_cfg mcfg[MRAM_CFG_NUM];
>  };
>  
> -- 
> 2.39.2
>