On 2022/6/17 19:45, Mark Brown wrote:
On Fri, Jun 17, 2022 at 02:24:10PM +0800, Jon Lin wrote:
On 2022/6/13 20:37, Mark Brown wrote:
On Mon, Jun 13, 2022 at 05:27:44PM +0800, Jon Lin wrote:
+ spin_unlock_irqrestore(&rs->lock, flags);
So this is effectively just disabling interrupts during PIO, there's no
other users of the lock which is rather heavyweight. What's the actual
issue here? We should also have something saying what's going on in the
code since right now the lock just looks redundant.
For lock: In order to avoid special situations, such as when the CPU
frequency drops to close to the IO rate, the water line interrupt is
triggered during FIFO filling (triggered by other CPUs), resulting in
critical resources still not being protected in place. For local IRQ
So essentially we're so slow in filling the FIFO when starting a
transfer that the interrupt triggers in the middle of the initial FIFO
fill? Something that tricky *really* needs a comment adding.
Ideally we'd just leave the interrupt masked until the FIFO is filled
though, looking at the driver I see that there is an interrupt mask
register which seems to have some level of masking available and I do
note that in rockchip_spi_prepare_irq() we unmask interrupts before we
start filling the FIFO rather than afterwards. Would reversing the
unmask order there address the issue more cleanly?
This idea is workable, and it's more efficient than previous code, So I
send a new commit:
https://patchwork.kernel.org/project/spi-devel-general/patch/20220617124251.5051-1-jon.lin@xxxxxxxxxxxxxx/
disable: Turning off the local interrupt is mainly to prevent the CPU
schedule from being interrupted when filling FIFO.
If it were just this then there's preempt_disable(), but what's the
problem with being preempted during the FIFO fill?
I think
