Linux CAN drivers development
 help / color / mirror / Atom feed
From: Torin Cooper-Bennun <torin@maxiluxsystems.com>
To: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Subject: Improving TX for m_can peripherals
Date: Wed, 5 May 2021 11:30:49 +0100	[thread overview]
Message-ID: <20210505103049.gboat4dr3zvdm4s6@bigthink> (raw)

Good morning candevs,

I've been testing the TCAN4550 recently with proper kit (no more jumper
wires, hooray!) and I'm happy to say the RX path is fixed in v5.12 with
the latest patches, and even with heavy load I see no missed frames or
errors.

However, TX still needs work. It's easy to break the driver due to the
following logic in m_can_start_xmit():

| 	if (cdev->tx_skb) {
| 		netdev_err(dev, "hard_xmit called while tx busy\n");
| 		return NETDEV_TX_BUSY;
| 	}

Regardless of your netif TX queue length or the number of TX buffers
allocated in the M_CAN core, if you try to transmit too quickly, you
will hit this. For the application I'm working on, I run into this very
quickly with real-world scenarios.

Also, the queue is always stopped before the tx work is queued in
m_can_start_xmit(), which seems wrong and clearly doesn't solve the
problem:

| 	/* Need to stop the queue to avoid numerous requests
| 	 * from being sent.  Suggested improvement is to create
| 	 * a queueing mechanism that will queue the skbs and
| 	 * process them in order.
| 	 */
| 	cdev->tx_skb = skb;
| 	netif_stop_queue(cdev->net);
| 	queue_work(cdev->tx_wq, &cdev->tx_work);


So - I'd like to fix this. The comment in the snippet above suggests a
queueing mechanism. It would be good to hear your take on this, Marc -
AFAIU you have written a similar mechanism for mcp251xfd. :)

--
Regards,

Torin Cooper-Bennun
Software Engineer | maxiluxsystems.com


             reply	other threads:[~2021-05-05 10:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 10:30 Torin Cooper-Bennun [this message]
2021-05-05 11:56 ` Improving TX for m_can peripherals Marc Kleine-Budde
2021-05-05 12:21   ` Torin Cooper-Bennun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210505103049.gboat4dr3zvdm4s6@bigthink \
    --to=torin@maxiluxsystems.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox