2024-02-07 12:04:52

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 00/17] spi: s3c64xx: straightforward cleanup

The patch set has no dependency although Sam prefers to have this simple
cleanup queued after the gs101 patches from:
https://lore.kernel.org/linux-spi/[email protected]/

Tested with gs101-spi.

Changes in v5:
- don't abuse the Fixes tag, it was wrongly used for:
- explicit header inclusions
- possible negative array index fix, which is just a posibility,
it never happened
- typo fix
- reorder patches, sort headers then explicitly include the missing ones
- new patch: "spi: s3c64xx: explicitly include <linux/types.h>"
- collect R-b tags

v4:
- drop "spi: s3c64xx: use bitfield access macros" patch as there was
no agreement on how the reg fields should be handled.
- don't change the style in s3c64xx_spi_dt_match(), drop just the
unneeded casts
- collect Sam's R-b tags

v3:
- reworked the bitfied access macros patch so that the bit operations
are the same as before the patch. Fix S3C64XX_SPI_PSR_MASK value,
drop S3C64XX_SPI_CS_NSC_CNT_MASK.
- add a new patches to explicitly remove a duplicated definition and to
drop a superfluous bitwise NOT operation.
- collect R-b tags

v2:
https://lore.kernel.org/linux-arm-kernel/[email protected]/

Tudor Ambarus (17):
spi: s3c64xx: sort headers alphabetically
spi: s3c64xx: explicitly include <linux/io.h>
spi: s3c64xx: explicitly include <linux/bits.h>
spi: s3c64xx: explicitly include <linux/types.h>
spi: s3c64xx: avoid possible negative array index
spi: s3c64xx: fix typo, s/configuartion/configuration
spi: s3c64xx: remove unneeded (void *) casts in of_match_table
spi: s3c64xx: remove else after return
spi: s3c64xx: move common code outside if else
spi: s3c64xx: check return code of dmaengine_slave_config()
spi: s3c64xx: propagate the dma_submit_error() error code
spi: s3c64xx: rename prepare_dma() to s3c64xx_prepare_dma()
spi: s3c64xx: return ETIMEDOUT for wait_for_completion_timeout()
spi: s3c64xx: drop blank line between declarations
spi: s3c64xx: downgrade dev_warn to dev_dbg for optional dt props
spi: s3c64xx: remove duplicated definition
spi: s3c64xx: drop a superfluous bitwise NOT operation

drivers/spi/spi-s3c64xx.c | 81 ++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 40 deletions(-)

--
2.43.0.687.g38aa6559b0-goog



2024-02-07 12:05:22

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 02/17] spi: s3c64xx: explicitly include <linux/io.h>

The driver uses readl() but does not include <linux/io.h>.

It is good practice to directly include all headers used, it avoids
implicit dependencies and spurious breakage if someone rearranges
headers and causes the implicit include to vanish.

Include the missing header.

Reviewed-by: Peter Griffin <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
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 1455cbd2fa8d..9882eb0f4bea 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -9,6 +9,7 @@
#include <linux/dmaengine.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_data/spi-s3c64xx.h>
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:05:47

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 05/17] spi: s3c64xx: avoid possible negative array index

The platform id is used as an index into the fifo_lvl_mask array.
Platforms can come with a negative device ID, PLATFORM_DEVID_NONE (-1),
thus we risked a negative array index. Catch such cases and fail to
probe.

Reviewed-by: Sam Protsenko <[email protected]>
Reviewed-by: Peter Griffin <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 0a30d034e288..dfe78ddfa233 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1228,6 +1228,9 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
"Failed to get alias id\n");
sdd->port_id = ret;
} else {
+ if (pdev->id < 0)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Negative platform ID is not allowed\n");
sdd->port_id = pdev->id;
}

--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:06:10

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 08/17] spi: s3c64xx: remove else after return

Else case is not needed after a return, remove it.

Reviewed-by: Andi Shyti <[email protected]>
Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index e3e7d625f3fe..0b42d28d2019 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -410,12 +410,10 @@ static bool s3c64xx_spi_can_dma(struct spi_controller *host,
{
struct s3c64xx_spi_driver_data *sdd = spi_controller_get_devdata(host);

- if (sdd->rx_dma.ch && sdd->tx_dma.ch) {
+ if (sdd->rx_dma.ch && sdd->tx_dma.ch)
return xfer->len > FIFO_DEPTH(sdd);
- } else {
- return false;
- }

+ return false;
}

static void s3c64xx_iowrite8_32_rep(volatile void __iomem *addr,
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:07:08

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 11/17] spi: s3c64xx: propagate the dma_submit_error() error code

DMA submit should just add the dma descriptor to a queue, without firing
it. EIO is misleading and hides what happens in DMA. Propagate the
dma_submit_error() error code, don't overwrite it.

Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 1f7356f6e5d2..ba17c5a04eef 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -324,7 +324,7 @@ static int prepare_dma(struct s3c64xx_spi_dma_data *dma,
ret = dma_submit_error(dma->cookie);
if (ret) {
dev_err(&sdd->pdev->dev, "DMA submission failed");
- return -EIO;
+ return ret;
}

dma_async_issue_pending(dma->ch);
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:08:42

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 12/17] spi: s3c64xx: rename prepare_dma() to s3c64xx_prepare_dma()

Don't monopolize the name. Prepend the driver prefix to the function
name.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index ba17c5a04eef..4cafec877931 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -281,8 +281,8 @@ static void s3c64xx_spi_dmacb(void *data)
spin_unlock_irqrestore(&sdd->lock, flags);
}

-static int prepare_dma(struct s3c64xx_spi_dma_data *dma,
- struct sg_table *sgt)
+static int s3c64xx_prepare_dma(struct s3c64xx_spi_dma_data *dma,
+ struct sg_table *sgt)
{
struct s3c64xx_spi_driver_data *sdd;
struct dma_slave_config config;
@@ -497,7 +497,7 @@ static int s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
chcfg |= S3C64XX_SPI_CH_TXCH_ON;
if (dma_mode) {
modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
- ret = prepare_dma(&sdd->tx_dma, &xfer->tx_sg);
+ ret = s3c64xx_prepare_dma(&sdd->tx_dma, &xfer->tx_sg);
} else {
s3c64xx_iowrite_rep(sdd, xfer);
}
@@ -516,7 +516,7 @@ static int 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);
- ret = prepare_dma(&sdd->rx_dma, &xfer->rx_sg);
+ ret = s3c64xx_prepare_dma(&sdd->rx_dma, &xfer->rx_sg);
}
}

--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:08:48

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 15/17] spi: s3c64xx: downgrade dev_warn to dev_dbg for optional dt props

"samsung,spi-src-clk" and "num-cs" are optional dt properties. Downgrade
the message from warning to debug message.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[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 d257c4f5623e..8ad28ec3e6c1 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1149,14 +1149,14 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
return ERR_PTR(-ENOMEM);

if (of_property_read_u32(dev->of_node, "samsung,spi-src-clk", &temp)) {
- dev_warn(dev, "spi bus clock parent not specified, using clock at index 0 as parent\n");
+ dev_dbg(dev, "spi bus clock parent not specified, using clock at index 0 as parent\n");
sci->src_clk_nr = 0;
} else {
sci->src_clk_nr = temp;
}

if (of_property_read_u32(dev->of_node, "num-cs", &temp)) {
- dev_warn(dev, "number of chip select lines not specified, assuming 1 chip select line\n");
+ dev_dbg(dev, "number of chip select lines not specified, assuming 1 chip select line\n");
sci->num_cs = 1;
} else {
sci->num_cs = temp;
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:09:04

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 06/17] spi: s3c64xx: fix typo, s/configuartion/configuration

Fix typo, s/configuartion/configuration.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index dfe78ddfa233..93d0e55e1249 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -182,7 +182,7 @@ struct s3c64xx_spi_port_config {
* @cur_speed: Current clock speed
* @rx_dma: Local receive DMA data (e.g. chan and direction)
* @tx_dma: Local transmit DMA data (e.g. chan and direction)
- * @port_conf: Local SPI port configuartion data
+ * @port_conf: Local SPI port configuration data
* @port_id: Port identification number
*/
struct s3c64xx_spi_driver_data {
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:11:28

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 09/17] spi: s3c64xx: move common code outside if else

Move common code outside if else to avoid code duplication.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 0b42d28d2019..4ce6cb3b43f6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -294,20 +294,18 @@ static int prepare_dma(struct s3c64xx_spi_dma_data *dma,
if (dma->direction == DMA_DEV_TO_MEM) {
sdd = container_of((void *)dma,
struct s3c64xx_spi_driver_data, rx_dma);
- config.direction = dma->direction;
config.src_addr = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
config.src_addr_width = sdd->cur_bpw / 8;
config.src_maxburst = 1;
- dmaengine_slave_config(dma->ch, &config);
} else {
sdd = container_of((void *)dma,
struct s3c64xx_spi_driver_data, tx_dma);
- config.direction = dma->direction;
config.dst_addr = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
config.dst_addr_width = sdd->cur_bpw / 8;
config.dst_maxburst = 1;
- dmaengine_slave_config(dma->ch, &config);
}
+ config.direction = dma->direction;
+ dmaengine_slave_config(dma->ch, &config);

desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
dma->direction, DMA_PREP_INTERRUPT);
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:11:47

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 03/17] spi: s3c64xx: explicitly include <linux/bits.h>

The driver uses GENMASK() but does not include <linux/bits.h>.

It is good practice to directly include all headers used, it avoids
implicit dependencies and spurious breakage if someone rearranges
headers and causes the implicit include to vanish.

Include the missing header.

Reviewed-by: Peter Griffin <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
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 9882eb0f4bea..b1564a447a6e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -3,6 +3,7 @@
// Copyright (c) 2009 Samsung Electronics Co., Ltd.
// Jaswinder Singh <[email protected]>

+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:11:51

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 07/17] spi: s3c64xx: remove unneeded (void *) casts in of_match_table

of_device_id::data is an opaque pointer. No explicit cast is needed.
Remove unneeded (void *) casts in of_match_table.

Reviewed-by: Andi Shyti <[email protected]>
Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 93d0e55e1249..e3e7d625f3fe 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1568,31 +1568,31 @@ static const struct of_device_id s3c64xx_spi_dt_match[] = {
.data = &gs101_spi_port_config,
},
{ .compatible = "samsung,s3c2443-spi",
- .data = (void *)&s3c2443_spi_port_config,
+ .data = &s3c2443_spi_port_config,
},
{ .compatible = "samsung,s3c6410-spi",
- .data = (void *)&s3c6410_spi_port_config,
+ .data = &s3c6410_spi_port_config,
},
{ .compatible = "samsung,s5pv210-spi",
- .data = (void *)&s5pv210_spi_port_config,
+ .data = &s5pv210_spi_port_config,
},
{ .compatible = "samsung,exynos4210-spi",
- .data = (void *)&exynos4_spi_port_config,
+ .data = &exynos4_spi_port_config,
},
{ .compatible = "samsung,exynos7-spi",
- .data = (void *)&exynos7_spi_port_config,
+ .data = &exynos7_spi_port_config,
},
{ .compatible = "samsung,exynos5433-spi",
- .data = (void *)&exynos5433_spi_port_config,
+ .data = &exynos5433_spi_port_config,
},
{ .compatible = "samsung,exynos850-spi",
- .data = (void *)&exynos850_spi_port_config,
+ .data = &exynos850_spi_port_config,
},
{ .compatible = "samsung,exynosautov9-spi",
- .data = (void *)&exynosautov9_spi_port_config,
+ .data = &exynosautov9_spi_port_config,
},
{ .compatible = "tesla,fsd-spi",
- .data = (void *)&fsd_spi_port_config,
+ .data = &fsd_spi_port_config,
},
{ },
};
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:14:06

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 13/17] spi: s3c64xx: return ETIMEDOUT for wait_for_completion_timeout()

ETIMEDOUT is more specific than EIO, use it for
wait_for_completion_timeout().

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 4cafec877931..bcc00cb5e0d1 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -566,7 +566,7 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,

/*
* If the previous xfer was completed within timeout, then
- * proceed further else return -EIO.
+ * proceed further else return -ETIMEDOUT.
* DmaTx returns after simply writing data in the FIFO,
* w/o waiting for real transmission on the bus to finish.
* DmaRx returns only after Dma read data from FIFO which
@@ -587,7 +587,7 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,

/* If timed out while checking rx/tx status return error */
if (!val)
- return -EIO;
+ return -ETIMEDOUT;

return 0;
}
@@ -617,7 +617,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
if (use_irq) {
val = msecs_to_jiffies(ms);
if (!wait_for_completion_timeout(&sdd->xfer_completion, val))
- return -EIO;
+ return -ETIMEDOUT;
}

val = msecs_to_loops(ms);
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:15:38

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 17/17] spi: s3c64xx: drop a superfluous bitwise NOT operation

val &= ~mask;
val |= mask;

is equivalent to:
val |= mask;

Drop the superfluous bitwise NOT operation.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b618efba0509..6f29dca68491 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1129,7 +1129,6 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)

val = readl(regs + S3C64XX_SPI_MODE_CFG);
val &= ~S3C64XX_SPI_MODE_4BURST;
- val &= ~(S3C64XX_SPI_MAX_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
val |= (S3C64XX_SPI_MAX_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
writel(val, regs + S3C64XX_SPI_MODE_CFG);

--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:16:27

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 01/17] spi: s3c64xx: sort headers alphabetically

Sorting headers alphabetically helps locating duplicates,
and makes it easier to figure out where to insert new headers.

Reviewed-by: Andi Shyti <[email protected]>
Reviewed-by: Peter Griffin <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 7ab3f3c2e9aa..1455cbd2fa8d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -3,19 +3,18 @@
// Copyright (c) 2009 Samsung Electronics Co., Ltd.
// Jaswinder Singh <[email protected]>

-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
#include <linux/clk.h>
+#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_data/spi-s3c64xx.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/spi/spi.h>
-#include <linux/of.h>
-
-#include <linux/platform_data/spi-s3c64xx.h>

#define MAX_SPI_PORTS 16
#define S3C64XX_SPI_QUIRK_CS_AUTO (1 << 1)
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:17:35

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 14/17] spi: s3c64xx: drop blank line between declarations

Drop the blank line and move the logical operation in the body of the
function rather than in initialization list.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index bcc00cb5e0d1..d257c4f5623e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1360,8 +1360,9 @@ static int s3c64xx_spi_suspend(struct device *dev)
{
struct spi_controller *host = dev_get_drvdata(dev);
struct s3c64xx_spi_driver_data *sdd = spi_controller_get_devdata(host);
+ int ret;

- int ret = spi_controller_suspend(host);
+ ret = spi_controller_suspend(host);
if (ret)
return ret;

--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:17:49

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 04/17] spi: s3c64xx: explicitly include <linux/types.h>

The driver uses u32 and relies on an implicit inclusion of
<linux/types.h>.

It is good practice to directly include all headers used, it avoids
implicit dependencies and spurious breakage if someone rearranges
headers and causes the implicit include to vanish.

Include the missing header.

Signed-off-by: Tudor Ambarus <[email protected]>
---
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 b1564a447a6e..0a30d034e288 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -17,6 +17,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/spi/spi.h>
+#include <linux/types.h>

#define MAX_SPI_PORTS 16
#define S3C64XX_SPI_QUIRK_CS_AUTO (1 << 1)
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:18:02

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 16/17] spi: s3c64xx: remove duplicated definition

S3C64XX_SPI_TRAILCNT brings no benefit in terms of name over
S3C64XX_SPI_MAX_TRAILCNT. Remove the duplicated definition.

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 8ad28ec3e6c1..b618efba0509 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -116,8 +116,6 @@
#define S3C64XX_SPI_MAX_TRAILCNT 0x3ff
#define S3C64XX_SPI_TRAILCNT_OFF 19

-#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)
@@ -1132,7 +1130,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
val = readl(regs + S3C64XX_SPI_MODE_CFG);
val &= ~S3C64XX_SPI_MODE_4BURST;
val &= ~(S3C64XX_SPI_MAX_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
- val |= (S3C64XX_SPI_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
+ val |= (S3C64XX_SPI_MAX_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
writel(val, regs + S3C64XX_SPI_MODE_CFG);

s3c64xx_flush_fifo(sdd);
--
2.43.0.687.g38aa6559b0-goog


2024-02-07 12:18:12

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v5 10/17] spi: s3c64xx: check return code of dmaengine_slave_config()

Check the return code of dmaengine_slave_config().

Reviewed-by: Sam Protsenko <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 4ce6cb3b43f6..1f7356f6e5d2 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -305,7 +305,9 @@ static int prepare_dma(struct s3c64xx_spi_dma_data *dma,
config.dst_maxburst = 1;
}
config.direction = dma->direction;
- dmaengine_slave_config(dma->ch, &config);
+ ret = dmaengine_slave_config(dma->ch, &config);
+ if (ret)
+ return ret;

desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
dma->direction, DMA_PREP_INTERRUPT);
--
2.43.0.687.g38aa6559b0-goog


2024-02-08 21:24:02

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v5 00/17] spi: s3c64xx: straightforward cleanup

On Wed, 07 Feb 2024 12:04:14 +0000, Tudor Ambarus wrote:
> The patch set has no dependency although Sam prefers to have this simple
> cleanup queued after the gs101 patches from:
> https://lore.kernel.org/linux-spi/[email protected]/
>
> Tested with gs101-spi.
>
> Changes in v5:
> - don't abuse the Fixes tag, it was wrongly used for:
> - explicit header inclusions
> - possible negative array index fix, which is just a posibility,
> it never happened
> - typo fix
> - reorder patches, sort headers then explicitly include the missing ones
> - new patch: "spi: s3c64xx: explicitly include <linux/types.h>"
> - collect R-b tags
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[01/17] spi: s3c64xx: sort headers alphabetically
commit: a77ce80f63f06d7ae933c332ed77c79136fa69b0
[02/17] spi: s3c64xx: explicitly include <linux/io.h>
commit: 42a9ac378d918176e17592cbe79d8b0606f951e4
[03/17] spi: s3c64xx: explicitly include <linux/bits.h>
commit: 4568fa574fcef3811a8140702979f076ef0f5bc0
[04/17] spi: s3c64xx: explicitly include <linux/types.h>
commit: 7256d6bdd4fe0eac6d4bcd138c3d87f95f79c750
[05/17] spi: s3c64xx: avoid possible negative array index
commit: a336d41bbea51e11e3e4f56bd3877a535c077129
[06/17] spi: s3c64xx: fix typo, s/configuartion/configuration
commit: 97b63f4707046b2ef99d077dd4d333c3acca06ae
[07/17] spi: s3c64xx: remove unneeded (void *) casts in of_match_table
commit: 271f18816b3ba2f75785660e427c16585b7302f2
[08/17] spi: s3c64xx: remove else after return
commit: 9d47e411f4d636519a8d26587928d34cf52c0c1f
[09/17] spi: s3c64xx: move common code outside if else
commit: 5d7f4f4367079992c7a1bb1654ffea87ddc82be8
[10/17] spi: s3c64xx: check return code of dmaengine_slave_config()
commit: e9c49effde70fb4b10d0ad9c94b69fe6314fc608
[11/17] spi: s3c64xx: propagate the dma_submit_error() error code
commit: 60dc8d342e933097eb82db5859edcac9077a6db5
[12/17] spi: s3c64xx: rename prepare_dma() to s3c64xx_prepare_dma()
commit: 4c6452050530b741daf108de0c02cd2299f8f5d1
[13/17] spi: s3c64xx: return ETIMEDOUT for wait_for_completion_timeout()
commit: 1a234accc93191a3a73eb4cc264abb6d79d63430
[14/17] spi: s3c64xx: drop blank line between declarations
commit: 91a9b8e6b63eeb3634e736a4b65ae536c08155b2
[15/17] spi: s3c64xx: downgrade dev_warn to dev_dbg for optional dt props
commit: f186d34071fb2a7db7249b09d5e1796b18b37d7d
[16/17] spi: s3c64xx: remove duplicated definition
commit: eb8096c30ad07e6201830816e398b3ad603cc096
[17/17] spi: s3c64xx: drop a superfluous bitwise NOT operation
commit: acd6c7b1d2765fd30b7d7487aff50dc824db314e

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark