Simple, straightforward patches that were compiled tested.
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 (16):
spi: s3c64xx: explicitly include <linux/io.h>
spi: s3c64xx: explicitly include <linux/bits.h>
spi: s3c64xx: avoid possible negative array index
spi: s3c64xx: fix typo, s/configuartion/configuration
spi: s3c64xx: sort headers alphabetically
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 | 80 +++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 40 deletions(-)
--
2.43.0.594.gd9cf4e227d-goog
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.
Fixes: 1224e29572f6 ("spi: s3c64xx: Fix large transfers with DMA")
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 c1cbc4780a3b..2b5bb7604526 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/init.h>
#include <linux/module.h>
#include <linux/interrupt.h>
--
2.43.0.594.gd9cf4e227d-goog
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.
Fixes: 230d42d422e7 ("spi: Add s3c64xx SPI Controller driver")
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 7f7eb8f742e4..c1cbc4780a3b 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -10,6 +10,7 @@
#include <linux/clk.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
+#include <linux/io.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/spi/spi.h>
--
2.43.0.594.gd9cf4e227d-goog
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.
Fixes: 2b90807549e5 ("spi: s3c64xx: add device tree support")
Reviewed-by: Sam Protsenko <[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 2b5bb7604526..c3176a510643 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1189,6 +1189,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.594.gd9cf4e227d-goog
Sorting headers alphabetically helps locating duplicates,
and makes it easier to figure out where to insert new headers.
Reviewed-by: Andi Shyti <[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 3df4906bba34..ccb700312d64 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -4,20 +4,19 @@
// Jaswinder Singh <[email protected]>
#include <linux/bits.h>
-#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/io.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 12
#define S3C64XX_SPI_QUIRK_CS_AUTO (1 << 1)
--
2.43.0.594.gd9cf4e227d-goog
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]>
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 ccb700312d64..807270ec3c8a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1512,31 +1512,31 @@ static const struct platform_device_id s3c64xx_spi_driver_ids[] = {
static const struct of_device_id s3c64xx_spi_dt_match[] = {
{ .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.594.gd9cf4e227d-goog
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 807270ec3c8a..3139a703f942 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -407,12 +407,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 int s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
--
2.43.0.594.gd9cf4e227d-goog
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 3139a703f942..fcc1f16d3ad1 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -291,20 +291,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.594.gd9cf4e227d-goog
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 fcc1f16d3ad1..43b9792988b6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -302,7 +302,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.594.gd9cf4e227d-goog
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 43b9792988b6..7a37f5d0abd9 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -321,7 +321,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.594.gd9cf4e227d-goog
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 7a37f5d0abd9..a51b5ff27d5a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -278,8 +278,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;
@@ -444,7 +444,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 {
switch (sdd->cur_bpw) {
case 32:
@@ -476,7 +476,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.594.gd9cf4e227d-goog
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 a51b5ff27d5a..fff430b9e849 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -526,7 +526,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
@@ -547,7 +547,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;
}
@@ -577,7 +577,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.594.gd9cf4e227d-goog
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 fff430b9e849..1b84f5fdebce 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1320,8 +1320,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.594.gd9cf4e227d-goog
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 ed0b5cf8fb4d..72c35dbe53b2 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1089,7 +1089,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.594.gd9cf4e227d-goog
Fix typo, s/configuartion/configuration.
Fixes: 6b8d1e4739f4 ("spi: spi-s3c64xx: Add missing entries for structs 's3c64xx_spi_dma_data' and 's3c64xx_spi_dma_data'")
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 c3176a510643..3df4906bba34 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -180,7 +180,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.594.gd9cf4e227d-goog
"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 1b84f5fdebce..a103d9e125b1 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1109,14 +1109,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.594.gd9cf4e227d-goog
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 a103d9e125b1..ed0b5cf8fb4d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -115,8 +115,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)
@@ -1092,7 +1090,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.594.gd9cf4e227d-goog
On Mon, 5 Feb 2024 at 12:45, Tudor Ambarus <[email protected]> wrote:
>
> 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.
>
> Fixes: 230d42d422e7 ("spi: Add s3c64xx SPI Controller driver")
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
Reviewed-by: Peter Griffin <[email protected]>
On Mon, 5 Feb 2024 at 12:45, Tudor Ambarus <[email protected]> wrote:
>
> 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.
>
> Fixes: 1224e29572f6 ("spi: s3c64xx: Fix large transfers with DMA")
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
Reviewed-by: Peter Griffin <[email protected]>
On Mon, 5 Feb 2024 at 12:45, Tudor Ambarus <[email protected]> wrote:
>
> 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.
>
> Fixes: 2b90807549e5 ("spi: s3c64xx: add device tree support")
> Reviewed-by: Sam Protsenko <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
Reviewed-by: Peter Griffin <[email protected]>
On Mon, Feb 5, 2024 at 6:45 AM Tudor Ambarus <[email protected]> wrote:
>
> 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]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
Reviewed-by: Sam Protsenko <[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 ccb700312d64..807270ec3c8a 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -1512,31 +1512,31 @@ static const struct platform_device_id s3c64xx_spi_driver_ids[] = {
>
> static const struct of_device_id s3c64xx_spi_dt_match[] = {
> { .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.594.gd9cf4e227d-goog
>
On Mon, 5 Feb 2024 at 12:45, Tudor Ambarus <[email protected]> wrote:
>
> Sorting headers alphabetically helps locating duplicates,
> and makes it easier to figure out where to insert new headers.
>
> Reviewed-by: Andi Shyti <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
Reviewed-by: Peter Griffin <[email protected]>
Hi, Mark,
Please don't queue this yet, I'll come up with the gs101 patches first,
as Sam suggests. I'll then add this patch set on top of gs101.
ta