2023-11-17 20:14:00

by David Lechner

[permalink] [raw]
Subject: [PATCH 00/14] spi: axi-spi-engine improvements

We are working towards adding support for the offload feature[1] of the
AXI SPI Engine IP core. Before we can do that, we want to make some
general fixes and improvements to the driver. In order to avoid a giant
series with 35+ patches, we are splitting this up into a few smaller
series.

This first series mostly doing some housekeeping:
* Convert device tree bindings to yaml.
* Add a MAINTAINERS entry.
* Clean up probe and remove using devm.
* Separate message state from driver state.
* Add support for cs_off and variable word size.

Once this series is applied, we will follow up with a second series of
general improvements, and then finally a 3rd series that implements the
offload support. The offload support will also involve the IIO
subsystem (a new IIO driver will depend on the new SPI offload feature),
so I'm mentioning this now in case we want to do anything ahead of time
to prepare for that (e.g. putting all of these changes on a separate
branch).

[1]: https://wiki.analog.com/resources/fpga/peripherals/spi_engine/offload

---
David Lechner (14):
dt-bindings: spi: axi-spi-engine: convert to yaml
MAINTAINERS: add entry for AXI SPI Engine
spi: axi-spi-engine: simplify driver data allocation
spi: axi-spi-engine: use devm_spi_alloc_host()
spi: axi-spi-engine: use devm action to reset hw on remove
spi: axi-spi-engine: use devm_request_irq()
spi: axi-spi-engine: use devm_spi_register_controller()
spi: axi-spi-engine: check for valid clock rate
spi: axi-spi-engine: move msg state to new struct
spi: axi-spi-engine: use message_prepare/unprepare
spi: axi-spi-engine: remove completed_id from driver state
spi: axi-spi-engine: remove struct spi_engine::msg
spi: axi-spi-engine: add support for cs_off
spi: axi-spi-engine: add support for any word size

.../devicetree/bindings/spi/adi,axi-spi-engine.txt | 31 --
.../bindings/spi/adi,axi-spi-engine.yaml | 66 ++++
MAINTAINERS | 10 +
drivers/spi/spi-axi-spi-engine.c | 399 +++++++++++++--------
4 files changed, 329 insertions(+), 177 deletions(-)
---
base-commit: 6f9da18171889fae105e1413a825c53fa5aab40c
change-id: 20231117-axi-spi-engine-series-1-7c76311440f9


2023-11-17 20:14:04

by David Lechner

[permalink] [raw]
Subject: [PATCH 01/14] dt-bindings: spi: axi-spi-engine: convert to yaml

This converts the axi-spi-engine binding to yaml.

There are a few minor fixes in the conversion:
* Added maintainers.
* Added descriptions for the clocks.
* Fixed the double "@" in the example.
* Added a comma between the clocks in the example.

Signed-off-by: David Lechner <[email protected]>
---

Note: This work is being done by BayLibre on behalf of Analog Devices Inc.
This is why the maintainers are @analog.com rather than @baylibre.com.

.../devicetree/bindings/spi/adi,axi-spi-engine.txt | 31 ----------
.../bindings/spi/adi,axi-spi-engine.yaml | 66 ++++++++++++++++++++++
2 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.txt b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.txt
deleted file mode 100644
index 8a18d71e6879..000000000000
--- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-Analog Devices AXI SPI Engine controller Device Tree Bindings
-
-Required properties:
-- compatible : Must be "adi,axi-spi-engine-1.00.a""
-- reg : Physical base address and size of the register map.
-- interrupts : Property with a value describing the interrupt
- number.
-- clock-names : List of input clock names - "s_axi_aclk", "spi_clk"
-- clocks : Clock phandles and specifiers (See clock bindings for
- details on clock-names and clocks).
-- #address-cells : Must be <1>
-- #size-cells : Must be <0>
-
-Optional subnodes:
- Subnodes are use to represent the SPI slave devices connected to the SPI
- master. They follow the generic SPI bindings as outlined in spi-bus.txt.
-
-Example:
-
- spi@@44a00000 {
- compatible = "adi,axi-spi-engine-1.00.a";
- reg = <0x44a00000 0x1000>;
- interrupts = <0 56 4>;
- clocks = <&clkc 15 &clkc 15>;
- clock-names = "s_axi_aclk", "spi_clk";
-
- #address-cells = <1>;
- #size-cells = <0>;
-
- /* SPI devices */
- };
diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
new file mode 100644
index 000000000000..d48faa42d025
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/adi,axi-spi-engine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AXI SPI Engine Controller
+
+description: |
+ The AXI SPI Engine controller is part of the SPI Engine framework[1] and
+ allows memory mapped access to the SPI Engine control bus. This allows it
+ to be used as a general purpose software driven SPI controller as well as
+ some optional advanced acceleration and offloading capabilities.
+
+ [1] https://wiki.analog.com/resources/fpga/peripherals/spi_engine
+
+maintainers:
+ - Michael Hennerich <[email protected]>
+ - Nuno Sá <[email protected]>
+
+allOf:
+ - $ref: /schemas/spi/spi-controller.yaml#
+
+properties:
+ compatible:
+ const: adi,axi-spi-engine-1.00.a
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: The AXI interconnect clock.
+ - description: The SPI controller clock.
+
+ clock-names:
+ items:
+ - const: s_axi_aclk
+ - const: spi_clk
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ spi@44a00000 {
+ compatible = "adi,axi-spi-engine-1.00.a";
+ reg = <0x44a00000 0x1000>;
+ interrupts = <0 56 4>;
+ clocks = <&clkc 15>, <&clkc 15>;
+ clock-names = "s_axi_aclk", "spi_clk";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* SPI devices */
+ };

--
2.42.0

2023-11-17 20:14:09

by David Lechner

[permalink] [raw]
Subject: [PATCH 03/14] spi: axi-spi-engine: simplify driver data allocation

This simplifies the private data allocation in the AXI SPI Engine driver
by making use of the feature built into the spi_alloc_host() function
instead of doing it manually.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index b96e55f59d1a..bdf0aa4ceb1d 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -473,15 +473,11 @@ static int spi_engine_probe(struct platform_device *pdev)
if (irq < 0)
return irq;

- spi_engine = devm_kzalloc(&pdev->dev, sizeof(*spi_engine), GFP_KERNEL);
- if (!spi_engine)
- return -ENOMEM;
-
- host = spi_alloc_host(&pdev->dev, 0);
+ host = spi_alloc_host(&pdev->dev, sizeof(*spi_engine));
if (!host)
return -ENOMEM;

- spi_controller_set_devdata(host, spi_engine);
+ spi_engine = spi_controller_get_devdata(host);

spin_lock_init(&spi_engine->lock);


--
2.42.0

2023-11-17 20:14:14

by David Lechner

[permalink] [raw]
Subject: [PATCH 04/14] spi: axi-spi-engine: use devm_spi_alloc_host()

This modifies the AXI SPI Engine driver to use devm_spi_alloc_host()
instead of spi_alloc_host() to simplify the code a bit.

In addition to simplifying the error paths in the probe function, we
can also remove spi_controller_get/put() calls in the remove function
since devm_spi_alloc_host() sets a flag to no longer decrement the
controller reference count in the spi_unregister_controller() function.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index bdf0aa4ceb1d..77c1c115448d 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -473,7 +473,7 @@ static int spi_engine_probe(struct platform_device *pdev)
if (irq < 0)
return irq;

- host = spi_alloc_host(&pdev->dev, sizeof(*spi_engine));
+ host = devm_spi_alloc_host(&pdev->dev, sizeof(*spi_engine));
if (!host)
return -ENOMEM;

@@ -482,22 +482,16 @@ static int spi_engine_probe(struct platform_device *pdev)
spin_lock_init(&spi_engine->lock);

spi_engine->clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
- if (IS_ERR(spi_engine->clk)) {
- ret = PTR_ERR(spi_engine->clk);
- goto err_put_host;
- }
+ if (IS_ERR(spi_engine->clk))
+ return PTR_ERR(spi_engine->clk);

spi_engine->ref_clk = devm_clk_get_enabled(&pdev->dev, "spi_clk");
- if (IS_ERR(spi_engine->ref_clk)) {
- ret = PTR_ERR(spi_engine->ref_clk);
- goto err_put_host;
- }
+ if (IS_ERR(spi_engine->ref_clk))
+ return PTR_ERR(spi_engine->ref_clk);

spi_engine->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(spi_engine->base)) {
- ret = PTR_ERR(spi_engine->base);
- goto err_put_host;
- }
+ if (IS_ERR(spi_engine->base))
+ return PTR_ERR(spi_engine->base);

version = readl(spi_engine->base + SPI_ENGINE_REG_VERSION);
if (SPI_ENGINE_VERSION_MAJOR(version) != 1) {
@@ -505,8 +499,7 @@ static int spi_engine_probe(struct platform_device *pdev)
SPI_ENGINE_VERSION_MAJOR(version),
SPI_ENGINE_VERSION_MINOR(version),
SPI_ENGINE_VERSION_PATCH(version));
- ret = -ENODEV;
- goto err_put_host;
+ return -ENODEV;
}

writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_RESET);
@@ -515,7 +508,7 @@ static int spi_engine_probe(struct platform_device *pdev)

ret = request_irq(irq, spi_engine_irq, 0, pdev->name, host);
if (ret)
- goto err_put_host;
+ return ret;

host->dev.of_node = pdev->dev.of_node;
host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_3WIRE;
@@ -533,14 +526,12 @@ static int spi_engine_probe(struct platform_device *pdev)
return 0;
err_free_irq:
free_irq(irq, host);
-err_put_host:
- spi_controller_put(host);
return ret;
}

static void spi_engine_remove(struct platform_device *pdev)
{
- struct spi_controller *host = spi_controller_get(platform_get_drvdata(pdev));
+ struct spi_controller *host = platform_get_drvdata(pdev);
struct spi_engine *spi_engine = spi_controller_get_devdata(host);
int irq = platform_get_irq(pdev, 0);

@@ -548,8 +539,6 @@ static void spi_engine_remove(struct platform_device *pdev)

free_irq(irq, host);

- spi_controller_put(host);
-
writel_relaxed(0xff, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_INT_ENABLE);
writel_relaxed(0x01, spi_engine->base + SPI_ENGINE_REG_RESET);

--
2.42.0

2023-11-17 20:14:19

by David Lechner

[permalink] [raw]
Subject: [PATCH 05/14] spi: axi-spi-engine: use devm action to reset hw on remove

This moves the reset of the hardware to a devm action in the AXI SPI
Engine driver. This will allow us to use devm on later calls in the
probe function while preserving the order during cleanup.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 77c1c115448d..c18a4b34777e 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -461,6 +461,15 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
return 0;
}

+static void spi_engine_release_hw(void *p)
+{
+ struct spi_engine *spi_engine = p;
+
+ writel_relaxed(0xff, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
+ writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_INT_ENABLE);
+ writel_relaxed(0x01, spi_engine->base + SPI_ENGINE_REG_RESET);
+}
+
static int spi_engine_probe(struct platform_device *pdev)
{
struct spi_engine *spi_engine;
@@ -506,6 +515,11 @@ static int spi_engine_probe(struct platform_device *pdev)
writel_relaxed(0xff, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_INT_ENABLE);

+ ret = devm_add_action_or_reset(&pdev->dev, spi_engine_release_hw,
+ spi_engine);
+ if (ret)
+ return ret;
+
ret = request_irq(irq, spi_engine_irq, 0, pdev->name, host);
if (ret)
return ret;
@@ -532,16 +546,11 @@ static int spi_engine_probe(struct platform_device *pdev)
static void spi_engine_remove(struct platform_device *pdev)
{
struct spi_controller *host = platform_get_drvdata(pdev);
- struct spi_engine *spi_engine = spi_controller_get_devdata(host);
int irq = platform_get_irq(pdev, 0);

spi_unregister_controller(host);

free_irq(irq, host);
-
- writel_relaxed(0xff, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
- writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_INT_ENABLE);
- writel_relaxed(0x01, spi_engine->base + SPI_ENGINE_REG_RESET);
}

static const struct of_device_id spi_engine_match_table[] = {

--
2.42.0

2023-11-17 20:14:23

by David Lechner

[permalink] [raw]
Subject: [PATCH 07/14] spi: axi-spi-engine: use devm_spi_register_controller()

This replaces spi_register_controller() with
devm_spi_register_controller() in the AXI SPI Engine driver. This
saves us from having to call spi_unregister_controller() in the
remove function.

The remove function is also removed since it is no longer needed.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 81d7352d2b8b..819744246952 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -532,7 +532,7 @@ static int spi_engine_probe(struct platform_device *pdev)
host->transfer_one_message = spi_engine_transfer_one_message;
host->num_chipselect = 8;

- ret = spi_register_controller(host);
+ ret = devm_spi_register_controller(&pdev->dev, host);
if (ret)
return ret;

@@ -541,13 +541,6 @@ static int spi_engine_probe(struct platform_device *pdev)
return 0;
}

-static void spi_engine_remove(struct platform_device *pdev)
-{
- struct spi_controller *host = platform_get_drvdata(pdev);
-
- spi_unregister_controller(host);
-}
-
static const struct of_device_id spi_engine_match_table[] = {
{ .compatible = "adi,axi-spi-engine-1.00.a" },
{ },
@@ -556,7 +549,6 @@ MODULE_DEVICE_TABLE(of, spi_engine_match_table);

static struct platform_driver spi_engine_driver = {
.probe = spi_engine_probe,
- .remove_new = spi_engine_remove,
.driver = {
.name = "spi-engine",
.of_match_table = spi_engine_match_table,

--
2.42.0

2023-11-17 20:14:30

by David Lechner

[permalink] [raw]
Subject: [PATCH 10/14] spi: axi-spi-engine: use message_prepare/unprepare

This modifies the AXI SPI Engine driver to make use of the
message_prepare and message_unprepare callbacks. This separates
the concerns of allocating and freeing the message state from the
transfer_one_message callback.

The main benfit of this is so that future callers of
spi_finalize_current_message() will not have to do manual cleanup
of the state.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 46 +++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 745000a9b2c7..210bea23f433 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -412,11 +412,7 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)

if (spi_engine->completed_id == st->sync_id) {
struct spi_message *msg = spi_engine->msg;
- struct spi_engine_message_state *st = msg->state;

- ida_free(&spi_engine->sync_ida, st->sync_id);
- kfree(st->p);
- kfree(st);
msg->status = 0;
msg->actual_length = msg->frame_length;
spi_engine->msg = NULL;
@@ -436,14 +432,12 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
return IRQ_HANDLED;
}

-static int spi_engine_transfer_one_message(struct spi_controller *host,
- struct spi_message *msg)
+static int spi_engine_prepare_message(struct spi_controller *host,
+ struct spi_message *msg)
{
struct spi_engine_program p_dry, *p;
struct spi_engine *spi_engine = spi_controller_get_devdata(host);
struct spi_engine_message_state *st;
- unsigned int int_enable = 0;
- unsigned long flags;
size_t size;
int ret;

@@ -472,15 +466,41 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,

spi_engine_compile_message(spi_engine, msg, false, p);

- spin_lock_irqsave(&spi_engine->lock, flags);
spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(st->sync_id));

- msg->state = st;
- spi_engine->msg = msg;
st->p = p;
-
st->cmd_buf = p->instructions;
st->cmd_length = p->length;
+ msg->state = st;
+
+ return 0;
+}
+
+static int spi_engine_unprepare_message(struct spi_controller *host,
+ struct spi_message *msg)
+{
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ struct spi_engine_message_state *st = msg->state;
+
+ ida_free(&spi_engine->sync_ida, st->sync_id);
+ kfree(st->p);
+ kfree(st);
+
+ return 0;
+}
+
+static int spi_engine_transfer_one_message(struct spi_controller *host,
+ struct spi_message *msg)
+{
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ struct spi_engine_message_state *st = msg->state;
+ unsigned int int_enable = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&spi_engine->lock, flags);
+
+ spi_engine->msg = msg;
+
if (spi_engine_write_cmd_fifo(spi_engine))
int_enable |= SPI_ENGINE_INT_CMD_ALMOST_EMPTY;

@@ -572,6 +592,8 @@ static int spi_engine_probe(struct platform_device *pdev)
host->bits_per_word_mask = SPI_BPW_MASK(8);
host->max_speed_hz = clk_get_rate(spi_engine->ref_clk) / 2;
host->transfer_one_message = spi_engine_transfer_one_message;
+ host->prepare_message = spi_engine_prepare_message;
+ host->unprepare_message = spi_engine_unprepare_message;
host->num_chipselect = 8;

if (host->max_speed_hz == 0)

--
2.42.0

2023-11-17 20:14:33

by David Lechner

[permalink] [raw]
Subject: [PATCH 11/14] spi: axi-spi-engine: remove completed_id from driver state

In the AXI SPI Engine driver, the completed_id field in the driver
state is only used in one function and the value does not need to
persist between function calls. Therefore, it can be removed from the
driver state and made a local variable in the function where it is used.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 210bea23f433..120001dbc4dc 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -115,7 +115,6 @@ struct spi_engine {

struct spi_message *msg;
struct ida sync_ida;
- unsigned int completed_id;

unsigned int int_enable;
};
@@ -380,13 +379,14 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
struct spi_engine *spi_engine = spi_controller_get_devdata(host);
unsigned int disable_int = 0;
unsigned int pending;
+ int completed_id = -1;

pending = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_INT_PENDING);

if (pending & SPI_ENGINE_INT_SYNC) {
writel_relaxed(SPI_ENGINE_INT_SYNC,
spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
- spi_engine->completed_id = readl_relaxed(
+ completed_id = readl_relaxed(
spi_engine->base + SPI_ENGINE_REG_SYNC_ID);
}

@@ -410,7 +410,7 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
if (pending & SPI_ENGINE_INT_SYNC && spi_engine->msg) {
struct spi_engine_message_state *st = spi_engine->msg->state;

- if (spi_engine->completed_id == st->sync_id) {
+ if (completed_id == st->sync_id) {
struct spi_message *msg = spi_engine->msg;

msg->status = 0;

--
2.42.0

2023-11-17 20:14:36

by David Lechner

[permalink] [raw]
Subject: [PATCH 12/14] spi: axi-spi-engine: remove struct spi_engine::msg

In the AXI SPI Engine driver, the struct spi_engine::msg member was used
to keep track of the current message being processed. The SPI core is
already keeping track of this, so we don't need to duplicate the effort.

In most cases, we already have a pointer to the current message, so we
can pass it directly to the functions that need it. In the one case
where we don't have a pointer to the current message, we can get it
from struct spi_controller::cur_msg.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 60 +++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 32 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 120001dbc4dc..c39f478f34a7 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -112,8 +112,6 @@ struct spi_engine {
spinlock_t lock;

void __iomem *base;
-
- struct spi_message *msg;
struct ida sync_ida;

unsigned int int_enable;
@@ -252,10 +250,9 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
return 0;
}

-static void spi_engine_xfer_next(struct spi_engine *spi_engine,
+static void spi_engine_xfer_next(struct spi_message *msg,
struct spi_transfer **_xfer)
{
- struct spi_message *msg = spi_engine->msg;
struct spi_transfer *xfer = *_xfer;

if (!xfer) {
@@ -270,13 +267,13 @@ static void spi_engine_xfer_next(struct spi_engine *spi_engine,
*_xfer = xfer;
}

-static void spi_engine_tx_next(struct spi_engine *spi_engine)
+static void spi_engine_tx_next(struct spi_message *msg)
{
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_engine_message_state *st = msg->state;
struct spi_transfer *xfer = st->tx_xfer;

do {
- spi_engine_xfer_next(spi_engine, &xfer);
+ spi_engine_xfer_next(msg, &xfer);
} while (xfer && !xfer->tx_buf);

st->tx_xfer = xfer;
@@ -288,13 +285,13 @@ static void spi_engine_tx_next(struct spi_engine *spi_engine)
}
}

-static void spi_engine_rx_next(struct spi_engine *spi_engine)
+static void spi_engine_rx_next(struct spi_message *msg)
{
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_engine_message_state *st = msg->state;
struct spi_transfer *xfer = st->rx_xfer;

do {
- spi_engine_xfer_next(spi_engine, &xfer);
+ spi_engine_xfer_next(msg, &xfer);
} while (xfer && !xfer->rx_buf);

st->rx_xfer = xfer;
@@ -306,10 +303,11 @@ static void spi_engine_rx_next(struct spi_engine *spi_engine)
}
}

-static bool spi_engine_write_cmd_fifo(struct spi_engine *spi_engine)
+static bool spi_engine_write_cmd_fifo(struct spi_engine *spi_engine,
+ struct spi_message *msg)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_CMD_FIFO;
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_engine_message_state *st = msg->state;
unsigned int n, m, i;
const uint16_t *buf;

@@ -327,10 +325,11 @@ static bool spi_engine_write_cmd_fifo(struct spi_engine *spi_engine)
return st->cmd_length != 0;
}

-static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
+static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine,
+ struct spi_message *msg)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDO_DATA_FIFO;
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_engine_message_state *st = msg->state;
unsigned int n, m, i;
const uint8_t *buf;

@@ -344,16 +343,17 @@ static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
st->tx_length -= m;
n -= m;
if (st->tx_length == 0)
- spi_engine_tx_next(spi_engine);
+ spi_engine_tx_next(msg);
}

return st->tx_length != 0;
}

-static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
+static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine,
+ struct spi_message *msg)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDI_DATA_FIFO;
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_engine_message_state *st = msg->state;
unsigned int n, m, i;
uint8_t *buf;

@@ -367,7 +367,7 @@ static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
st->rx_length -= m;
n -= m;
if (st->rx_length == 0)
- spi_engine_rx_next(spi_engine);
+ spi_engine_rx_next(msg);
}

return st->rx_length != 0;
@@ -376,6 +376,7 @@ static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
static irqreturn_t spi_engine_irq(int irq, void *devid)
{
struct spi_controller *host = devid;
+ struct spi_message *msg = host->cur_msg;
struct spi_engine *spi_engine = spi_controller_get_devdata(host);
unsigned int disable_int = 0;
unsigned int pending;
@@ -393,29 +394,26 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
spin_lock(&spi_engine->lock);

if (pending & SPI_ENGINE_INT_CMD_ALMOST_EMPTY) {
- if (!spi_engine_write_cmd_fifo(spi_engine))
+ if (!spi_engine_write_cmd_fifo(spi_engine, msg))
disable_int |= SPI_ENGINE_INT_CMD_ALMOST_EMPTY;
}

if (pending & SPI_ENGINE_INT_SDO_ALMOST_EMPTY) {
- if (!spi_engine_write_tx_fifo(spi_engine))
+ if (!spi_engine_write_tx_fifo(spi_engine, msg))
disable_int |= SPI_ENGINE_INT_SDO_ALMOST_EMPTY;
}

if (pending & (SPI_ENGINE_INT_SDI_ALMOST_FULL | SPI_ENGINE_INT_SYNC)) {
- if (!spi_engine_read_rx_fifo(spi_engine))
+ if (!spi_engine_read_rx_fifo(spi_engine, msg))
disable_int |= SPI_ENGINE_INT_SDI_ALMOST_FULL;
}

- if (pending & SPI_ENGINE_INT_SYNC && spi_engine->msg) {
- struct spi_engine_message_state *st = spi_engine->msg->state;
+ if (pending & SPI_ENGINE_INT_SYNC && msg) {
+ struct spi_engine_message_state *st = msg->state;

if (completed_id == st->sync_id) {
- struct spi_message *msg = spi_engine->msg;
-
msg->status = 0;
msg->actual_length = msg->frame_length;
- spi_engine->msg = NULL;
spi_finalize_current_message(host);
disable_int |= SPI_ENGINE_INT_SYNC;
}
@@ -499,16 +497,14 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,

spin_lock_irqsave(&spi_engine->lock, flags);

- spi_engine->msg = msg;
-
- if (spi_engine_write_cmd_fifo(spi_engine))
+ if (spi_engine_write_cmd_fifo(spi_engine, msg))
int_enable |= SPI_ENGINE_INT_CMD_ALMOST_EMPTY;

- spi_engine_tx_next(spi_engine);
- if (spi_engine_write_tx_fifo(spi_engine))
+ spi_engine_tx_next(msg);
+ if (spi_engine_write_tx_fifo(spi_engine, msg))
int_enable |= SPI_ENGINE_INT_SDO_ALMOST_EMPTY;

- spi_engine_rx_next(spi_engine);
+ spi_engine_rx_next(msg);
if (st->rx_length != 0)
int_enable |= SPI_ENGINE_INT_SDI_ALMOST_FULL;


--
2.42.0

2023-11-17 20:14:42

by David Lechner

[permalink] [raw]
Subject: [PATCH 13/14] spi: axi-spi-engine: add support for cs_off

This adds support for the spi_transfer::cs_off flag to the AXI SPI
Engine driver.

The logic is copied from the generic spi_transfer_one_message() in
spi.c.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index c39f478f34a7..1c60e6486ee2 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -216,7 +216,7 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
struct spi_device *spi = msg->spi;
struct spi_transfer *xfer;
int clk_div, new_clk_div;
- bool cs_change = true;
+ bool keep_cs = false;

clk_div = -1;

@@ -224,6 +224,9 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
SPI_ENGINE_CMD_WRITE(SPI_ENGINE_CMD_REG_CONFIG,
spi_engine_get_config(spi)));

+ xfer = list_first_entry(&msg->transfers, struct spi_transfer, transfer_list);
+ spi_engine_gen_cs(p, dry, spi, !xfer->cs_off);
+
list_for_each_entry(xfer, &msg->transfers, transfer_list) {
new_clk_div = spi_engine_get_clk_div(spi_engine, spi, xfer);
if (new_clk_div != clk_div) {
@@ -233,20 +236,28 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
clk_div));
}

- if (cs_change)
- spi_engine_gen_cs(p, dry, spi, true);
-
spi_engine_gen_xfer(p, dry, xfer);
spi_engine_gen_sleep(p, dry, spi_engine, clk_div, xfer);

- cs_change = xfer->cs_change;
- if (list_is_last(&xfer->transfer_list, &msg->transfers))
- cs_change = !cs_change;
-
- if (cs_change)
- spi_engine_gen_cs(p, dry, spi, false);
+ if (xfer->cs_change) {
+ if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
+ keep_cs = true;
+ } else {
+ if (!xfer->cs_off)
+ spi_engine_gen_cs(p, dry, spi, false);
+
+ if (!list_next_entry(xfer, transfer_list)->cs_off)
+ spi_engine_gen_cs(p, dry, spi, true);
+ }
+ } else if (!list_is_last(&xfer->transfer_list, &msg->transfers) &&
+ xfer->cs_off != list_next_entry(xfer, transfer_list)->cs_off) {
+ spi_engine_gen_cs(p, dry, spi, xfer->cs_off);
+ }
}

+ if (!keep_cs)
+ spi_engine_gen_cs(p, dry, spi, false);
+
return 0;
}


--
2.42.0

2023-11-17 20:14:47

by David Lechner

[permalink] [raw]
Subject: [PATCH 14/14] spi: axi-spi-engine: add support for any word size

The AXI SPI Engine IP supports any word size from 1 to 32 bits. This
adds support for this by setting the bits_per_word_mask and emitting
the appropriate instruction to the SPI Engine each time a transfer
requires a new word size.

The functions that transfer tx/rx buffers from/to the SPI Engine
registers (spi_engine_write_{tx,rx}_fifo()) as well as the function that
creates the transfer instruction (spi_engine_gen_xfer()) also have to be
modified to take into account the word size since xfer->len is the
size of the buffers in bytes rather than words.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 84 ++++++++++++++++++++++++++++++++--------
1 file changed, 68 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 1c60e6486ee2..cbca783830ea 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -53,6 +53,7 @@

#define SPI_ENGINE_CMD_REG_CLK_DIV 0x0
#define SPI_ENGINE_CMD_REG_CONFIG 0x1
+#define SPI_ENGINE_CMD_REG_XFER_BITS 0x2

#define SPI_ENGINE_MISC_SYNC 0x0
#define SPI_ENGINE_MISC_SLEEP 0x1
@@ -157,7 +158,14 @@ static unsigned int spi_engine_get_clk_div(struct spi_engine *spi_engine,
static void spi_engine_gen_xfer(struct spi_engine_program *p, bool dry,
struct spi_transfer *xfer)
{
- unsigned int len = xfer->len;
+ unsigned int len;
+
+ if (xfer->bits_per_word <= 8)
+ len = xfer->len;
+ else if (xfer->bits_per_word <= 16)
+ len = xfer->len / 2;
+ else
+ len = xfer->len / 4;

while (len) {
unsigned int n = min(len, 256U);
@@ -217,6 +225,7 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
struct spi_transfer *xfer;
int clk_div, new_clk_div;
bool keep_cs = false;
+ u8 bits_per_word = 0;

clk_div = -1;

@@ -236,6 +245,13 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
clk_div));
}

+ if (bits_per_word != xfer->bits_per_word) {
+ bits_per_word = xfer->bits_per_word;
+ spi_engine_program_add_cmd(p, dry,
+ SPI_ENGINE_CMD_WRITE(SPI_ENGINE_CMD_REG_XFER_BITS,
+ bits_per_word));
+ }
+
spi_engine_gen_xfer(p, dry, xfer);
spi_engine_gen_sleep(p, dry, spi_engine, clk_div, xfer);

@@ -342,16 +358,34 @@ static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine,
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDO_DATA_FIFO;
struct spi_engine_message_state *st = msg->state;
unsigned int n, m, i;
- const uint8_t *buf;

n = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_SDO_FIFO_ROOM);
while (n && st->tx_length) {
- m = min(n, st->tx_length);
- buf = st->tx_buf;
- for (i = 0; i < m; i++)
- writel_relaxed(buf[i], addr);
- st->tx_buf += m;
- st->tx_length -= m;
+ if (st->tx_xfer->bits_per_word <= 8) {
+ const u8 *buf = st->tx_buf;
+
+ m = min(n, st->tx_length);
+ for (i = 0; i < m; i++)
+ writel_relaxed(buf[i], addr);
+ st->tx_buf += m;
+ st->tx_length -= m;
+ } else if (st->tx_xfer->bits_per_word <= 16) {
+ const u16 *buf = (const u16 *)st->tx_buf;
+
+ m = min(n, st->tx_length / 2);
+ for (i = 0; i < m; i++)
+ writel_relaxed(buf[i], addr);
+ st->tx_buf += m * 2;
+ st->tx_length -= m * 2;
+ } else {
+ const u32 *buf = (const u32 *)st->tx_buf;
+
+ m = min(n, st->tx_length / 4);
+ for (i = 0; i < m; i++)
+ writel_relaxed(buf[i], addr);
+ st->tx_buf += m * 4;
+ st->tx_length -= m * 4;
+ }
n -= m;
if (st->tx_length == 0)
spi_engine_tx_next(msg);
@@ -366,16 +400,34 @@ static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine,
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDI_DATA_FIFO;
struct spi_engine_message_state *st = msg->state;
unsigned int n, m, i;
- uint8_t *buf;

n = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_SDI_FIFO_LEVEL);
while (n && st->rx_length) {
- m = min(n, st->rx_length);
- buf = st->rx_buf;
- for (i = 0; i < m; i++)
- buf[i] = readl_relaxed(addr);
- st->rx_buf += m;
- st->rx_length -= m;
+ if (st->rx_xfer->bits_per_word <= 8) {
+ u8 *buf = st->rx_buf;
+
+ m = min(n, st->rx_length);
+ for (i = 0; i < m; i++)
+ buf[i] = readl_relaxed(addr);
+ st->rx_buf += m;
+ st->rx_length -= m;
+ } else if (st->rx_xfer->bits_per_word <= 16) {
+ u16 *buf = (u16 *)st->rx_buf;
+
+ m = min(n, st->rx_length / 2);
+ for (i = 0; i < m; i++)
+ buf[i] = readl_relaxed(addr);
+ st->rx_buf += m * 2;
+ st->rx_length -= m * 2;
+ } else {
+ u32 *buf = (u32 *)st->rx_buf;
+
+ m = min(n, st->rx_length / 4);
+ for (i = 0; i < m; i++)
+ buf[i] = readl_relaxed(addr);
+ st->rx_buf += m * 4;
+ st->rx_length -= m * 4;
+ }
n -= m;
if (st->rx_length == 0)
spi_engine_rx_next(msg);
@@ -596,7 +648,7 @@ static int spi_engine_probe(struct platform_device *pdev)

host->dev.of_node = pdev->dev.of_node;
host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_3WIRE;
- host->bits_per_word_mask = SPI_BPW_MASK(8);
+ host->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
host->max_speed_hz = clk_get_rate(spi_engine->ref_clk) / 2;
host->transfer_one_message = spi_engine_transfer_one_message;
host->prepare_message = spi_engine_prepare_message;

--
2.42.0

2023-11-17 20:14:50

by David Lechner

[permalink] [raw]
Subject: [PATCH 02/14] MAINTAINERS: add entry for AXI SPI Engine

The AXI SPI Engine driver has been in the kernel for many years but has
lacked a proper maintainers entry. This adds a new entry for the driver
and the devicetree bindings.

Signed-off-by: David Lechner <[email protected]>
---

Note: This work is being done by BayLibre on behalf of Analog Devices Inc.
This is why the maintainers are @analog.com rather than @baylibre.com.

MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..a40f61ad5843 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3415,6 +3415,16 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/hwmon/adi,axi-fan-control.yaml
F: drivers/hwmon/axi-fan-control.c

+AXI SPI ENGINE
+M: Michael Hennerich <[email protected]>
+M: Nuno Sá <[email protected]>
+R: David Lechner <[email protected]>
+L: [email protected]
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
+F: drivers/spi/spi-axi-spi-engine.c
+
AXXIA I2C CONTROLLER
M: Krzysztof Adamski <[email protected]>
L: [email protected]

--
2.42.0

2023-11-17 20:14:55

by David Lechner

[permalink] [raw]
Subject: [PATCH 06/14] spi: axi-spi-engine: use devm_request_irq()

This replaces request_irq() with devm_request_irq() in the AXI SPI
Engine driver. This simplifies the error path and removes the need to
call free_irq() in the remove function.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index c18a4b34777e..81d7352d2b8b 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -520,7 +520,8 @@ static int spi_engine_probe(struct platform_device *pdev)
if (ret)
return ret;

- ret = request_irq(irq, spi_engine_irq, 0, pdev->name, host);
+ ret = devm_request_irq(&pdev->dev, irq, spi_engine_irq, 0, pdev->name,
+ host);
if (ret)
return ret;

@@ -533,24 +534,18 @@ static int spi_engine_probe(struct platform_device *pdev)

ret = spi_register_controller(host);
if (ret)
- goto err_free_irq;
+ return ret;

platform_set_drvdata(pdev, host);

return 0;
-err_free_irq:
- free_irq(irq, host);
- return ret;
}

static void spi_engine_remove(struct platform_device *pdev)
{
struct spi_controller *host = platform_get_drvdata(pdev);
- int irq = platform_get_irq(pdev, 0);

spi_unregister_controller(host);
-
- free_irq(irq, host);
}

static const struct of_device_id spi_engine_match_table[] = {

--
2.42.0

2023-11-17 20:15:03

by David Lechner

[permalink] [raw]
Subject: [PATCH 08/14] spi: axi-spi-engine: check for valid clock rate

This adds a check for a valid SCLK rate in the axi-spi-engine driver
during probe. A valid rate is required to get accurate timing for delays
and by not allowing 0 we can avoid divide by zero errors later without
additional checks.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 819744246952..8a6fbb3bb3f1 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -532,6 +532,9 @@ static int spi_engine_probe(struct platform_device *pdev)
host->transfer_one_message = spi_engine_transfer_one_message;
host->num_chipselect = 8;

+ if (host->max_speed_hz == 0)
+ return dev_err_probe(&pdev->dev, -EINVAL, "spi_clk rate is 0");
+
ret = devm_spi_register_controller(&pdev->dev, host);
if (ret)
return ret;

--
2.42.0

2023-11-17 20:15:24

by David Lechner

[permalink] [raw]
Subject: [PATCH 09/14] spi: axi-spi-engine: move msg state to new struct

This moves the message state in the AXI SPI Engine driver to a new
struct spi_engine_msg_state.

Previously, the driver state contained various pointers that pointed
to memory owned by a struct spi_message. However, it did not set any of
these pointers to NULL when a message was completed. This could lead to
use after free bugs.

Example of how this could happen:
1. SPI core calls into spi_engine_transfer_one_message() with msg1.
2. Assume something was misconfigured and spi_engine_tx_next() is not
called enough times in interrupt callbacks for msg1 such that
spi_engine->tx_xfer is never set to NULL before the msg1 completes.
3. SYNC interrupt is received and spi_finalize_current_message() is
called for msg1. spi_engine->msg is set to NULL but no other
message-specific state is reset.
4. Caller that sent msg1 is notified of the completion and frees msg1
and the associated xfers and tx/rx buffers.
4. SPI core calls into spi_engine_transfer_one_message() with msg2.
5. When spi_engine_tx_next() is called for msg2, spi_engine->tx_xfer is
still be pointing to an xfer from msg1, which was already freed.
spi_engine_xfer_next() tries to access xfer->transfer_list of one
of the freed xfers and we get a segfault or undefined behavior.

To avoid issues like this, instead of putting per-message state in the
driver state struct, we can make use of the struct spi_message::state
field to store a pointer to a new struct spi_engine_msg_state. This way,
all of the state that belongs to specific message stays with that
message and we don't have to remember to manually reset all aspects of
the message state when a message is completed. Rather, a new state is
allocated for each message.

Most of the changes are just renames where the state is accessed. One
place where this wasn't straightforward was the sync_id member. This
has been changed to use ida_alloc_range() since we needed to separate
the per-message sync_id from the per-controller next available sync_id.

Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 150 +++++++++++++++++++++++++--------------
1 file changed, 96 insertions(+), 54 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 8a6fbb3bb3f1..745000a9b2c7 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -6,6 +6,7 @@
*/

#include <linux/clk.h>
+#include <linux/idr.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/of.h>
@@ -78,28 +79,42 @@ struct spi_engine_program {
uint16_t instructions[];
};

-struct spi_engine {
- struct clk *clk;
- struct clk *ref_clk;
-
- spinlock_t lock;
-
- void __iomem *base;
-
- struct spi_message *msg;
+/**
+ * struct spi_engine_message_state - SPI engine per-message state
+ */
+struct spi_engine_message_state {
+ /** Instructions for executing this message. */
struct spi_engine_program *p;
+ /** Number of elements in cmd_buf array. */
unsigned cmd_length;
+ /** Array of commands not yet written to CMD FIFO. */
const uint16_t *cmd_buf;
-
+ /** Next xfer with tx_buf not yet fully written to TX FIFO. */
struct spi_transfer *tx_xfer;
+ /** Size of tx_buf in bytes. */
unsigned int tx_length;
+ /** Bytes not yet written to TX FIFO. */
const uint8_t *tx_buf;
-
+ /** Next xfer with rx_buf not yet fully written to RX FIFO. */
struct spi_transfer *rx_xfer;
+ /** Size of tx_buf in bytes. */
unsigned int rx_length;
+ /** Bytes not yet written to the RX FIFO. */
uint8_t *rx_buf;
+ /** ID to correlate SYNC interrupts with this message. */
+ u8 sync_id;
+};
+
+struct spi_engine {
+ struct clk *clk;
+ struct clk *ref_clk;

- unsigned int sync_id;
+ spinlock_t lock;
+
+ void __iomem *base;
+
+ struct spi_message *msg;
+ struct ida sync_ida;
unsigned int completed_id;

unsigned int int_enable;
@@ -258,100 +273,105 @@ static void spi_engine_xfer_next(struct spi_engine *spi_engine,

static void spi_engine_tx_next(struct spi_engine *spi_engine)
{
- struct spi_transfer *xfer = spi_engine->tx_xfer;
+ struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_transfer *xfer = st->tx_xfer;

do {
spi_engine_xfer_next(spi_engine, &xfer);
} while (xfer && !xfer->tx_buf);

- spi_engine->tx_xfer = xfer;
+ st->tx_xfer = xfer;
if (xfer) {
- spi_engine->tx_length = xfer->len;
- spi_engine->tx_buf = xfer->tx_buf;
+ st->tx_length = xfer->len;
+ st->tx_buf = xfer->tx_buf;
} else {
- spi_engine->tx_buf = NULL;
+ st->tx_buf = NULL;
}
}

static void spi_engine_rx_next(struct spi_engine *spi_engine)
{
- struct spi_transfer *xfer = spi_engine->rx_xfer;
+ struct spi_engine_message_state *st = spi_engine->msg->state;
+ struct spi_transfer *xfer = st->rx_xfer;

do {
spi_engine_xfer_next(spi_engine, &xfer);
} while (xfer && !xfer->rx_buf);

- spi_engine->rx_xfer = xfer;
+ st->rx_xfer = xfer;
if (xfer) {
- spi_engine->rx_length = xfer->len;
- spi_engine->rx_buf = xfer->rx_buf;
+ st->rx_length = xfer->len;
+ st->rx_buf = xfer->rx_buf;
} else {
- spi_engine->rx_buf = NULL;
+ st->rx_buf = NULL;
}
}

static bool spi_engine_write_cmd_fifo(struct spi_engine *spi_engine)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_CMD_FIFO;
+ struct spi_engine_message_state *st = spi_engine->msg->state;
unsigned int n, m, i;
const uint16_t *buf;

n = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_CMD_FIFO_ROOM);
- while (n && spi_engine->cmd_length) {
- m = min(n, spi_engine->cmd_length);
- buf = spi_engine->cmd_buf;
+ while (n && st->cmd_length) {
+ m = min(n, st->cmd_length);
+ buf = st->cmd_buf;
for (i = 0; i < m; i++)
writel_relaxed(buf[i], addr);
- spi_engine->cmd_buf += m;
- spi_engine->cmd_length -= m;
+ st->cmd_buf += m;
+ st->cmd_length -= m;
n -= m;
}

- return spi_engine->cmd_length != 0;
+ return st->cmd_length != 0;
}

static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDO_DATA_FIFO;
+ struct spi_engine_message_state *st = spi_engine->msg->state;
unsigned int n, m, i;
const uint8_t *buf;

n = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_SDO_FIFO_ROOM);
- while (n && spi_engine->tx_length) {
- m = min(n, spi_engine->tx_length);
- buf = spi_engine->tx_buf;
+ while (n && st->tx_length) {
+ m = min(n, st->tx_length);
+ buf = st->tx_buf;
for (i = 0; i < m; i++)
writel_relaxed(buf[i], addr);
- spi_engine->tx_buf += m;
- spi_engine->tx_length -= m;
+ st->tx_buf += m;
+ st->tx_length -= m;
n -= m;
- if (spi_engine->tx_length == 0)
+ if (st->tx_length == 0)
spi_engine_tx_next(spi_engine);
}

- return spi_engine->tx_length != 0;
+ return st->tx_length != 0;
}

static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDI_DATA_FIFO;
+ struct spi_engine_message_state *st = spi_engine->msg->state;
unsigned int n, m, i;
uint8_t *buf;

n = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_SDI_FIFO_LEVEL);
- while (n && spi_engine->rx_length) {
- m = min(n, spi_engine->rx_length);
- buf = spi_engine->rx_buf;
+ while (n && st->rx_length) {
+ m = min(n, st->rx_length);
+ buf = st->rx_buf;
for (i = 0; i < m; i++)
buf[i] = readl_relaxed(addr);
- spi_engine->rx_buf += m;
- spi_engine->rx_length -= m;
+ st->rx_buf += m;
+ st->rx_length -= m;
n -= m;
- if (spi_engine->rx_length == 0)
+ if (st->rx_length == 0)
spi_engine_rx_next(spi_engine);
}

- return spi_engine->rx_length != 0;
+ return st->rx_length != 0;
}

static irqreturn_t spi_engine_irq(int irq, void *devid)
@@ -387,12 +407,16 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
disable_int |= SPI_ENGINE_INT_SDI_ALMOST_FULL;
}

- if (pending & SPI_ENGINE_INT_SYNC) {
- if (spi_engine->msg &&
- spi_engine->completed_id == spi_engine->sync_id) {
+ if (pending & SPI_ENGINE_INT_SYNC && spi_engine->msg) {
+ struct spi_engine_message_state *st = spi_engine->msg->state;
+
+ if (spi_engine->completed_id == st->sync_id) {
struct spi_message *msg = spi_engine->msg;
+ struct spi_engine_message_state *st = msg->state;

- kfree(spi_engine->p);
+ ida_free(&spi_engine->sync_ida, st->sync_id);
+ kfree(st->p);
+ kfree(st);
msg->status = 0;
msg->actual_length = msg->frame_length;
spi_engine->msg = NULL;
@@ -417,29 +441,46 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
{
struct spi_engine_program p_dry, *p;
struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ struct spi_engine_message_state *st;
unsigned int int_enable = 0;
unsigned long flags;
size_t size;
+ int ret;
+
+ st = kzalloc(sizeof(*st), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;

p_dry.length = 0;
spi_engine_compile_message(spi_engine, msg, true, &p_dry);

size = sizeof(*p->instructions) * (p_dry.length + 1);
p = kzalloc(sizeof(*p) + size, GFP_KERNEL);
- if (!p)
+ if (!p) {
+ kfree(st);
return -ENOMEM;
+ }
+
+ ret = ida_alloc_range(&spi_engine->sync_ida, 0, U8_MAX, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(p);
+ kfree(st);
+ return ret;
+ }
+
+ st->sync_id = ret;
+
spi_engine_compile_message(spi_engine, msg, false, p);

spin_lock_irqsave(&spi_engine->lock, flags);
- spi_engine->sync_id = (spi_engine->sync_id + 1) & 0xff;
- spi_engine_program_add_cmd(p, false,
- SPI_ENGINE_CMD_SYNC(spi_engine->sync_id));
+ spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(st->sync_id));

+ msg->state = st;
spi_engine->msg = msg;
- spi_engine->p = p;
+ st->p = p;

- spi_engine->cmd_buf = p->instructions;
- spi_engine->cmd_length = p->length;
+ st->cmd_buf = p->instructions;
+ st->cmd_length = p->length;
if (spi_engine_write_cmd_fifo(spi_engine))
int_enable |= SPI_ENGINE_INT_CMD_ALMOST_EMPTY;

@@ -448,7 +489,7 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
int_enable |= SPI_ENGINE_INT_SDO_ALMOST_EMPTY;

spi_engine_rx_next(spi_engine);
- if (spi_engine->rx_length != 0)
+ if (st->rx_length != 0)
int_enable |= SPI_ENGINE_INT_SDI_ALMOST_FULL;

int_enable |= SPI_ENGINE_INT_SYNC;
@@ -489,6 +530,7 @@ static int spi_engine_probe(struct platform_device *pdev)
spi_engine = spi_controller_get_devdata(host);

spin_lock_init(&spi_engine->lock);
+ ida_init(&spi_engine->sync_ida);

spi_engine->clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
if (IS_ERR(spi_engine->clk))

--
2.42.0

2023-11-17 20:46:17

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH 02/14] MAINTAINERS: add entry for AXI SPI Engine

On Fri, Nov 17, 2023 at 2:13 PM David Lechner <[email protected]> wrote:
>
> The AXI SPI Engine driver has been in the kernel for many years but has
> lacked a proper maintainers entry. This adds a new entry for the driver
> and the devicetree bindings.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
>
> Note: This work is being done by BayLibre on behalf of Analog Devices Inc.
> This is why the maintainers are @analog.com rather than @baylibre.com.
>
> MAINTAINERS | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 97f51d5ec1cf..a40f61ad5843 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3415,6 +3415,16 @@ W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/hwmon/adi,axi-fan-control.yaml
> F: drivers/hwmon/axi-fan-control.c
>
> +AXI SPI ENGINE
> +M: Michael Hennerich <[email protected]>
> +M: Nuno Sá <[email protected]>
> +R: David Lechner <[email protected]>
> +L: [email protected]
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> +F: drivers/spi/spi-axi-spi-engine.c
> +
> AXXIA I2C CONTROLLER
> M: Krzysztof Adamski <[email protected]>
> L: [email protected]
>
> --
> 2.42.0
>

Hi Lars,

As the original author, do you have an interested in being included in
this list?

2023-11-18 03:57:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 09/14] spi: axi-spi-engine: move msg state to new struct

Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6f9da18171889fae105e1413a825c53fa5aab40c]

url: https://github.com/intel-lab-lkp/linux/commits/David-Lechner/dt-bindings-spi-axi-spi-engine-convert-to-yaml/20231118-041730
base: 6f9da18171889fae105e1413a825c53fa5aab40c
patch link: https://lore.kernel.org/r/20231117-axi-spi-engine-series-1-v1-9-cc59db999b87%40baylibre.com
patch subject: [PATCH 09/14] spi: axi-spi-engine: move msg state to new struct
config: alpha-randconfig-r081-20231118 (https://download.01.org/0day-ci/archive/20231118/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231118/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'p' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'cmd_length' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'cmd_buf' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'tx_xfer' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'tx_length' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'tx_buf' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'rx_xfer' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'rx_length' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'rx_buf' not described in 'spi_engine_message_state'
>> drivers/spi/spi-axi-spi-engine.c:106: warning: Function parameter or member 'sync_id' not described in 'spi_engine_message_state'


vim +106 drivers/spi/spi-axi-spi-engine.c

81
82 /**
83 * struct spi_engine_message_state - SPI engine per-message state
84 */
85 struct spi_engine_message_state {
86 /** Instructions for executing this message. */
87 struct spi_engine_program *p;
88 /** Number of elements in cmd_buf array. */
89 unsigned cmd_length;
90 /** Array of commands not yet written to CMD FIFO. */
91 const uint16_t *cmd_buf;
92 /** Next xfer with tx_buf not yet fully written to TX FIFO. */
93 struct spi_transfer *tx_xfer;
94 /** Size of tx_buf in bytes. */
95 unsigned int tx_length;
96 /** Bytes not yet written to TX FIFO. */
97 const uint8_t *tx_buf;
98 /** Next xfer with rx_buf not yet fully written to RX FIFO. */
99 struct spi_transfer *rx_xfer;
100 /** Size of tx_buf in bytes. */
101 unsigned int rx_length;
102 /** Bytes not yet written to the RX FIFO. */
103 uint8_t *rx_buf;
104 /** ID to correlate SYNC interrupts with this message. */
105 u8 sync_id;
> 106 };
107

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-19 16:18:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 01/14] dt-bindings: spi: axi-spi-engine: convert to yaml


On Fri, 17 Nov 2023 14:12:52 -0600, David Lechner wrote:
> This converts the axi-spi-engine binding to yaml.
>
> There are a few minor fixes in the conversion:
> * Added maintainers.
> * Added descriptions for the clocks.
> * Fixed the double "@" in the example.
> * Added a comma between the clocks in the example.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
>
> Note: This work is being done by BayLibre on behalf of Analog Devices Inc.
> This is why the maintainers are @analog.com rather than @baylibre.com.
>
> .../devicetree/bindings/spi/adi,axi-spi-engine.txt | 31 ----------
> .../bindings/spi/adi,axi-spi-engine.yaml | 66 ++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 31 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>

2023-11-19 19:27:01

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 02/14] MAINTAINERS: add entry for AXI SPI Engine

On 11/17/23 12:44, David Lechner wrote:
> On Fri, Nov 17, 2023 at 2:13 PM David Lechner <[email protected]> wrote:
>> The AXI SPI Engine driver has been in the kernel for many years but has
>> lacked a proper maintainers entry. This adds a new entry for the driver
>> and the devicetree bindings.
>>
>> Signed-off-by: David Lechner <[email protected]>
>> ---
>>
>> Note: This work is being done by BayLibre on behalf of Analog Devices Inc.
>> This is why the maintainers are @analog.com rather than @baylibre.com.
>>
>> MAINTAINERS | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 97f51d5ec1cf..a40f61ad5843 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3415,6 +3415,16 @@ W: https://ez.analog.com/linux-software-drivers
>> F: Documentation/devicetree/bindings/hwmon/adi,axi-fan-control.yaml
>> F: drivers/hwmon/axi-fan-control.c
>>
>> +AXI SPI ENGINE
>> +M: Michael Hennerich <[email protected]>
>> +M: Nuno Sá <[email protected]>
>> +R: David Lechner <[email protected]>
>> +L: [email protected]
>> +S: Supported
>> +W: https://ez.analog.com/linux-software-drivers
>> +F: Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
>> +F: drivers/spi/spi-axi-spi-engine.c
>> +
>> AXXIA I2C CONTROLLER
>> M: Krzysztof Adamski <[email protected]>
>> L: [email protected]
>>
>> --
>> 2.42.0
>>
> Hi Lars,
>
> As the original author, do you have an interested in being included in
> this list?

No :)

2023-11-20 18:26:08

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 00/14] spi: axi-spi-engine improvements

On Fri, 17 Nov 2023 14:12:51 -0600, David Lechner wrote:
> We are working towards adding support for the offload feature[1] of the
> AXI SPI Engine IP core. Before we can do that, we want to make some
> general fixes and improvements to the driver. In order to avoid a giant
> series with 35+ patches, we are splitting this up into a few smaller
> series.
>
> This first series mostly doing some housekeeping:
> * Convert device tree bindings to yaml.
> * Add a MAINTAINERS entry.
> * Clean up probe and remove using devm.
> * Separate message state from driver state.
> * Add support for cs_off and variable word size.
>
> [...]

Applied to

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

Thanks!

[01/14] dt-bindings: spi: axi-spi-engine: convert to yaml
commit: 252eafe11ffc032579a56c7a29faa8431785a91e
[02/14] MAINTAINERS: add entry for AXI SPI Engine
commit: 68539d1803476b4ecd403c126aa74b9f25b45f2b
[03/14] spi: axi-spi-engine: simplify driver data allocation
commit: 9e4ce5220eedea2cc440f3961dec1b5122e815b2
[04/14] spi: axi-spi-engine: use devm_spi_alloc_host()
commit: e12cd96e8e93044646fdf4b2c9a1de62cfa01e7c
[05/14] spi: axi-spi-engine: use devm action to reset hw on remove
commit: e094de13ae78035c5642d5dfc65b07301765eebc
[06/14] spi: axi-spi-engine: use devm_request_irq()
commit: 076f32d5db73f16c95b38149f9168210cf267b33
[07/14] spi: axi-spi-engine: use devm_spi_register_controller()
commit: e16e71e3f3c4b73b20f8c79f7ce8465542a337e9
[08/14] spi: axi-spi-engine: check for valid clock rate
commit: e6d5eb85e84aeace5e231b951ece86b20df9f63a
[09/14] spi: axi-spi-engine: move msg state to new struct
commit: 7f970ecb77b6759d37ee743fc36fc0daba960e75
[10/14] spi: axi-spi-engine: use message_prepare/unprepare
commit: 0c74de5c6853b0e83413ad237867a37ba30ef3f9
[11/14] spi: axi-spi-engine: remove completed_id from driver state
commit: 4a074ddeb90f5e81738b401643651b2dea257f57
[12/14] spi: axi-spi-engine: remove struct spi_engine::msg
commit: 4e991445478c6404a6846928093837249c52694a
[13/14] spi: axi-spi-engine: add support for cs_off
commit: 145bb2aedb9f78f290c2b5503b553894a6ec53fe
[14/14] spi: axi-spi-engine: add support for any word size
commit: d861b417e1893a46c63cef2cb46d3587da1e5b15

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