Re: [PATCH 07/16] spi: bcm63xx-hsspi: Add polling mode support

From: William Zhang <william.zhang@broadcom.com>
To: Mark Brown <broonie@kernel.org>
Cc: Linux SPI List <linux-spi@vger.kernel.org>,
	Broadcom Kernel List <bcm-kernel-feedback-list@broadcom.com>,
	anand.gore@broadcom.com, tomer.yacoby@broadcom.com,
	dan.beygelman@broadcom.com, joel.peshkin@broadcom.com,
	f.fainelli@gmail.com, jonas.gorski@gmail.com,
	kursad.oney@broadcom.com, dregan@mail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 07/16] spi: bcm63xx-hsspi: Add polling mode support
Date: Wed, 11 Jan 2023 14:57:31 -0800	[thread overview]
Message-ID: <45f71d68-1cbb-a775-46f3-e07ccce4c8d8@broadcom.com> (raw)
In-Reply-To: <Y787JLl9Hdp0sWsC@sirena.org.uk>

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


On 01/11/2023 02:41 PM, Mark Brown wrote:
> On Wed, Jan 11, 2023 at 12:13:43PM -0800, William Zhang wrote:
> 
>> Thanks for the explanation. I saw the spi-uniphier.c and spi-bcm2835.c doing
>> the trick you mentioned(thanks Kursad for pointing out).  In our case, even
>> the maximum fifo size usage(512bytes), the polling still have better
>> performance than interrupt. The MTD test result included in this patch is
>> based on maximum fifo usage. So there is no benefit to switch to interrupt
>> based on transfer size.
> 
>> Does the spi framework has any requirement on how much time that the
>> driver's transfer_one function can spend?  I can see the polling function
>> might take quite some time in busy loop if the clock is low, for example, at
>> 100Hz(slowest clock this controller can go), it takes 512x8/100Hz ~= 41ms to
>> complete.  If this is something in concern,  I can do the interrupt switch
>> based on a time limit value if interrupt is available.
> 
> There's no fixed limit, some hardware just doesn't have DMA.  Some
> doesn't even have interrupts which is even better.  If there's always a
> clear benefit for using PIO then just do that, perhaps creating a sysfs
> hook to allow people to switch to DMA if they need it (rather than
> requiring people to update their DT, this is really a runtime
> performance tradeoff rather than a description of the hardware).  If
> there's a point at which the performance is very similar then it's
> probably worth switching to DMA there to lower the CPU usage, but if
> it's always faster to use PIO then just defaulting to PIO seems like the
> best option.
> 
Sure. I will add the sysfs option. Interrupt node is now required in dts 
but by default it will still runs at polling mode until sysfs option is 
set to use interrupt.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]