Re: [PATCH v2 4/4] spi: s3c64xx: support interrupt based pio mode

From: Andi Shyti
Date: Wed Apr 19 2023 - 12:03:31 EST


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

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

Attachment: signature.asc
Description: PGP signature