2020-10-02 12:27:43

by Łukasz Stelmach

[permalink] [raw]
Subject: [PATCH v3 0/9] Some fixes for spi-s3c64xx

This is a series of fixes created during porting a device driver (these
patches will be released soon too) for an SPI device to the current kernel.

The two most important are

spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath()
spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250

Without them DMA transfers larger than 512 bytes from the SPI controller
would fail.

Łukasz Stelmach (9):
spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and
s3c64xx_enable_datapath()
spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250
spi: spi-s3c64xx: Check return values
spi: spi-s3c64xx: Report more information when errors occur
spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_*
spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data
spi: spi-s3c64xx: Ensure cur_speed holds actual clock value
spi: spi-s3c64xx: Increase transfer timeout
spi: spi-s3c64xx: Turn on interrupts upon resume

drivers/spi/spi-s3c64xx.c | 111 +++++++++++++++++++++++++++-----------
1 file changed, 79 insertions(+), 32 deletions(-)

Changes in v3:
- added Reviewed-by and Suggested-by tags to commit messages
- added information about non-CMU case in the commit message
of (Ensure cur_speed holds actual clock value)

Changes in v2:
- added missing commit descriptions
- added spi: spi-s3c64xx: Ensure cur_speed holds actual clock value
- implemented error propagation in
spi: spi-s3c64xx: Check return values
- rebased onto v5.9-rc1 which contains
spi: spi-s3c64xx: Add missing entries for structs 's3c64xx_spi_dma_data' and 's3c64xx_spi_dma_data'
--
2.26.2


2020-10-02 12:27:46

by Łukasz Stelmach

[permalink] [raw]
Subject: [PATCH v3 3/9] spi: spi-s3c64xx: Check return values

Check return values in prepare_dma() and s3c64xx_spi_config() and
propagate errors upwards.

Fixes: 788437273fa8 ("spi: s3c64xx: move to generic dmaengine API")
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Łukasz Stelmach <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 50 ++++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 4a9ca9a99857..48afd4818558 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -122,6 +122,7 @@

struct s3c64xx_spi_dma_data {
struct dma_chan *ch;
+ dma_cookie_t cookie;
enum dma_transfer_direction direction;
};

@@ -271,12 +272,13 @@ static void s3c64xx_spi_dmacb(void *data)
spin_unlock_irqrestore(&sdd->lock, flags);
}

-static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
+static int prepare_dma(struct s3c64xx_spi_dma_data *dma,
struct sg_table *sgt)
{
struct s3c64xx_spi_driver_data *sdd;
struct dma_slave_config config;
struct dma_async_tx_descriptor *desc;
+ int ret;

memset(&config, 0, sizeof(config));

@@ -300,12 +302,24 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,

desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
dma->direction, DMA_PREP_INTERRUPT);
+ if (!desc) {
+ dev_err(&sdd->pdev->dev, "unable to prepare %s scatterlist",
+ dma->direction == DMA_DEV_TO_MEM ? "rx" : "tx");
+ return -ENOMEM;
+ }

desc->callback = s3c64xx_spi_dmacb;
desc->callback_param = dma;

- dmaengine_submit(desc);
+ dma->cookie = dmaengine_submit(desc);
+ ret = dma_submit_error(dma->cookie);
+ if (ret) {
+ dev_err(&sdd->pdev->dev, "DMA submission failed");
+ return -EIO;
+ }
+
dma_async_issue_pending(dma->ch);
+ return 0;
}

static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
@@ -355,11 +369,12 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
return xfer->len > (FIFO_LVL_MASK(sdd) >> 1) + 1;
}

-static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
+static int s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
struct spi_transfer *xfer, int dma_mode)
{
void __iomem *regs = sdd->regs;
u32 modecfg, chcfg;
+ int ret = 0;

modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON | S3C64XX_SPI_MODE_RXDMA_ON);
@@ -385,7 +400,7 @@ static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
chcfg |= S3C64XX_SPI_CH_TXCH_ON;
if (dma_mode) {
modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
- prepare_dma(&sdd->tx_dma, &xfer->tx_sg);
+ ret = prepare_dma(&sdd->tx_dma, &xfer->tx_sg);
} else {
switch (sdd->cur_bpw) {
case 32:
@@ -417,12 +432,17 @@ static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
| S3C64XX_SPI_PACKET_CNT_EN,
regs + S3C64XX_SPI_PACKET_CNT);
- prepare_dma(&sdd->rx_dma, &xfer->rx_sg);
+ ret = prepare_dma(&sdd->rx_dma, &xfer->rx_sg);
}
}

+ if (ret)
+ return ret;
+
writel(modecfg, regs + S3C64XX_SPI_MODE_CFG);
writel(chcfg, regs + S3C64XX_SPI_CH_CFG);
+
+ return 0;
}

static u32 s3c64xx_spi_wait_for_timeout(struct s3c64xx_spi_driver_data *sdd,
@@ -555,9 +575,10 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
return 0;
}

-static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
+static int s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
{
void __iomem *regs = sdd->regs;
+ int ret;
u32 val;

/* Disable Clock */
@@ -605,7 +626,9 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)

if (sdd->port_conf->clk_from_cmu) {
/* The src_clk clock is divided internally by 2 */
- clk_set_rate(sdd->src_clk, sdd->cur_speed * 2);
+ ret = clk_set_rate(sdd->src_clk, sdd->cur_speed * 2);
+ if (ret)
+ return ret;
} else {
/* Configure Clock */
val = readl(regs + S3C64XX_SPI_CLK_CFG);
@@ -619,6 +642,8 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
val |= S3C64XX_SPI_ENCLK_ENABLE;
writel(val, regs + S3C64XX_SPI_CLK_CFG);
}
+
+ return 0;
}

#define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
@@ -661,7 +686,9 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
sdd->cur_bpw = bpw;
sdd->cur_speed = speed;
sdd->cur_mode = spi->mode;
- s3c64xx_spi_config(sdd);
+ status = s3c64xx_spi_config(sdd);
+ if (status)
+ return status;
}

if (!is_polling(sdd) && (xfer->len > fifo_len) &&
@@ -688,10 +715,15 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
/* Start the signals */
s3c64xx_spi_set_cs(spi, true);

- s3c64xx_enable_datapath(sdd, xfer, use_dma);
+ status = s3c64xx_enable_datapath(sdd, xfer, use_dma);

spin_unlock_irqrestore(&sdd->lock, flags);

+ if (status) {
+ dev_err(&spi->dev, "failed to enable data path for transfer: %d\n", status);
+ break;
+ }
+
if (use_dma)
status = s3c64xx_wait_for_dma(sdd, xfer);
else
--
2.26.2

2020-10-02 12:27:57

by Łukasz Stelmach

[permalink] [raw]
Subject: [PATCH v3 1/9] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath()

Fix issues with DMA transfers bigger than 512 bytes on Exynos3250. Without
the patches such transfers fail to complete. This solution to the problem
is found in the vendor kernel for ARTIK5 boards based on Exynos3250.

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Łukasz Stelmach <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 924b24441789..26c7cb79cd78 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -685,11 +685,11 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
sdd->state &= ~RXBUSY;
sdd->state &= ~TXBUSY;

- s3c64xx_enable_datapath(sdd, xfer, use_dma);
-
/* Start the signals */
s3c64xx_spi_set_cs(spi, true);

+ s3c64xx_enable_datapath(sdd, xfer, use_dma);
+
spin_unlock_irqrestore(&sdd->lock, flags);

if (use_dma)
--
2.26.2