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