Messages in this thread | | | From | Doug Anderson <> | Date | Fri, 7 Sep 2018 09:19:06 -0700 | Subject | Re: [PATCH V3] spi: spi-geni-qcom: Add SPI driver support for GENI based QUP |
| |
Hi,
On Fri, Sep 7, 2018 at 3:00 AM, <dkota@codeaurora.org> wrote: >> In v2, I said: >> >>> I'm not sure where to comment about this, so adding it to the end: >>> >>> Between v1 and v2 you totally removed all the locking. Presumably >>> this is because you didn't want to hold the lock in >>> handle_fifo_timeout() while waiting for the completion. IMO taking >>> the lock out was the wrong thing to do. You should keep it, but just >>> drop the lock before wait_for_completion_timeout() and add it back >>> afterwards. Specifically you _don't_ want the IRQ and timeout code >>> stomping on each other. >> >> >> ...but still no spinlock? > > I see there is no need of taking the spinlock as timeout will be handled > after the calculated time as per data size and speed. > There is 99.9% less chances of interrupt during the timeout handler. >> >> >> >> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1201081
The thing is, we want it to be 100% reliable, not 99.9% reliable. Is it somehow wrong to add the spinlock? ...or are you noticing performance problems with the spinlock there? It's just nice not to have to think about it.
-Doug
| |