Re: [PATCH 1/4] spi: lantiq: Synchronize interrupt handlers and transfers

[Dilip Kota <eswara.kota@linux.intel.com>]

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