Messages in this thread | | | Subject | Re: [PATCH 1/4] spi: lantiq: Synchronize interrupt handlers and transfers | From | Dilip Kota <> | Date | Wed, 29 Apr 2020 16:20:53 +0800 |
| |
On 4/28/2020 7:10 PM, Daniel Schwierzeck wrote: > > Am 24.04.20 um 12:42 schrieb Dilip Kota: >> Synchronize tx, rx and error interrupts by registering to the >> same interrupt handler. Interrupt handler will recognize and process >> the appropriate interrupt on the basis of interrupt status register. >> Also, establish synchronization between the interrupt handler and >> transfer operation by taking the locks and registering the interrupt >> handler as thread IRQ which avoids the bottom half. > actually there is no real bottom half. Reading or writing the FIFOs is > fast and is therefore be done in hard IRQ context. But as the comment Doing FIFO r/w in threaded irqs shouldn't cause any impact on maximum transfer rate i think. Also the ISR should be quick enough, doing FIFO r/w in ISR adds up more latency to ISR. Handling the FIFOs r/w in threaded irq will be a better way. > for lantiq_ssc_bussy_work() state, the driver needs some busy-waiting > after the last interrupt. I don't think it's worth to replace this with > threaded interrupts which add more runtime overhead and likely decrease > the maximum transfer speed. Workqueue has a higher chances of causing SPI transfers timedout.
> >> Fixes the wrongly populated interrupt register offsets too. >> >> Fixes: 17f84b793c01 ("spi: lantiq-ssc: add support for Lantiq SSC SPI controller") >> Fixes: ad2fca0721d1 ("spi: lantiq-ssc: add LTQ_ prefix to defines") >> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com> >> --- >> drivers/spi/spi-lantiq-ssc.c | 89 ++++++++++++++++++++++---------------------- >> 1 file changed, 45 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/spi/spi-lantiq-ssc.c b/drivers/spi/spi-lantiq-ssc.c >> index 1fd7ee53d451..b67f5925bcb0 100644 >> --- a/drivers/spi/spi-lantiq-ssc.c >> +++ b/drivers/spi/spi-lantiq-ssc.c >> @@ -6,6 +6,7 @@ >> >> #include <linux/kernel.h> >> #include <linux/module.h> >> +#include <linux/mutex.h> >> #include <linux/of_device.h> >> #include <linux/clk.h> >> #include <linux/io.h> >> @@ -13,7 +14,6 @@ >> #include <linux/interrupt.h> >> #include <linux/sched.h> >> #include <linux/completion.h> >> -#include <linux/spinlock.h> >> #include <linux/err.h> >> #include <linux/gpio.h> >> #include <linux/pm_runtime.h> >> @@ -50,8 +50,8 @@ >> #define LTQ_SPI_RXCNT 0x84 >> #define LTQ_SPI_DMACON 0xec >> #define LTQ_SPI_IRNEN 0xf4 >> -#define LTQ_SPI_IRNICR 0xf8 >> -#define LTQ_SPI_IRNCR 0xfc >> +#define LTQ_SPI_IRNCR 0xf8 >> +#define LTQ_SPI_IRNICR 0xfc > the values are matching the datasheets for Danube and VRX200 family. > AFAICS the registers have been swapped for some newer SoCs like GRX330 > or GRX550. It didn't matter until now because those registers were > unused by the driver. So if you want to use those registers, you have to > deal somehow with the register offset swap in struct lantiq_ssc_hwcfg. In the initial phase of the patch, i have written the code considering the interrupt offsets in latest chipsets are different from the older chipsets. But, when i refered the Hauke's changes to add support for xrx500(which he done internally), offsets are corrected . So i did the same.
I will define these offsets in the SoC data structure which i have done initially.
Regards, Dilip >> >>
| |