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

[Dilip Kota]

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