2024-02-09 13:46:42

by Théo Lebrun

[permalink] [raw]
Subject: [PATCH 0/4] spi: cadence-quadspi: correct chip-select logic

Hi,

Here are three independent patches that relate to the handling of
chip-select and the number of those in the spi-cadence-quadspi.c
driver.

- First one is about checking each flash node reg (ie CS) against the
->num_chipselect value instead of the hardcoded max constant. That
means it checks against the num-cs DT prop if it existed. Previously
num-cs==1 with 2 flash nodes would have lead to no error,
a ->num_chipselect==1 and 2 flashes.

- Second, we lower the max CS constant from 16 to 4. The hardware only
supports 4 anyway, and that makes for less memory used. This got
discovered on v6.8-rc2 when the SPI subsystem imposed a max CS of 4.
The change got reverted later.

- Lastly, we adjust the ->num_chipselect value reported to the actual
number of chip-selects. Previously, it reported either the num-cs DT
prop or the max value (if no num-cs was provided).

There is also a small fix to move to modern names and avoid using the
legacy compatibility layer (slave, etc).

Thanks,
Théo

Signed-off-by: Théo Lebrun <[email protected]>
---
Théo Lebrun (4):
spi: cadence-qspi: assert each subnode flash CS is valid
spi: cadence-qspi: set maximum chip-select to 4
spi: cadence-qspi: report correct number of chip-select
spi: cadence-qspi: switch from legacy names to modern ones

drivers/spi/spi-cadence-quadspi.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
---
base-commit: 19b50f80b3a4865bd477aa5c026dd234d39a50d2
change-id: 20240209-cdns-qspi-cs-621bfe7f327f

Best regards,
--
Théo Lebrun <[email protected]>



2024-02-09 13:46:54

by Théo Lebrun

[permalink] [raw]
Subject: [PATCH 3/4] spi: cadence-qspi: report correct number of chip-select

Set the ->num_chipselect field in struct cqspi_st and struct
spi_controller to the current number of chip-select. The value is
dependent on declared flashes in devicetree.

Previously, the num-cs property from devicetree or the maximum value was
being reported.

Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index e9e3abd2142c..895c950e7fd6 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1621,6 +1621,7 @@ static const struct spi_controller_mem_caps cqspi_mem_caps = {

static int cqspi_setup_flash(struct cqspi_st *cqspi)
{
+ unsigned int max_cs = cqspi->num_chipselect - 1;
struct platform_device *pdev = cqspi->pdev;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
@@ -1641,6 +1642,8 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi)
dev_err(dev, "Chip select %d out of range.\n", cs);
of_node_put(np);
return -EINVAL;
+ } else if (cs < max_cs) {
+ max_cs = cs;
}

f_pdata = &cqspi->f_pdata[cs];
@@ -1654,6 +1657,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi)
}
}

+ cqspi->num_chipselect = max_cs + 1;
return 0;
}

@@ -1865,14 +1869,14 @@ static int cqspi_probe(struct platform_device *pdev)
cqspi->current_cs = -1;
cqspi->sclk = 0;

- host->num_chipselect = cqspi->num_chipselect;
-
ret = cqspi_setup_flash(cqspi);
if (ret) {
dev_err(dev, "failed to setup flash parameters %d\n", ret);
goto probe_setup_failed;
}

+ host->num_chipselect = cqspi->num_chipselect;
+
if (cqspi->use_direct_mode) {
ret = cqspi_request_mmap_dma(cqspi);
if (ret == -EPROBE_DEFER)

--
2.43.0


2024-02-09 13:47:11

by Théo Lebrun

[permalink] [raw]
Subject: [PATCH 2/4] spi: cadence-qspi: set maximum chip-select to 4

Change the maximum chip-select count in cadence-qspi to 4 instead of 16.
The value gets used as default ->num_chipselect when the num-cs DT
property isn't received from devicetree. It also determines the
cqspi->f_pdata array size.

Hardware only supports values up to 4; see cqspi_chipselect() that sets
CS using a one-bit-per-CS 4-bit register field.

Add a static_assert() call as a defensive measure to ensure we stay
under the SPI subsystem limit. It got set to 4 when introduced in
4d8ff6b0991d ("spi: Add multi-cs memories support in SPI core") and
later increased to 16 in 2f8c7c3715f2 ("spi: Raise limit on number of
chip selects").

Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 7ba4d5d16fd2..e9e3abd2142c 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -31,7 +31,9 @@
#include <linux/timer.h>

#define CQSPI_NAME "cadence-qspi"
-#define CQSPI_MAX_CHIPSELECT 16
+#define CQSPI_MAX_CHIPSELECT 4
+
+static_assert(CQSPI_MAX_CHIPSELECT <= SPI_CS_CNT_MAX);

/* Quirks */
#define CQSPI_NEEDS_WR_DELAY BIT(0)

--
2.43.0


2024-02-09 13:47:27

by Théo Lebrun

[permalink] [raw]
Subject: [PATCH 4/4] spi: cadence-qspi: switch from legacy names to modern ones

Both spi_master_get_devdata() and the ->master field in struct
spi_device are part of the compatibility layer provided by
<linux/spi/spi.h>. Switch away from them.

Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 895c950e7fd6..7ae3b2329089 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1412,7 +1412,7 @@ static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
{
int ret;
- struct cqspi_st *cqspi = spi_master_get_devdata(mem->spi->master);
+ struct cqspi_st *cqspi = spi_controller_get_devdata(mem->spi->controller);
struct device *dev = &cqspi->pdev->dev;

ret = pm_runtime_resume_and_get(dev);

--
2.43.0


2024-02-21 18:50:11

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/4] spi: cadence-quadspi: correct chip-select logic

On Fri, 09 Feb 2024 14:45:29 +0100, Théo Lebrun wrote:
> Here are three independent patches that relate to the handling of
> chip-select and the number of those in the spi-cadence-quadspi.c
> driver.
>
> - First one is about checking each flash node reg (ie CS) against the
> ->num_chipselect value instead of the hardcoded max constant. That
> means it checks against the num-cs DT prop if it existed. Previously
> num-cs==1 with 2 flash nodes would have lead to no error,
> a ->num_chipselect==1 and 2 flashes.
>
> [...]

Applied to

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

Thanks!

[1/4] spi: cadence-qspi: assert each subnode flash CS is valid
commit: 0d62c64a8e48438545dcef7e5d2f4839ff5cfe4c
[2/4] spi: cadence-qspi: set maximum chip-select to 4
commit: 7cc3522aedb5f4360c4502b2e89b279b7aa94ceb
[3/4] spi: cadence-qspi: report correct number of chip-select
commit: 0f3841a5e1152eca1a58cfbd9ceb6d311aa7e647
[4/4] spi: cadence-qspi: switch from legacy names to modern ones
(no commit info)

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