Re: [PATCH RFC] can: c_can: don't cache TX messages for C_CAN cores

[Jacob Kroon <jacob.kroon@xxxxxxxxx>]

Hi Marc,

On 9/23/22 13:42, Marc Kleine-Budde wrote:
> Link: https://lore.kernel.org/all/15a8084b-9617-2da1-6704-d7e39d60643b@xxxxxxxxx
> Reported-by: Jacob Kroon <jacob.kroon@xxxxxxxxx>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
>   drivers/net/can/c_can/c_can.h      | 17 +++++++++++++++--
>   drivers/net/can/c_can/c_can_main.c | 11 +++++------
>   2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index f23a03300a81..029cd8194ed5 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -235,9 +235,22 @@ static inline u8 c_can_get_tx_tail(const struct c_can_tx_ring *ring)
>   	return ring->tail & (ring->obj_num - 1);
>   }
>   
> -static inline u8 c_can_get_tx_free(const struct c_can_tx_ring *ring)
> +static inline u8 c_can_get_tx_free(const struct c_can_priv *priv,
> +				   const struct c_can_tx_ring *ring)
>   {
> -	return ring->obj_num - (ring->head - ring->tail);
> +	u8 head = c_can_get_tx_head(ring);
> +	u8 tail = c_can_get_tx_tail(ring);
> +
> +	if (priv->type == BOSCH_D_CAN)
> +		return ring->obj_num - (ring->head - ring->tail);
> +
> +	/* This is not a FIFO. C/D_CAN sends out the buffers
> +	 * prioritized. The lowest buffer number wins.
> +	 */
> +	if (head < tail)
> +		return 0;
> +
> +	return ring->obj_num - head;
>   }
>   
>   #endif /* C_CAN_H */
> diff --git a/drivers/net/can/c_can/c_can_main.c b/drivers/net/can/c_can/c_can_main.c
> index dc8132862f33..d6605dbb7737 100644
> --- a/drivers/net/can/c_can/c_can_main.c
> +++ b/drivers/net/can/c_can/c_can_main.c
> @@ -429,7 +429,7 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
>   static bool c_can_tx_busy(const struct c_can_priv *priv,
>   			  const struct c_can_tx_ring *tx_ring)
>   {
> -	if (c_can_get_tx_free(tx_ring) > 0)
> +	if (c_can_get_tx_free(priv, tx_ring) > 0)
>   		return false;
>   
>   	netif_stop_queue(priv->dev);
> @@ -437,7 +437,7 @@ static bool c_can_tx_busy(const struct c_can_priv *priv,
>   	/* Memory barrier before checking tx_free (head and tail) */
>   	smp_mb();
>   
> -	if (c_can_get_tx_free(tx_ring) == 0) {
> +	if (c_can_get_tx_free(priv, tx_ring) == 0) {
>   		netdev_dbg(priv->dev,
>   			   "Stopping tx-queue (tx_head=0x%08x, tx_tail=0x%08x, len=%d).\n",
>   			   tx_ring->head, tx_ring->tail,
> @@ -465,7 +465,7 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
>   
>   	idx = c_can_get_tx_head(tx_ring);
>   	tx_ring->head++;
> -	if (c_can_get_tx_free(tx_ring) == 0)
> +	if (c_can_get_tx_free(priv, tx_ring) == 0)
>   		netif_stop_queue(dev);
>   
>   	if (idx < c_can_get_tx_tail(tx_ring))
> @@ -748,7 +748,7 @@ static void c_can_do_tx(struct net_device *dev)
>   		return;
>   
>   	tx_ring->tail += pkts;
> -	if (c_can_get_tx_free(tx_ring)) {
> +	if (c_can_get_tx_free(priv, tx_ring)) {
>   		/* Make sure that anybody stopping the queue after
>   		 * this sees the new tx_ring->tail.
>   		 */
> @@ -760,8 +760,7 @@ static void c_can_do_tx(struct net_device *dev)
>   	stats->tx_packets += pkts;
>   
>   	tail = c_can_get_tx_tail(tx_ring);
> -
> -	if (tail == 0) {
> +	if (priv->type == BOSCH_D_CAN && tail == 0) {
>   		u8 head = c_can_get_tx_head(tx_ring);
>   
>   		/* Start transmission for all cached messages */

Thanks for looking into the issue.

Perhaps not too surprising, but I'm testing with this patch and 
everything looks good sofar, ~20 minutes into testing. I will leave it 
running overnight to be sure.

Thanks,
Jacob