Re: [PATCH 02/15] can: m_can: Wakeup net queue once tx was issued - Marc Kleine-Budde

From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Markus Schneider-Pargmann <msp@baylibre.com>
Cc: Chandrasekar Ramakrishnan <rcsekar@samsung.com>,
	Wolfgang Grandegger <wg@grandegger.com>,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/15] can: m_can: Wakeup net queue once tx was issued
Date: Thu, 1 Dec 2022 10:16:05 +0100	[thread overview]
Message-ID: <20221201091605.jgd7dlswcbxapdy3@pengutronix.de> (raw)
In-Reply-To: <20221201084302.oodh22xgvwsjmoc3@blmsp>

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

On 01.12.2022 09:43:02, Markus Schneider-Pargmann wrote:
> Hi Marc,
> 
> On Wed, Nov 30, 2022 at 06:21:00PM +0100, Marc Kleine-Budde wrote:
> > On 16.11.2022 21:52:55, Markus Schneider-Pargmann wrote:
> > > Currently the driver waits to wakeup the queue until the interrupt for
> > > the transmit event is received and acknowledged. If we want to use the
> > > hardware FIFO, this is too late.
> > > 
> > > Instead release the queue as soon as the transmit was transferred into
> > > the hardware FIFO. We are then ready for the next transmit to be
> > > transferred.
> > 
> > If you want to really speed up the TX path, remove the worker and use
> > the spi_async() API from the xmit callback, see mcp251xfd_start_xmit().
> 
> Good idea. I will check how regmap's async_write works and if it is
> suitable to do the job. I don't want to drop the regmap usage for this
> right now.

IIRC regmap async write still uses mutexes, but sleeping is not allowed
in the xmit handler. The mcp251xfd driver does the endianness conversion
(and the optional CRC) manually for the TX path.

Sending directly from the xmit handler basically eliminates the queuing
between the network stack and the worker. Getting rid of the worker
makes life easier and it's faster anyways.

> > Extra bonus if you implement xmit_more() and transfer more than 1 skb
> > per SPI transfer.
> 
> That's on my todo list, but I am not sure I will get to it soonish.

I haven't implemented this for the mcp251xfd, yet, but I have some
proof-of-concept code somewhere. However, the mcp251xfd driver already
implemented byte queue limits: 0084e298acfe ("can: mcp251xfd: add BQL
support").

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