| Message ID | 20230419060639.38853-1-jaewon02.kim@samsung.com |
|---|---|
| Headers | show |
| Series | Improves polling mode of s3c64xx driver | expand |
On 19/04/2023 08:06, Jaewon Kim wrote: > 1. > s3cx64xx driver was supporting polling mode using quirk for SOC without DMA. > However, in order to use PIO mode as an optional rather than a quirk, when DMA > is not described, spi operates with pio mode rather than probe fail. > > 2. > Fixed the problem of high CPU usage in PIO mode. > > 3. > If the transfer data size is larger than 32-bit, IRQ base PIO mode used. > What changed in the patches? You need to provide changelog. Best regards, Krzysztof
On 19/04/2023 08:06, Jaewon Kim wrote: > Adds cpu_relax() to prevent long busy-wait. How cpu_relax prevents long waiting? > There is busy-wait loop to check data transfer completion in polling mode. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 273aa02322d9..886722fb40ea 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -568,6 +568,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, > > val = msecs_to_loops(ms); > do { > + cpu_relax(); Shouldn't this be just readl_poll_timeout()? Or the syntax would be too complicated? > status = readl(regs + S3C64XX_SPI_STATUS); > } while (RX_FIFO_LVL(status, sdd) < xfer->len && --val); > Best regards, Krzysztof
On 19/04/2023 08:06, Jaewon Kim wrote: > Interrupt based pio mode is supported to reduce CPU load. > If transfer size is larger than 32 byte, it is processed using interrupt. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 82 ++++++++++++++++++++++++++++++++------- > 1 file changed, 67 insertions(+), 15 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index cf3060b2639b..ce1afb9a4ed4 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -58,6 +58,8 @@ > #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17) > #define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17) > #define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17) > +#define S3C64XX_SPI_MODE_RX_RDY_LVL GENMASK(16, 11) > +#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT 11 > #define S3C64XX_SPI_MODE_SELF_LOOPBACK (1<<3) > #define S3C64XX_SPI_MODE_RXDMA_ON (1<<2) > #define S3C64XX_SPI_MODE_TXDMA_ON (1<<1) > @@ -114,6 +116,8 @@ > > #define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT > > +#define S3C64XX_SPI_POLLING_SIZE 32 > + > #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t) > #define is_polling(x) (x->cntrlr_info->polling) > > @@ -552,10 +556,11 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, > } > > static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, > - struct spi_transfer *xfer) > + struct spi_transfer *xfer, int use_irq) > { > void __iomem *regs = sdd->regs; > unsigned long val; > + unsigned long time; > u32 status; > int loops; > u32 cpy_len; > @@ -563,17 +568,24 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, > int ms; > u32 tx_time; > > - /* sleep during signal transfer time */ > - status = readl(regs + S3C64XX_SPI_STATUS); > - if (RX_FIFO_LVL(status, sdd) < xfer->len) { > - tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; > - usleep_range(tx_time / 2, tx_time); > - } You just added this code. Adding and immediately removing it, suggests this should be one patch. Best regards, Krzysztof
On 23. 4. 19. 17:19, Krzysztof Kozlowski wrote: > On 19/04/2023 08:06, Jaewon Kim wrote: >> In polling mode, the status register is constantly read to check transfer >> completion. It cause excessive CPU usage. >> So, it calculates the SPI transfer time and made it sleep. >> >> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 886722fb40ea..cf3060b2639b 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -561,6 +561,14 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >> u32 cpy_len; >> u8 *buf; >> int ms; >> + u32 tx_time; >> + >> + /* sleep during signal transfer time */ >> + status = readl(regs + S3C64XX_SPI_STATUS); >> + if (RX_FIFO_LVL(status, sdd) < xfer->len) { >> + tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; >> + usleep_range(tx_time / 2, tx_time); >> + } > Did you actually check the delays introduced by it? Is it worth? Yes, I already test it. Throughput was the same, CPU utilization decreased to 30~40% from 100%. Tested board is ExynosAutov9 SADK. > >> >> /* millisecs to xfer 'len' bytes @ 'cur_speed' */ >> ms = xfer->len * 8 * 1000 / sdd->cur_speed; > You have now some code duplication so this could be combined. > > Best regards, > Krzysztof > > Thanks Jaewon Kim
On 23. 4. 19. 17:14, Krzysztof Kozlowski wrote: > On 19/04/2023 08:06, Jaewon Kim wrote: >> Adds cpu_relax() to prevent long busy-wait. > How cpu_relax prevents long waiting? As I know, cpu_relax() can be converted to yield. This can prevent excessive use of the CPU in busy-loop. I'll replace poor sentence like below in v3. ("Adds cpu_relax() to allow CPU relaxation in busy-loop") >> There is busy-wait loop to check data transfer completion in polling mode. >> >> Signed-off-by: Jaewon Kim<jaewon02.kim@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 273aa02322d9..886722fb40ea 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -568,6 +568,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >> >> val = msecs_to_loops(ms); >> do { >> + cpu_relax(); > Shouldn't this be just readl_poll_timeout()? Or the syntax would be too > complicated? I think we can replace this while() loop to readl_poll_timeout(). However, we should use 0 value as 'delay_us' parameter. Because delay can affect throughput. My purpose is add relax to this busy-loop. we cannot give relax if we change to readl_poll_timeout(). >> status = readl(regs + S3C64XX_SPI_STATUS); >> } while (RX_FIFO_LVL(status, sdd) < xfer->len && --val); >> > Best regards, > Krzysztof > > Thanks Jaewon Kim
Hi Jaewon, On Wed, Apr 19, 2023 at 03:06:39PM +0900, Jaewon Kim wrote: > Interrupt based pio mode is supported to reduce CPU load. > If transfer size is larger than 32 byte, it is processed using interrupt. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 82 ++++++++++++++++++++++++++++++++------- > 1 file changed, 67 insertions(+), 15 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index cf3060b2639b..ce1afb9a4ed4 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -58,6 +58,8 @@ > #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17) > #define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17) > #define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17) > +#define S3C64XX_SPI_MODE_RX_RDY_LVL GENMASK(16, 11) > +#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT 11 > #define S3C64XX_SPI_MODE_SELF_LOOPBACK (1<<3) > #define S3C64XX_SPI_MODE_RXDMA_ON (1<<2) > #define S3C64XX_SPI_MODE_TXDMA_ON (1<<1) > @@ -114,6 +116,8 @@ > > #define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT > > +#define S3C64XX_SPI_POLLING_SIZE 32 > + > #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t) > #define is_polling(x) (x->cntrlr_info->polling) > > @@ -552,10 +556,11 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, > } > > static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, > - struct spi_transfer *xfer) > + struct spi_transfer *xfer, int use_irq) bool use_irq > { > void __iomem *regs = sdd->regs; > unsigned long val; > + unsigned long time; this time is used only in "if (use_irq)" can you move its declaration under the if? > u32 status; > int loops; > u32 cpy_len; > @@ -563,17 +568,24 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, > int ms; > u32 tx_time; > > - /* sleep during signal transfer time */ > - status = readl(regs + S3C64XX_SPI_STATUS); > - if (RX_FIFO_LVL(status, sdd) < xfer->len) { > - tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; > - usleep_range(tx_time / 2, tx_time); > - } > - > /* millisecs to xfer 'len' bytes @ 'cur_speed' */ > ms = xfer->len * 8 * 1000 / sdd->cur_speed; > ms += 10; /* some tolerance */ > > + if (use_irq) { > + val = msecs_to_jiffies(ms); > + time = wait_for_completion_timeout(&sdd->xfer_completion, val); > + if (!time) > + return -EIO; > + } else { > + /* sleep during signal transfer time */ > + status = readl(regs + S3C64XX_SPI_STATUS); > + if (RX_FIFO_LVL(status, sdd) < xfer->len) { > + tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; > + usleep_range(tx_time / 2, tx_time); yeah... just use 'ms'. > + } > + } > + > val = msecs_to_loops(ms); > do { > cpu_relax(); > @@ -737,10 +749,13 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, > void *rx_buf = NULL; > int target_len = 0, origin_len = 0; > int use_dma = 0; > + int use_irq = 0; > int status; > u32 speed; > u8 bpw; > unsigned long flags; > + u32 rdy_lv; > + u32 val; > > reinit_completion(&sdd->xfer_completion); > > @@ -761,17 +776,46 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, > sdd->rx_dma.ch && sdd->tx_dma.ch) { > use_dma = 1; > > - } else if (xfer->len > fifo_len) { > + } else if (xfer->len >= fifo_len) { > tx_buf = xfer->tx_buf; > rx_buf = xfer->rx_buf; > origin_len = xfer->len; > - > target_len = xfer->len; > - if (xfer->len > fifo_len) > - xfer->len = fifo_len; > + xfer->len = fifo_len - 1; > } Is this change related to this patch? The rest looks good. Andi
On 19/04/2023 11:45, Jaewon Kim wrote: >>> static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >>> - struct spi_transfer *xfer) >>> + struct spi_transfer *xfer, int use_irq) >>> { >>> void __iomem *regs = sdd->regs; >>> unsigned long val; >>> + unsigned long time; >>> u32 status; >>> int loops; >>> u32 cpy_len; >>> @@ -563,17 +568,24 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >>> int ms; >>> u32 tx_time; >>> >>> - /* sleep during signal transfer time */ >>> - status = readl(regs + S3C64XX_SPI_STATUS); >>> - if (RX_FIFO_LVL(status, sdd) < xfer->len) { >>> - tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; >>> - usleep_range(tx_time / 2, tx_time); >>> - } >> You just added this code. Adding and immediately removing it, suggests >> this should be one patch. >> > This code has been moved, not removed. Move consists of remove and add. Add it in correct place since beginning. Best regards, Krzysztof
On 19/04/2023 13:13, Jaewon Kim wrote: > > On 23. 4. 19. 17:14, Krzysztof Kozlowski wrote: >> On 19/04/2023 08:06, Jaewon Kim wrote: >>> Adds cpu_relax() to prevent long busy-wait. >> How cpu_relax prevents long waiting? > > As I know, cpu_relax() can be converted to yield. This can prevent > excessive use of the CPU in busy-loop. That's ok, you just wrote that it will prevent long waiting, so I assume it will shorten the wait time. > > I'll replace poor sentence like below in v3. > > ("Adds cpu_relax() to allow CPU relaxation in busy-loop") > >>> There is busy-wait loop to check data transfer completion in polling mode. >>> >>> Signed-off-by: Jaewon Kim<jaewon02.kim@samsung.com> >>> --- >>> drivers/spi/spi-s3c64xx.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >>> index 273aa02322d9..886722fb40ea 100644 >>> --- a/drivers/spi/spi-s3c64xx.c >>> +++ b/drivers/spi/spi-s3c64xx.c >>> @@ -568,6 +568,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >>> >>> val = msecs_to_loops(ms); >>> do { >>> + cpu_relax(); >> Shouldn't this be just readl_poll_timeout()? Or the syntax would be too >> complicated? > > I think we can replace this while() loop to readl_poll_timeout(). > > However, we should use 0 value as 'delay_us' parameter. Because delay > can affect throughput. > > > My purpose is add relax to this busy-loop. > > we cannot give relax if we change to readl_poll_timeout(). readl_poll_timeout() will know to do the best. You do not need to add cpu_relax there. Best regards, Krzysztof
On 23. 4. 21. 00:39, Krzysztof Kozlowski wrote: > On 19/04/2023 13:13, Jaewon Kim wrote: >> On 23. 4. 19. 17:14, Krzysztof Kozlowski wrote: >>> On 19/04/2023 08:06, Jaewon Kim wrote: >>>> Adds cpu_relax() to prevent long busy-wait. >>> How cpu_relax prevents long waiting? >> As I know, cpu_relax() can be converted to yield. This can prevent >> excessive use of the CPU in busy-loop. > That's ok, you just wrote that it will prevent long waiting, so I assume > it will shorten the wait time. > >> I'll replace poor sentence like below in v3. >> >> ("Adds cpu_relax() to allow CPU relaxation in busy-loop") >> >>>> There is busy-wait loop to check data transfer completion in polling mode. >>>> >>>> Signed-off-by: Jaewon Kim<jaewon02.kim@samsung.com> >>>> --- >>>> drivers/spi/spi-s3c64xx.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >>>> index 273aa02322d9..886722fb40ea 100644 >>>> --- a/drivers/spi/spi-s3c64xx.c >>>> +++ b/drivers/spi/spi-s3c64xx.c >>>> @@ -568,6 +568,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >>>> >>>> val = msecs_to_loops(ms); >>>> do { >>>> + cpu_relax(); >>> Shouldn't this be just readl_poll_timeout()? Or the syntax would be too >>> complicated? >> I think we can replace this while() loop to readl_poll_timeout(). >> >> However, we should use 0 value as 'delay_us' parameter. Because delay >> can affect throughput. >> >> >> My purpose is add relax to this busy-loop. >> >> we cannot give relax if we change to readl_poll_timeout(). > readl_poll_timeout() will know to do the best. You do not need to add > cpu_relax there. Okay, I will change it to readl_poll_timeout() > > Best regards, > Krzysztof > > Thanks Jaewon Kim
Hi Andi, On 23. 4. 20. 00:56, Andi Shyti wrote: > Hi Jaewon, > >>>> In polling mode, the status register is constantly read to check transfer >>>> completion. It cause excessive CPU usage. >>>> So, it calculates the SPI transfer time and made it sleep. >>>> >>>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >>>> --- >>>> drivers/spi/spi-s3c64xx.c | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >>>> index 886722fb40ea..cf3060b2639b 100644 >>>> --- a/drivers/spi/spi-s3c64xx.c >>>> +++ b/drivers/spi/spi-s3c64xx.c >>>> @@ -561,6 +561,14 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >>>> u32 cpy_len; >>>> u8 *buf; >>>> int ms; >>>> + u32 tx_time; >>>> + >>>> + /* sleep during signal transfer time */ >>>> + status = readl(regs + S3C64XX_SPI_STATUS); >>>> + if (RX_FIFO_LVL(status, sdd) < xfer->len) { >>>> + tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; >>>> + usleep_range(tx_time / 2, tx_time); >>>> + } >>> Did you actually check the delays introduced by it? Is it worth? >> Yes, I already test it. >> >> Throughput was the same, CPU utilization decreased to 30~40% from 100%. >> >> Tested board is ExynosAutov9 SADK. >> >> >>>> >>>> /* millisecs to xfer 'len' bytes @ 'cur_speed' */ >>>> ms = xfer->len * 8 * 1000 / sdd->cur_speed; >>> You have now some code duplication so this could be combined. > you could put the 'if' under the 'ms = ...' and just use ms > without declaring any tx_time. > > Andi The unit of 'tx_time' is 'us'. tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; ms = xfer->len * 8 * 1000 / sdd->cur_speed; I add tx_time to minimize existing code modifications. If we are not using tx_time, we need to change ms to us and change the related code. Thanks Jaewon Kim
Hi Andy, On 23. 4. 20. 01:03, Andi Shyti wrote: > Hi Jaewon, > > On Wed, Apr 19, 2023 at 03:06:39PM +0900, Jaewon Kim wrote: >> Interrupt based pio mode is supported to reduce CPU load. >> If transfer size is larger than 32 byte, it is processed using interrupt. >> >> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 82 ++++++++++++++++++++++++++++++++------- >> 1 file changed, 67 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index cf3060b2639b..ce1afb9a4ed4 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -58,6 +58,8 @@ >> #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17) >> #define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17) >> #define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17) >> +#define S3C64XX_SPI_MODE_RX_RDY_LVL GENMASK(16, 11) >> +#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT 11 >> #define S3C64XX_SPI_MODE_SELF_LOOPBACK (1<<3) >> #define S3C64XX_SPI_MODE_RXDMA_ON (1<<2) >> #define S3C64XX_SPI_MODE_TXDMA_ON (1<<1) >> @@ -114,6 +116,8 @@ >> >> #define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT >> >> +#define S3C64XX_SPI_POLLING_SIZE 32 >> + >> #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t) >> #define is_polling(x) (x->cntrlr_info->polling) >> >> @@ -552,10 +556,11 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, >> } >> >> static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >> - struct spi_transfer *xfer) >> + struct spi_transfer *xfer, int use_irq) > bool use_irq Okay, I will change it to bool. > >> { >> void __iomem *regs = sdd->regs; >> unsigned long val; >> + unsigned long time; > this time is used only in "if (use_irq)" can you move its > declaration under the if? > >> u32 status; >> int loops; >> u32 cpy_len; >> @@ -563,17 +568,24 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd, >> int ms; >> u32 tx_time; >> >> - /* sleep during signal transfer time */ >> - status = readl(regs + S3C64XX_SPI_STATUS); >> - if (RX_FIFO_LVL(status, sdd) < xfer->len) { >> - tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; >> - usleep_range(tx_time / 2, tx_time); >> - } >> - >> /* millisecs to xfer 'len' bytes @ 'cur_speed' */ >> ms = xfer->len * 8 * 1000 / sdd->cur_speed; >> ms += 10; /* some tolerance */ >> >> + if (use_irq) { >> + val = msecs_to_jiffies(ms); >> + time = wait_for_completion_timeout(&sdd->xfer_completion, val); >> + if (!time) >> + return -EIO; >> + } else { >> + /* sleep during signal transfer time */ >> + status = readl(regs + S3C64XX_SPI_STATUS); >> + if (RX_FIFO_LVL(status, sdd) < xfer->len) { >> + tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed; >> + usleep_range(tx_time / 2, tx_time); > yeah... just use 'ms'. As I mentioned in the previous mail, the unit of tx_time is us. > >> + } >> + } >> + >> val = msecs_to_loops(ms); >> do { >> cpu_relax(); >> @@ -737,10 +749,13 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, >> void *rx_buf = NULL; >> int target_len = 0, origin_len = 0; >> int use_dma = 0; >> + int use_irq = 0; >> int status; >> u32 speed; >> u8 bpw; >> unsigned long flags; >> + u32 rdy_lv; >> + u32 val; >> >> reinit_completion(&sdd->xfer_completion); >> >> @@ -761,17 +776,46 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, >> sdd->rx_dma.ch && sdd->tx_dma.ch) { >> use_dma = 1; >> >> - } else if (xfer->len > fifo_len) { >> + } else if (xfer->len >= fifo_len) { >> tx_buf = xfer->tx_buf; >> rx_buf = xfer->rx_buf; >> origin_len = xfer->len; >> - >> target_len = xfer->len; >> - if (xfer->len > fifo_len) >> - xfer->len = fifo_len; >> + xfer->len = fifo_len - 1; >> } > Is this change related to this patch? Yes, it is related to this patch. If data is filled as much as the size of FIFO, underrun/overrun IRQ occurs. In CPU polling mode, it did not occur because the FIFO was read before the IRQ was set. So, I set xfer->len to fifo_len-1. > > The rest looks good. > > Andi Thanks Jaewon Kim