2023-04-28 12:17:41

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 0/7] spi: stm32: add spi slave mode

STM32 SPI can operate in slave mode.
This series add this functionnality in spi-stm32 driver.

The device-tree property st,spi-slave-underrun can configure STM32 SPI
reaction to an underrun.

Alain Volmat (3):
spi: stm32: renaming of spi_master into spi_controller
spi: stm32: use dmaengine_terminate_{a}sync instead of _all
dt-bindings: spi: stm32: add bindings regarding stm32h7 spi slave

Valentin Caron (4):
dt-bindings: spi: stm32: add address-cells and size-cells into yaml
spi: stm32: introduction of stm32h7 SPI slave support
dt-bindings: spi: stm32: add stm32h7 st,spi-slave-underrun property
spi: stm32: add support for stm32h7 SPI slave underrun detection

.../devicetree/bindings/spi/st,stm32-spi.yaml | 19 +
MAINTAINERS | 1 +
drivers/spi/Kconfig | 1 +
drivers/spi/spi-stm32.c | 386 ++++++++++++------
include/dt-bindings/spi/spi-stm32.h | 15 +
5 files changed, 307 insertions(+), 115 deletions(-)
create mode 100644 include/dt-bindings/spi/spi-stm32.h

--
2.25.1


2023-04-28 12:18:18

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 6/7] dt-bindings: spi: stm32: add stm32h7 st,spi-slave-underrun property

This property is used to enable and configure stm32h7 SPI controller to
handle underrun that could appear in slave mode.

Signed-off-by: Valentin Caron <[email protected]>
---
.../devicetree/bindings/spi/st,stm32-spi.yaml | 8 ++++++++
MAINTAINERS | 1 +
include/dt-bindings/spi/spi-stm32.h | 15 +++++++++++++++
3 files changed, 24 insertions(+)
create mode 100644 include/dt-bindings/spi/spi-stm32.h

diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
index 1d26fa2658c5..e946ea71a247 100644
--- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
@@ -28,6 +28,7 @@ allOf:
properties:
st,spi-midi-ns: false
spi-slave: false
+ st,spi-slave-underrun: false

properties:
"#address-cells": true
@@ -70,6 +71,13 @@ properties:
In case of spi-slave defined, if <0>, indicate that SS should be
detected via the dedicated HW pin

+ st,spi-slave-underrun:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ First parameter enables and selects slave underrun reaction.
+ Refer to "dt-bindings/spi/spi-stm32.h" for the supported values.
+ Second parameter is the pattern in case of SPI_SEND_PATTERN mode.
+
patternProperties:
"^[a-zA-Z][a-zA-Z0-9,+\\-._]{0,63}@[0-9a-f]+$":
type: object
diff --git a/MAINTAINERS b/MAINTAINERS
index 007a9cdb9cc8..9dcf861834fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19790,6 +19790,7 @@ M: Alain Volmat <[email protected]>
L: [email protected]
S: Maintained
F: drivers/spi/spi-stm32.c
+F: include/dt-bindings/spi/spi-stm32.h

ST STPDDC60 DRIVER
M: Daniel Nilsson <[email protected]>
diff --git a/include/dt-bindings/spi/spi-stm32.h b/include/dt-bindings/spi/spi-stm32.h
new file mode 100644
index 000000000000..260109fd1631
--- /dev/null
+++ b/include/dt-bindings/spi/spi-stm32.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-3-Clause) */
+/*
+ * This header provides constants for STM32_SPI bindings.
+ */
+
+#ifndef _DT_BINDINGS_SPI_SPI_STM32_H
+#define _DT_BINDINGS_SPI_SPI_STM32_H
+
+/* st,spi-slave-underrun first parameter */
+#define SPI_NO_ACTION 0
+#define SPI_SEND_PATTERN 1
+#define SPI_REPEAT_LAST_RECEIVED_DATA 2
+#define SPI_REPEAT_LAST_TRANSMITTED_DATA 3
+
+#endif
--
2.25.1

2023-04-28 12:18:24

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 1/7] dt-bindings: spi: stm32: add address-cells and size-cells into yaml

Theses properties need to be described to satisfy dtbs_check.

Signed-off-by: Valentin Caron <[email protected]>
---
Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
index 9ca1a843c820..c599eb359d56 100644
--- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
@@ -29,6 +29,9 @@ allOf:
st,spi-midi-ns: false

properties:
+ "#address-cells": true
+ "#size-cells": true
+
compatible:
enum:
- st,stm32f4-spi
--
2.25.1

2023-04-28 12:18:31

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 3/7] spi: stm32: use dmaengine_terminate_{a}sync instead of _all

From: Alain Volmat <[email protected]>

Avoid usage of deprecated dmaengine_terminate_all and use
dmaengine_terminate_sync and dmaengine_terminate_async instead.

Signed-off-by: Alain Volmat <[email protected]>
Signed-off-by: Valentin Caron <[email protected]>
---
drivers/spi/spi-stm32.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 5d9439ae1c09..82fbd20e8a96 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -657,9 +657,9 @@ static void stm32f4_spi_disable(struct stm32_spi *spi)
}

if (spi->cur_usedma && spi->dma_tx)
- dmaengine_terminate_all(spi->dma_tx);
+ dmaengine_terminate_async(spi->dma_tx);
if (spi->cur_usedma && spi->dma_rx)
- dmaengine_terminate_all(spi->dma_rx);
+ dmaengine_terminate_async(spi->dma_rx);

stm32_spi_clr_bits(spi, STM32F4_SPI_CR1, STM32F4_SPI_CR1_SPE);

@@ -696,9 +696,9 @@ static void stm32h7_spi_disable(struct stm32_spi *spi)
}

if (spi->cur_usedma && spi->dma_tx)
- dmaengine_terminate_all(spi->dma_tx);
+ dmaengine_terminate_async(spi->dma_tx);
if (spi->cur_usedma && spi->dma_rx)
- dmaengine_terminate_all(spi->dma_rx);
+ dmaengine_terminate_async(spi->dma_rx);

stm32_spi_clr_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_SPE);

@@ -1302,7 +1302,7 @@ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,

dma_submit_error:
if (spi->dma_rx)
- dmaengine_terminate_all(spi->dma_rx);
+ dmaengine_terminate_sync(spi->dma_rx);

dma_desc_error:
stm32_spi_clr_bits(spi, spi->cfg->regs->dma_rx_en.reg,
--
2.25.1

2023-04-28 12:18:39

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 2/7] spi: stm32: renaming of spi_master into spi_controller

From: Alain Volmat <[email protected]>

Preparing introduction of SPI slave, rename the spi_master structure
into spi_controller. This doesn't have any functional impact since
spi_master was already a macro for spi_controller.
Referring now to ctrl instead of master since the spi_controller
structure might not be used as a master controller only.

Signed-off-by: Alain Volmat <[email protected]>
Signed-off-by: Valentin Caron <[email protected]>
---
drivers/spi/spi-stm32.c | 154 ++++++++++++++++++++--------------------
1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index d6598e4116bd..5d9439ae1c09 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
//
-// STMicroelectronics STM32 SPI Controller driver (master mode only)
+// STMicroelectronics STM32 SPI Controller driver
//
// Copyright (C) 2017, STMicroelectronics - All Rights Reserved
// Author(s): Amelie Delaunay <[email protected]> for STMicroelectronics.
@@ -258,7 +258,7 @@ struct stm32_spi_cfg {
/**
* struct stm32_spi - private data of the SPI controller
* @dev: driver model representation of the controller
- * @master: controller master interface
+ * @ctrl: controller interface
* @cfg: compatible configuration data
* @base: virtual memory area
* @clk: hw kernel clock feeding the SPI clock generator
@@ -283,7 +283,7 @@ struct stm32_spi_cfg {
*/
struct stm32_spi {
struct device *dev;
- struct spi_master *master;
+ struct spi_controller *ctrl;
const struct stm32_spi_cfg *cfg;
void __iomem *base;
struct clk *clk;
@@ -437,9 +437,9 @@ static int stm32_spi_prepare_mbr(struct stm32_spi *spi, u32 speed_hz,
div = DIV_ROUND_CLOSEST(spi->clk_rate & ~0x1, speed_hz);

/*
- * SPI framework set xfer->speed_hz to master->max_speed_hz if
- * xfer->speed_hz is greater than master->max_speed_hz, and it returns
- * an error when xfer->speed_hz is lower than master->min_speed_hz, so
+ * SPI framework set xfer->speed_hz to ctrl->max_speed_hz if
+ * xfer->speed_hz is greater than ctrl->max_speed_hz, and it returns
+ * an error when xfer->speed_hz is lower than ctrl->min_speed_hz, so
* no need to check it there.
* However, we need to ensure the following calculations.
*/
@@ -714,19 +714,19 @@ static void stm32h7_spi_disable(struct stm32_spi *spi)

/**
* stm32_spi_can_dma - Determine if the transfer is eligible for DMA use
- * @master: controller master interface
+ * @ctrl: controller interface
* @spi_dev: pointer to the spi device
* @transfer: pointer to spi transfer
*
* If driver has fifo and the current transfer size is greater than fifo size,
* use DMA. Otherwise use DMA for transfer longer than defined DMA min bytes.
*/
-static bool stm32_spi_can_dma(struct spi_master *master,
+static bool stm32_spi_can_dma(struct spi_controller *ctrl,
struct spi_device *spi_dev,
struct spi_transfer *transfer)
{
unsigned int dma_size;
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);

if (spi->cfg->has_fifo)
dma_size = spi->fifo_size;
@@ -742,12 +742,12 @@ static bool stm32_spi_can_dma(struct spi_master *master,
/**
* stm32f4_spi_irq_event - Interrupt handler for SPI controller events
* @irq: interrupt line
- * @dev_id: SPI controller master interface
+ * @dev_id: SPI controller ctrl interface
*/
static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
{
- struct spi_master *master = dev_id;
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_id;
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
u32 sr, mask = 0;
bool end = false;

@@ -830,14 +830,14 @@ static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
/**
* stm32f4_spi_irq_thread - Thread of interrupt handler for SPI controller
* @irq: interrupt line
- * @dev_id: SPI controller master interface
+ * @dev_id: SPI controller interface
*/
static irqreturn_t stm32f4_spi_irq_thread(int irq, void *dev_id)
{
- struct spi_master *master = dev_id;
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_id;
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);

- spi_finalize_current_transfer(master);
+ spi_finalize_current_transfer(ctrl);
stm32f4_spi_disable(spi);

return IRQ_HANDLED;
@@ -846,12 +846,12 @@ static irqreturn_t stm32f4_spi_irq_thread(int irq, void *dev_id)
/**
* stm32h7_spi_irq_thread - Thread of interrupt handler for SPI controller
* @irq: interrupt line
- * @dev_id: SPI controller master interface
+ * @dev_id: SPI controller interface
*/
static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
{
- struct spi_master *master = dev_id;
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_id;
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
u32 sr, ier, mask;
unsigned long flags;
bool end = false;
@@ -931,7 +931,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)

if (end) {
stm32h7_spi_disable(spi);
- spi_finalize_current_transfer(master);
+ spi_finalize_current_transfer(ctrl);
}

return IRQ_HANDLED;
@@ -939,13 +939,13 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)

/**
* stm32_spi_prepare_msg - set up the controller to transfer a single message
- * @master: controller master interface
+ * @ctrl: controller interface
* @msg: pointer to spi message
*/
-static int stm32_spi_prepare_msg(struct spi_master *master,
+static int stm32_spi_prepare_msg(struct spi_controller *ctrl,
struct spi_message *msg)
{
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
struct spi_device *spi_dev = msg->spi;
struct device_node *np = spi_dev->dev.of_node;
unsigned long flags;
@@ -984,9 +984,9 @@ static int stm32_spi_prepare_msg(struct spi_master *master,
if (spi->cfg->set_number_of_data) {
int ret;

- ret = spi_split_transfers_maxwords(master, msg,
- STM32H7_SPI_TSIZE_MAX,
- GFP_KERNEL | GFP_DMA);
+ ret = spi_split_transfers_maxsize(ctrl, msg,
+ STM32H7_SPI_TSIZE_MAX,
+ GFP_KERNEL | GFP_DMA);
if (ret)
return ret;
}
@@ -1016,7 +1016,7 @@ static void stm32f4_spi_dma_tx_cb(void *data)
struct stm32_spi *spi = data;

if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
- spi_finalize_current_transfer(spi->master);
+ spi_finalize_current_transfer(spi->ctrl);
stm32f4_spi_disable(spi);
}
}
@@ -1031,7 +1031,7 @@ static void stm32_spi_dma_rx_cb(void *data)
{
struct stm32_spi *spi = data;

- spi_finalize_current_transfer(spi->master);
+ spi_finalize_current_transfer(spi->ctrl);
spi->cfg->disable(spi);
}

@@ -1589,18 +1589,18 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,

/**
* stm32_spi_transfer_one - transfer a single spi_transfer
- * @master: controller master interface
+ * @ctrl: controller interface
* @spi_dev: pointer to the spi device
* @transfer: pointer to spi transfer
*
* It must return 0 if the transfer is finished or 1 if the transfer is still
* in progress.
*/
-static int stm32_spi_transfer_one(struct spi_master *master,
+static int stm32_spi_transfer_one(struct spi_controller *ctrl,
struct spi_device *spi_dev,
struct spi_transfer *transfer)
{
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
int ret;

spi->tx_buf = transfer->tx_buf;
@@ -1608,8 +1608,8 @@ static int stm32_spi_transfer_one(struct spi_master *master,
spi->tx_len = spi->tx_buf ? transfer->len : 0;
spi->rx_len = spi->rx_buf ? transfer->len : 0;

- spi->cur_usedma = (master->can_dma &&
- master->can_dma(master, spi_dev, transfer));
+ spi->cur_usedma = (ctrl->can_dma &&
+ ctrl->can_dma(ctrl, spi_dev, transfer));

ret = stm32_spi_transfer_one_setup(spi, spi_dev, transfer);
if (ret) {
@@ -1625,13 +1625,13 @@ static int stm32_spi_transfer_one(struct spi_master *master,

/**
* stm32_spi_unprepare_msg - relax the hardware
- * @master: controller master interface
+ * @ctrl: controller interface
* @msg: pointer to the spi message
*/
-static int stm32_spi_unprepare_msg(struct spi_master *master,
+static int stm32_spi_unprepare_msg(struct spi_controller *ctrl,
struct spi_message *msg)
{
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);

spi->cfg->disable(spi);

@@ -1758,22 +1758,22 @@ MODULE_DEVICE_TABLE(of, stm32_spi_of_match);

static int stm32_spi_probe(struct platform_device *pdev)
{
- struct spi_master *master;
+ struct spi_controller *ctrl;
struct stm32_spi *spi;
struct resource *res;
struct reset_control *rst;
int ret;

- master = devm_spi_alloc_master(&pdev->dev, sizeof(struct stm32_spi));
- if (!master) {
+ ctrl = devm_spi_alloc_master(&pdev->dev, sizeof(struct stm32_spi));
+ if (!ctrl) {
dev_err(&pdev->dev, "spi master allocation failed\n");
return -ENOMEM;
}
- platform_set_drvdata(pdev, master);
+ platform_set_drvdata(pdev, ctrl);

- spi = spi_master_get_devdata(master);
+ spi = spi_controller_get_devdata(ctrl);
spi->dev = &pdev->dev;
- spi->master = master;
+ spi->ctrl = ctrl;
spin_lock_init(&spi->lock);

spi->cfg = (const struct stm32_spi_cfg *)
@@ -1794,7 +1794,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
ret = devm_request_threaded_irq(&pdev->dev, spi->irq,
spi->cfg->irq_handler_event,
spi->cfg->irq_handler_thread,
- IRQF_ONESHOT, pdev->name, master);
+ IRQF_ONESHOT, pdev->name, ctrl);
if (ret) {
dev_err(&pdev->dev, "irq%d request failed: %d\n", spi->irq,
ret);
@@ -1843,19 +1843,19 @@ static int stm32_spi_probe(struct platform_device *pdev)
goto err_clk_disable;
}

- master->dev.of_node = pdev->dev.of_node;
- master->auto_runtime_pm = true;
- master->bus_num = pdev->id;
- master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | SPI_LSB_FIRST |
- SPI_3WIRE;
- master->bits_per_word_mask = spi->cfg->get_bpw_mask(spi);
- master->max_speed_hz = spi->clk_rate / spi->cfg->baud_rate_div_min;
- master->min_speed_hz = spi->clk_rate / spi->cfg->baud_rate_div_max;
- master->use_gpio_descriptors = true;
- master->prepare_message = stm32_spi_prepare_msg;
- master->transfer_one = stm32_spi_transfer_one;
- master->unprepare_message = stm32_spi_unprepare_msg;
- master->flags = spi->cfg->flags;
+ ctrl->dev.of_node = pdev->dev.of_node;
+ ctrl->auto_runtime_pm = true;
+ ctrl->bus_num = pdev->id;
+ ctrl->mode_bits = SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | SPI_LSB_FIRST |
+ SPI_3WIRE;
+ ctrl->bits_per_word_mask = spi->cfg->get_bpw_mask(spi);
+ ctrl->max_speed_hz = spi->clk_rate / spi->cfg->baud_rate_div_min;
+ ctrl->min_speed_hz = spi->clk_rate / spi->cfg->baud_rate_div_max;
+ ctrl->use_gpio_descriptors = true;
+ ctrl->prepare_message = stm32_spi_prepare_msg;
+ ctrl->transfer_one = stm32_spi_transfer_one;
+ ctrl->unprepare_message = stm32_spi_unprepare_msg;
+ ctrl->flags = spi->cfg->flags;

spi->dma_tx = dma_request_chan(spi->dev, "tx");
if (IS_ERR(spi->dma_tx)) {
@@ -1866,7 +1866,7 @@ static int stm32_spi_probe(struct platform_device *pdev)

dev_warn(&pdev->dev, "failed to request tx dma channel\n");
} else {
- master->dma_tx = spi->dma_tx;
+ ctrl->dma_tx = spi->dma_tx;
}

spi->dma_rx = dma_request_chan(spi->dev, "rx");
@@ -1878,11 +1878,11 @@ static int stm32_spi_probe(struct platform_device *pdev)

dev_warn(&pdev->dev, "failed to request rx dma channel\n");
} else {
- master->dma_rx = spi->dma_rx;
+ ctrl->dma_rx = spi->dma_rx;
}

if (spi->dma_tx || spi->dma_rx)
- master->can_dma = stm32_spi_can_dma;
+ ctrl->can_dma = stm32_spi_can_dma;

pm_runtime_set_autosuspend_delay(&pdev->dev,
STM32_SPI_AUTOSUSPEND_DELAY);
@@ -1891,9 +1891,9 @@ static int stm32_spi_probe(struct platform_device *pdev)
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_enable(&pdev->dev);

- ret = spi_register_master(master);
+ ret = spi_register_controller(ctrl);
if (ret) {
- dev_err(&pdev->dev, "spi master registration failed: %d\n",
+ dev_err(&pdev->dev, "spi controller registration failed: %d\n",
ret);
goto err_pm_disable;
}
@@ -1923,12 +1923,12 @@ static int stm32_spi_probe(struct platform_device *pdev)

static void stm32_spi_remove(struct platform_device *pdev)
{
- struct spi_master *master = platform_get_drvdata(pdev);
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = platform_get_drvdata(pdev);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);

pm_runtime_get_sync(&pdev->dev);

- spi_unregister_master(master);
+ spi_unregister_controller(ctrl);
spi->cfg->disable(spi);

pm_runtime_disable(&pdev->dev);
@@ -1936,10 +1936,10 @@ static void stm32_spi_remove(struct platform_device *pdev)
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);

- if (master->dma_tx)
- dma_release_channel(master->dma_tx);
- if (master->dma_rx)
- dma_release_channel(master->dma_rx);
+ if (ctrl->dma_tx)
+ dma_release_channel(ctrl->dma_tx);
+ if (ctrl->dma_rx)
+ dma_release_channel(ctrl->dma_rx);

clk_disable_unprepare(spi->clk);

@@ -1949,8 +1949,8 @@ static void stm32_spi_remove(struct platform_device *pdev)

static int __maybe_unused stm32_spi_runtime_suspend(struct device *dev)
{
- struct spi_master *master = dev_get_drvdata(dev);
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);

clk_disable_unprepare(spi->clk);

@@ -1959,8 +1959,8 @@ static int __maybe_unused stm32_spi_runtime_suspend(struct device *dev)

static int __maybe_unused stm32_spi_runtime_resume(struct device *dev)
{
- struct spi_master *master = dev_get_drvdata(dev);
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
int ret;

ret = pinctrl_pm_select_default_state(dev);
@@ -1972,10 +1972,10 @@ static int __maybe_unused stm32_spi_runtime_resume(struct device *dev)

static int __maybe_unused stm32_spi_suspend(struct device *dev)
{
- struct spi_master *master = dev_get_drvdata(dev);
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
int ret;

- ret = spi_master_suspend(master);
+ ret = spi_controller_suspend(ctrl);
if (ret)
return ret;

@@ -1984,15 +1984,15 @@ static int __maybe_unused stm32_spi_suspend(struct device *dev)

static int __maybe_unused stm32_spi_resume(struct device *dev)
{
- struct spi_master *master = dev_get_drvdata(dev);
- struct stm32_spi *spi = spi_master_get_devdata(master);
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
int ret;

ret = pm_runtime_force_resume(dev);
if (ret)
return ret;

- ret = spi_master_resume(master);
+ ret = spi_controller_resume(ctrl);
if (ret) {
clk_disable_unprepare(spi->clk);
return ret;
--
2.25.1

2023-04-28 12:19:12

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 5/7] spi: stm32: introduction of stm32h7 SPI slave support

Add support for stm32h7 to use SPI controller in slave role.
In such case, the spi instance should have the spi-slave property
defined.

Signed-off-by: Alain Volmat <[email protected]>
Signed-off-by: Valentin Caron <[email protected]>
---
drivers/spi/Kconfig | 1 +
drivers/spi/spi-stm32.c | 112 ++++++++++++++++++++++++++++------------
2 files changed, 79 insertions(+), 34 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3de2ebe8294a..14810d24733b 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -936,6 +936,7 @@ config SPI_SPRD_ADI
config SPI_STM32
tristate "STMicroelectronics STM32 SPI controller"
depends on ARCH_STM32 || COMPILE_TEST
+ select SPI_SLAVE
help
SPI driver for STMicroelectronics STM32 SoCs.

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 82fbd20e8a96..2db6f93654d7 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -117,6 +117,7 @@
#define STM32H7_SPI_CFG2_CPHA BIT(24)
#define STM32H7_SPI_CFG2_CPOL BIT(25)
#define STM32H7_SPI_CFG2_SSM BIT(26)
+#define STM32H7_SPI_CFG2_SSIOP BIT(28)
#define STM32H7_SPI_CFG2_AFCNTR BIT(31)

/* STM32H7_SPI_IER bit fields */
@@ -170,6 +171,10 @@
*/
#define SPI_DMA_MIN_BYTES 16

+/* STM32 SPI driver helpers */
+#define STM32_SPI_MASTER_MODE(stm32_spi) (!(stm32_spi)->slave_mode)
+#define STM32_SPI_SLAVE_MODE(stm32_spi) ((stm32_spi)->slave_mode)
+
/**
* struct stm32_spi_reg - stm32 SPI register & bitfield desc
* @reg: register offset
@@ -190,6 +195,7 @@ struct stm32_spi_reg {
* @cpol: clock polarity register and polarity bit
* @cpha: clock phase register and phase bit
* @lsb_first: LSB transmitted first register and bit
+ * @cs_high: chips select active value
* @br: baud rate register and bitfields
* @rx: SPI RX data register
* @tx: SPI TX data register
@@ -201,6 +207,7 @@ struct stm32_spi_regspec {
const struct stm32_spi_reg cpol;
const struct stm32_spi_reg cpha;
const struct stm32_spi_reg lsb_first;
+ const struct stm32_spi_reg cs_high;
const struct stm32_spi_reg br;
const struct stm32_spi_reg rx;
const struct stm32_spi_reg tx;
@@ -280,6 +287,7 @@ struct stm32_spi_cfg {
* @dma_tx: dma channel for TX transfer
* @dma_rx: dma channel for RX transfer
* @phys_addr: SPI registers physical base address
+ * @slave_mode: the controller is configured as SPI slave
*/
struct stm32_spi {
struct device *dev;
@@ -307,6 +315,8 @@ struct stm32_spi {
struct dma_chan *dma_tx;
struct dma_chan *dma_rx;
dma_addr_t phys_addr;
+
+ bool slave_mode;
};

static const struct stm32_spi_regspec stm32f4_spi_regspec = {
@@ -318,6 +328,7 @@ static const struct stm32_spi_regspec stm32f4_spi_regspec = {
.cpol = { STM32F4_SPI_CR1, STM32F4_SPI_CR1_CPOL },
.cpha = { STM32F4_SPI_CR1, STM32F4_SPI_CR1_CPHA },
.lsb_first = { STM32F4_SPI_CR1, STM32F4_SPI_CR1_LSBFRST },
+ .cs_high = {},
.br = { STM32F4_SPI_CR1, STM32F4_SPI_CR1_BR, STM32F4_SPI_CR1_BR_SHIFT },

.rx = { STM32F4_SPI_DR },
@@ -336,6 +347,7 @@ static const struct stm32_spi_regspec stm32h7_spi_regspec = {
.cpol = { STM32H7_SPI_CFG2, STM32H7_SPI_CFG2_CPOL },
.cpha = { STM32H7_SPI_CFG2, STM32H7_SPI_CFG2_CPHA },
.lsb_first = { STM32H7_SPI_CFG2, STM32H7_SPI_CFG2_LSBFRST },
+ .cs_high = { STM32H7_SPI_CFG2, STM32H7_SPI_CFG2_SSIOP },
.br = { STM32H7_SPI_CFG1, STM32H7_SPI_CFG1_MBR,
STM32H7_SPI_CFG1_MBR_SHIFT },

@@ -971,6 +983,11 @@ static int stm32_spi_prepare_msg(struct spi_controller *ctrl,
else
clrb |= spi->cfg->regs->lsb_first.mask;

+ if (STM32_SPI_SLAVE_MODE(spi) && spi_dev->mode & SPI_CS_HIGH)
+ setb |= spi->cfg->regs->cs_high.mask;
+ else
+ clrb |= spi->cfg->regs->cs_high.mask;
+
dev_dbg(spi->dev, "cpol=%d cpha=%d lsb_first=%d cs_high=%d\n",
!!(spi_dev->mode & SPI_CPOL),
!!(spi_dev->mode & SPI_CPHA),
@@ -1161,7 +1178,8 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
if (spi->tx_buf)
stm32h7_spi_write_txfifo(spi);

- stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
+ if (STM32_SPI_MASTER_MODE(spi))
+ stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);

writel_relaxed(ier, spi->base + STM32H7_SPI_IER);

@@ -1208,7 +1226,8 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)

stm32_spi_enable(spi);

- stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
+ if (STM32_SPI_MASTER_MODE(spi))
+ stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
}

/**
@@ -1536,16 +1555,18 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
spi->cfg->set_bpw(spi);

/* Update spi->cur_speed with real clock speed */
- mbr = stm32_spi_prepare_mbr(spi, transfer->speed_hz,
- spi->cfg->baud_rate_div_min,
- spi->cfg->baud_rate_div_max);
- if (mbr < 0) {
- ret = mbr;
- goto out;
- }
+ if (STM32_SPI_MASTER_MODE(spi)) {
+ mbr = stm32_spi_prepare_mbr(spi, transfer->speed_hz,
+ spi->cfg->baud_rate_div_min,
+ spi->cfg->baud_rate_div_max);
+ if (mbr < 0) {
+ ret = mbr;
+ goto out;
+ }

- transfer->speed_hz = spi->cur_speed;
- stm32_spi_set_mbr(spi, mbr);
+ transfer->speed_hz = spi->cur_speed;
+ stm32_spi_set_mbr(spi, mbr);
+ }

comm_type = stm32_spi_communication_type(spi_dev, transfer);
ret = spi->cfg->set_mode(spi, comm_type);
@@ -1554,7 +1575,7 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,

spi->cur_comm = comm_type;

- if (spi->cfg->set_data_idleness)
+ if (STM32_SPI_MASTER_MODE(spi) && spi->cfg->set_data_idleness)
spi->cfg->set_data_idleness(spi, transfer->len);

if (spi->cur_bpw <= 8)
@@ -1575,7 +1596,8 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
dev_dbg(spi->dev,
"data frame of %d-bit, data packet of %d data frames\n",
spi->cur_bpw, spi->cur_fthlv);
- dev_dbg(spi->dev, "speed set to %dHz\n", spi->cur_speed);
+ if (STM32_SPI_MASTER_MODE(spi))
+ dev_dbg(spi->dev, "speed set to %dHz\n", spi->cur_speed);
dev_dbg(spi->dev, "transfer of %d bytes (%d data frames)\n",
spi->cur_xferlen, nb_words);
dev_dbg(spi->dev, "dma %s\n",
@@ -1670,12 +1692,13 @@ static int stm32f4_spi_config(struct stm32_spi *spi)
}

/**
- * stm32h7_spi_config - Configure SPI controller as SPI master
+ * stm32h7_spi_config - Configure SPI controller
* @spi: pointer to the spi controller data structure
*/
static int stm32h7_spi_config(struct stm32_spi *spi)
{
unsigned long flags;
+ u32 cr1 = 0, cfg2 = 0;

spin_lock_irqsave(&spi->lock, flags);

@@ -1683,24 +1706,28 @@ static int stm32h7_spi_config(struct stm32_spi *spi)
stm32_spi_clr_bits(spi, STM32H7_SPI_I2SCFGR,
STM32H7_SPI_I2SCFGR_I2SMOD);

- /*
- * - SS input value high
- * - transmitter half duplex direction
- * - automatic communication suspend when RX-Fifo is full
- */
- stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_SSI |
- STM32H7_SPI_CR1_HDDIR |
- STM32H7_SPI_CR1_MASRX);
+ if (STM32_SPI_SLAVE_MODE(spi)) {
+ /* Use native slave select */
+ cfg2 &= ~STM32H7_SPI_CFG2_SSM;
+ } else {
+ /*
+ * - Transmitter half duplex direction
+ * - Automatic communication suspend when RX-Fifo is full
+ * - SS input value high
+ */
+ cr1 |= STM32H7_SPI_CR1_HDDIR | STM32H7_SPI_CR1_MASRX | STM32H7_SPI_CR1_SSI;

- /*
- * - Set the master mode (default Motorola mode)
- * - Consider 1 master/n slaves configuration and
- * SS input value is determined by the SSI bit
- * - keep control of all associated GPIOs
- */
- stm32_spi_set_bits(spi, STM32H7_SPI_CFG2, STM32H7_SPI_CFG2_MASTER |
- STM32H7_SPI_CFG2_SSM |
- STM32H7_SPI_CFG2_AFCNTR);
+ /*
+ * - Set the master mode (default Motorola mode)
+ * - Consider 1 master/n slaves configuration and
+ * SS input value is determined by the SSI bit
+ * - keep control of all associated GPIOs
+ */
+ cfg2 |= STM32H7_SPI_CFG2_MASTER | STM32H7_SPI_CFG2_SSM | STM32H7_SPI_CFG2_AFCNTR;
+ }
+
+ stm32_spi_set_bits(spi, STM32H7_SPI_CR1, cr1);
+ stm32_spi_set_bits(spi, STM32H7_SPI_CFG2, cfg2);

spin_unlock_irqrestore(&spi->lock, flags);

@@ -1756,17 +1783,30 @@ static const struct of_device_id stm32_spi_of_match[] = {
};
MODULE_DEVICE_TABLE(of, stm32_spi_of_match);

+static int stm32h7_spi_slave_abort(struct spi_controller *ctrl)
+{
+ spi_finalize_current_transfer(ctrl);
+ return 0;
+}
+
static int stm32_spi_probe(struct platform_device *pdev)
{
struct spi_controller *ctrl;
struct stm32_spi *spi;
struct resource *res;
struct reset_control *rst;
+ struct device_node *np = pdev->dev.of_node;
+ bool slave_mode;
int ret;

- ctrl = devm_spi_alloc_master(&pdev->dev, sizeof(struct stm32_spi));
+ slave_mode = of_property_read_bool(np, "spi-slave");
+
+ if (slave_mode)
+ ctrl = devm_spi_alloc_slave(&pdev->dev, sizeof(struct stm32_spi));
+ else
+ ctrl = devm_spi_alloc_master(&pdev->dev, sizeof(struct stm32_spi));
if (!ctrl) {
- dev_err(&pdev->dev, "spi master allocation failed\n");
+ dev_err(&pdev->dev, "spi controller allocation failed\n");
return -ENOMEM;
}
platform_set_drvdata(pdev, ctrl);
@@ -1774,6 +1814,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
spi = spi_controller_get_devdata(ctrl);
spi->dev = &pdev->dev;
spi->ctrl = ctrl;
+ spi->slave_mode = slave_mode;
spin_lock_init(&spi->lock);

spi->cfg = (const struct stm32_spi_cfg *)
@@ -1856,6 +1897,8 @@ static int stm32_spi_probe(struct platform_device *pdev)
ctrl->transfer_one = stm32_spi_transfer_one;
ctrl->unprepare_message = stm32_spi_unprepare_msg;
ctrl->flags = spi->cfg->flags;
+ if (STM32_SPI_SLAVE_MODE(spi))
+ ctrl->slave_abort = stm32h7_spi_slave_abort;

spi->dma_tx = dma_request_chan(spi->dev, "tx");
if (IS_ERR(spi->dma_tx)) {
@@ -1901,7 +1944,8 @@ static int stm32_spi_probe(struct platform_device *pdev)
pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_put_autosuspend(&pdev->dev);

- dev_info(&pdev->dev, "driver initialized\n");
+ dev_info(&pdev->dev, "driver initialized (%s mode)\n",
+ STM32_SPI_MASTER_MODE(spi) ? "master" : "slave");

return 0;

--
2.25.1

2023-04-28 12:19:19

by Valentin Caron

[permalink] [raw]
Subject: [PATCH 7/7] spi: stm32: add support for stm32h7 SPI slave underrun detection

If stm32h7 SPI controller is in slave role and TX FIFO is abnormally empty
during transaction, controller is able to automatically send either a
pattern, the last transmitted data, or the last received data.

Signed-off-by: Valentin Caron <[email protected]>
---
drivers/spi/spi-stm32.c | 112 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 112 insertions(+)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 2db6f93654d7..0063c2f69265 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -18,6 +18,7 @@
#include <linux/pm_runtime.h>
#include <linux/reset.h>
#include <linux/spi/spi.h>
+#include <dt-bindings/spi/spi-stm32.h>

#define DRIVER_NAME "spi_stm32"

@@ -84,6 +85,7 @@
#define STM32H7_SPI_IFCR 0x18
#define STM32H7_SPI_TXDR 0x20
#define STM32H7_SPI_RXDR 0x30
+#define STM32H7_SPI_UDRDR 0x4C
#define STM32H7_SPI_I2SCFGR 0x50

/* STM32H7_SPI_CR1 bit fields */
@@ -101,6 +103,14 @@
/* STM32H7_SPI_CFG1 bit fields */
#define STM32H7_SPI_CFG1_DSIZE GENMASK(4, 0)
#define STM32H7_SPI_CFG1_FTHLV GENMASK(8, 5)
+#define STM32H7_SPI_CFG1_UDRDET GENMASK(12, 11)
+#define STM32H7_SPI_CFG1_UDRDET_BEGIN 0
+#define STM32H7_SPI_CFG1_UDRDET_LAST 1
+#define STM32H7_SPI_CFG1_UDRDET_SS 2
+#define STM32H7_SPI_CFG1_UDRCFG GENMASK(10, 9)
+#define STM32H7_SPI_CFG1_UDRCFG_PTRN 0
+#define STM32H7_SPI_CFG1_UDRCFG_LAST_R 1
+#define STM32H7_SPI_CFG1_UDRCFG_LAST_T 2
#define STM32H7_SPI_CFG1_RXDMAEN BIT(14)
#define STM32H7_SPI_CFG1_TXDMAEN BIT(15)
#define STM32H7_SPI_CFG1_MBR GENMASK(30, 28)
@@ -126,6 +136,7 @@
#define STM32H7_SPI_IER_DXPIE BIT(2)
#define STM32H7_SPI_IER_EOTIE BIT(3)
#define STM32H7_SPI_IER_TXTFIE BIT(4)
+#define STM32H7_SPI_IER_UDRIE BIT(5)
#define STM32H7_SPI_IER_OVRIE BIT(6)
#define STM32H7_SPI_IER_MODFIE BIT(9)
#define STM32H7_SPI_IER_ALL GENMASK(10, 0)
@@ -134,6 +145,7 @@
#define STM32H7_SPI_SR_RXP BIT(0)
#define STM32H7_SPI_SR_TXP BIT(1)
#define STM32H7_SPI_SR_EOT BIT(3)
+#define STM32H7_SPI_SR_UDR BIT(5)
#define STM32H7_SPI_SR_OVR BIT(6)
#define STM32H7_SPI_SR_MODF BIT(9)
#define STM32H7_SPI_SR_SUSP BIT(11)
@@ -239,6 +251,8 @@ struct stm32_spi;
* @baud_rate_div_max: maximum baud rate divisor
* @has_fifo: boolean to know if fifo is used for driver
* @flags: compatible specific SPI controller flags used at registration time
+ * @set_slave_udr: routine to configure registers to desired slave underrun
+ * behavior (if driver has this functionality)
*/
struct stm32_spi_cfg {
const struct stm32_spi_regspec *regs;
@@ -260,6 +274,7 @@ struct stm32_spi_cfg {
unsigned int baud_rate_div_max;
bool has_fifo;
u16 flags;
+ void (*set_slave_udr)(struct stm32_spi *spi);
};

/**
@@ -288,6 +303,8 @@ struct stm32_spi_cfg {
* @dma_rx: dma channel for RX transfer
* @phys_addr: SPI registers physical base address
* @slave_mode: the controller is configured as SPI slave
+ * @slave_udr_mode: slave underrun behavior
+ * @slave_udr_pattern: slave underrun pattern parameter
*/
struct stm32_spi {
struct device *dev;
@@ -317,6 +334,8 @@ struct stm32_spi {
dma_addr_t phys_addr;

bool slave_mode;
+ u32 slave_udr_mode;
+ u32 slave_udr_pattern;
};

static const struct stm32_spi_regspec stm32f4_spi_regspec = {
@@ -921,6 +940,14 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
end = true;
}

+ if (sr & STM32H7_SPI_SR_UDR) {
+ static DEFINE_RATELIMIT_STATE(rs,
+ DEFAULT_RATELIMIT_INTERVAL * 10,
+ 1);
+ if (__ratelimit(&rs))
+ dev_dbg_ratelimited(spi->dev, "Underrun detected\n");
+ }
+
if (sr & STM32H7_SPI_SR_EOT) {
if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
stm32h7_spi_read_rxfifo(spi);
@@ -1178,6 +1205,9 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
if (spi->tx_buf)
stm32h7_spi_write_txfifo(spi);

+ if (STM32_SPI_SLAVE_MODE(spi) && spi->slave_udr_mode != SPI_NO_ACTION)
+ ier |= STM32H7_SPI_IER_UDRIE;
+
if (STM32_SPI_MASTER_MODE(spi))
stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);

@@ -1222,6 +1252,9 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX)
ier |= STM32H7_SPI_IER_EOTIE | STM32H7_SPI_IER_TXTFIE;

+ if (STM32_SPI_SLAVE_MODE(spi) && spi->slave_udr_mode != SPI_NO_ACTION)
+ ier |= STM32H7_SPI_IER_UDRIE;
+
stm32_spi_set_bits(spi, STM32H7_SPI_IER, ier);

stm32_spi_enable(spi);
@@ -1530,6 +1563,53 @@ static int stm32h7_spi_number_of_data(struct stm32_spi *spi, u32 nb_words)
return 0;
}

+/**
+ * stm32h7_spi_set_slave_udr - configure slave underrun detection and reaction
+ * @spi: pointer to the spi controller data structure
+ */
+static void stm32h7_spi_set_slave_udr(struct stm32_spi *spi)
+{
+ u32 max_udr_ptrn, udr_ptrn, cfg1_setb = 0;
+
+ if (spi->slave_udr_mode == SPI_NO_ACTION)
+ return;
+
+ switch (spi->slave_udr_mode) {
+ case SPI_SEND_PATTERN:
+ max_udr_ptrn = (1 << spi->cur_bpw) - 1;
+ if (spi->slave_udr_pattern > max_udr_ptrn) {
+ udr_ptrn = spi->slave_udr_pattern & max_udr_ptrn;
+ dev_warn(spi->dev,
+ "force slave underrun pattern to data width (> 0x%x, set 0x%x)\n",
+ max_udr_ptrn, udr_ptrn);
+ } else {
+ udr_ptrn = spi->slave_udr_pattern;
+ dev_dbg(spi->dev, "spi slave underrun: send pattern (0x%x)\n",
+ spi->slave_udr_pattern);
+ }
+ writel_relaxed(udr_ptrn, spi->base + STM32H7_SPI_UDRDR);
+ cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_PTRN);
+ break;
+ case SPI_REPEAT_LAST_RECEIVED_DATA:
+ cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_LAST_R);
+ dev_dbg(spi->dev, "spi slave underrun: repeat received data\n");
+ break;
+ case SPI_REPEAT_LAST_TRANSMITTED_DATA:
+ cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_LAST_T);
+ dev_dbg(spi->dev, "spi slave underrun: repeat transmitted data\n");
+ break;
+ default:
+ dev_warn(spi->dev, "slave underrun detection disabled\n");
+ spi->slave_udr_mode = SPI_NO_ACTION;
+ }
+
+ if (spi->slave_udr_mode != SPI_NO_ACTION) {
+ cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRDET, STM32H7_SPI_CFG1_UDRDET_LAST);
+
+ stm32_spi_set_bits(spi, STM32H7_SPI_CFG1, cfg1_setb);
+ }
+}
+
/**
* stm32_spi_transfer_one_setup - common setup to transfer a single
* spi_transfer either using DMA or
@@ -1591,6 +1671,9 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
goto out;
}

+ if (STM32_SPI_SLAVE_MODE(spi) && spi->cfg->set_slave_udr)
+ spi->cfg->set_slave_udr(spi);
+
dev_dbg(spi->dev, "transfer communication mode set to %d\n",
spi->cur_comm);
dev_dbg(spi->dev,
@@ -1774,6 +1857,7 @@ static const struct stm32_spi_cfg stm32h7_spi_cfg = {
.baud_rate_div_min = STM32H7_SPI_MBR_DIV_MIN,
.baud_rate_div_max = STM32H7_SPI_MBR_DIV_MAX,
.has_fifo = true,
+ .set_slave_udr = stm32h7_spi_set_slave_udr,
};

static const struct of_device_id stm32_spi_of_match[] = {
@@ -1789,6 +1873,31 @@ static int stm32h7_spi_slave_abort(struct spi_controller *ctrl)
return 0;
}

+static void stm32h7_spi_parse_slave_config(struct stm32_spi *spi, struct device_node *np)
+{
+ u32 udr_configs[2] = { 0, 0 };
+ int count, ret;
+
+ count = of_property_count_elems_of_size(np, "st,spi-slave-underrun", sizeof(u32));
+ if (count <= 0) {
+ if (count != -EINVAL)
+ dev_err(spi->dev, "Invalid st,spi-slave-underrun property\n");
+ return;
+ }
+
+ ret = of_property_read_u32_array(np, "st,spi-slave-underrun", udr_configs, count);
+ if (ret)
+ return;
+
+ spi->slave_udr_mode = udr_configs[0];
+ if (spi->slave_udr_mode == SPI_SEND_PATTERN) {
+ if (count > 1)
+ spi->slave_udr_pattern = udr_configs[1];
+ else
+ dev_warn(spi->dev, "Missing pattern in st,spi-slave-underrun property\n");
+ }
+}
+
static int stm32_spi_probe(struct platform_device *pdev)
{
struct spi_controller *ctrl;
@@ -1842,6 +1951,9 @@ static int stm32_spi_probe(struct platform_device *pdev)
return ret;
}

+ if (STM32_SPI_SLAVE_MODE(spi))
+ stm32h7_spi_parse_slave_config(spi, np);
+
spi->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(spi->clk)) {
ret = PTR_ERR(spi->clk);
--
2.25.1

2023-04-28 21:45:48

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/7] dt-bindings: spi: stm32: add address-cells and size-cells into yaml

On Fri, Apr 28, 2023 at 02:15:18PM +0200, Valentin Caron wrote:
> Theses properties need to be described to satisfy dtbs_check.

No, they are defined in spi-controller.yaml, so they should not be
needed here.

>
> Signed-off-by: Valentin Caron <[email protected]>
> ---
> Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> index 9ca1a843c820..c599eb359d56 100644
> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> @@ -29,6 +29,9 @@ allOf:
> st,spi-midi-ns: false
>
> properties:
> + "#address-cells": true
> + "#size-cells": true
> +
> compatible:
> enum:
> - st,stm32f4-spi
> --
> 2.25.1
>

2023-04-28 22:04:36

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: spi: stm32: add stm32h7 st,spi-slave-underrun property

On Fri, Apr 28, 2023 at 02:15:23PM +0200, Valentin Caron wrote:
> This property is used to enable and configure stm32h7 SPI controller to
> handle underrun that could appear in slave mode.
>
> Signed-off-by: Valentin Caron <[email protected]>
> ---
> .../devicetree/bindings/spi/st,stm32-spi.yaml | 8 ++++++++
> MAINTAINERS | 1 +
> include/dt-bindings/spi/spi-stm32.h | 15 +++++++++++++++
> 3 files changed, 24 insertions(+)
> create mode 100644 include/dt-bindings/spi/spi-stm32.h
>
> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> index 1d26fa2658c5..e946ea71a247 100644
> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> @@ -28,6 +28,7 @@ allOf:
> properties:
> st,spi-midi-ns: false
> spi-slave: false
> + st,spi-slave-underrun: false
>
> properties:
> "#address-cells": true
> @@ -70,6 +71,13 @@ properties:
> In case of spi-slave defined, if <0>, indicate that SS should be
> detected via the dedicated HW pin
>
> + st,spi-slave-underrun:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + First parameter enables and selects slave underrun reaction.
> + Refer to "dt-bindings/spi/spi-stm32.h" for the supported values.
> + Second parameter is the pattern in case of SPI_SEND_PATTERN mode.

So, max 2 cells? Then:

minItems: 1
maxItems: 2

Though I don't really think this belongs in DT. The driver implementing
the SPI slave function defines all the rest of the protocol the slave
implements. Why not this little bit? Perhaps there is no way for a SPI
slave driver to tell the SPI controller which controller specific mode
to use, so you abuse DT to configure the SPI controller. Also, with a
controller specific response, then the slave driver is coupled to that
SPI controller which isn't great either.

Rob

2023-05-02 07:28:27

by Alain Volmat

[permalink] [raw]
Subject: Re: [PATCH 7/7] spi: stm32: add support for stm32h7 SPI slave underrun detection

Hi Valentin,

thanks.

On Fri, Apr 28, 2023 at 02:15:24PM +0200, Valentin Caron wrote:
> If stm32h7 SPI controller is in slave role and TX FIFO is abnormally empty
> during transaction, controller is able to automatically send either a
> pattern, the last transmitted data, or the last received data.
>
> Signed-off-by: Valentin Caron <[email protected]>
> ---
> drivers/spi/spi-stm32.c | 112 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 112 insertions(+)
>
> diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
> index 2db6f93654d7..0063c2f69265 100644
> --- a/drivers/spi/spi-stm32.c
> +++ b/drivers/spi/spi-stm32.c
> @@ -18,6 +18,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
> #include <linux/spi/spi.h>
> +#include <dt-bindings/spi/spi-stm32.h>
>
> #define DRIVER_NAME "spi_stm32"
>
> @@ -84,6 +85,7 @@
> #define STM32H7_SPI_IFCR 0x18
> #define STM32H7_SPI_TXDR 0x20
> #define STM32H7_SPI_RXDR 0x30
> +#define STM32H7_SPI_UDRDR 0x4C
> #define STM32H7_SPI_I2SCFGR 0x50
>
> /* STM32H7_SPI_CR1 bit fields */
> @@ -101,6 +103,14 @@
> /* STM32H7_SPI_CFG1 bit fields */
> #define STM32H7_SPI_CFG1_DSIZE GENMASK(4, 0)
> #define STM32H7_SPI_CFG1_FTHLV GENMASK(8, 5)
> +#define STM32H7_SPI_CFG1_UDRDET GENMASK(12, 11)
> +#define STM32H7_SPI_CFG1_UDRDET_BEGIN 0
> +#define STM32H7_SPI_CFG1_UDRDET_LAST 1
> +#define STM32H7_SPI_CFG1_UDRDET_SS 2
> +#define STM32H7_SPI_CFG1_UDRCFG GENMASK(10, 9)
> +#define STM32H7_SPI_CFG1_UDRCFG_PTRN 0
> +#define STM32H7_SPI_CFG1_UDRCFG_LAST_R 1
> +#define STM32H7_SPI_CFG1_UDRCFG_LAST_T 2
> #define STM32H7_SPI_CFG1_RXDMAEN BIT(14)
> #define STM32H7_SPI_CFG1_TXDMAEN BIT(15)
> #define STM32H7_SPI_CFG1_MBR GENMASK(30, 28)
> @@ -126,6 +136,7 @@
> #define STM32H7_SPI_IER_DXPIE BIT(2)
> #define STM32H7_SPI_IER_EOTIE BIT(3)
> #define STM32H7_SPI_IER_TXTFIE BIT(4)
> +#define STM32H7_SPI_IER_UDRIE BIT(5)
> #define STM32H7_SPI_IER_OVRIE BIT(6)
> #define STM32H7_SPI_IER_MODFIE BIT(9)
> #define STM32H7_SPI_IER_ALL GENMASK(10, 0)
> @@ -134,6 +145,7 @@
> #define STM32H7_SPI_SR_RXP BIT(0)
> #define STM32H7_SPI_SR_TXP BIT(1)
> #define STM32H7_SPI_SR_EOT BIT(3)
> +#define STM32H7_SPI_SR_UDR BIT(5)
> #define STM32H7_SPI_SR_OVR BIT(6)
> #define STM32H7_SPI_SR_MODF BIT(9)
> #define STM32H7_SPI_SR_SUSP BIT(11)
> @@ -239,6 +251,8 @@ struct stm32_spi;
> * @baud_rate_div_max: maximum baud rate divisor
> * @has_fifo: boolean to know if fifo is used for driver
> * @flags: compatible specific SPI controller flags used at registration time
> + * @set_slave_udr: routine to configure registers to desired slave underrun
> + * behavior (if driver has this functionality)
> */
> struct stm32_spi_cfg {
> const struct stm32_spi_regspec *regs;
> @@ -260,6 +274,7 @@ struct stm32_spi_cfg {
> unsigned int baud_rate_div_max;
> bool has_fifo;
> u16 flags;
> + void (*set_slave_udr)(struct stm32_spi *spi);
> };
>
> /**
> @@ -288,6 +303,8 @@ struct stm32_spi_cfg {
> * @dma_rx: dma channel for RX transfer
> * @phys_addr: SPI registers physical base address
> * @slave_mode: the controller is configured as SPI slave
> + * @slave_udr_mode: slave underrun behavior
> + * @slave_udr_pattern: slave underrun pattern parameter
> */
> struct stm32_spi {
> struct device *dev;
> @@ -317,6 +334,8 @@ struct stm32_spi {
> dma_addr_t phys_addr;
>
> bool slave_mode;
> + u32 slave_udr_mode;
> + u32 slave_udr_pattern;
> };
>
> static const struct stm32_spi_regspec stm32f4_spi_regspec = {
> @@ -921,6 +940,14 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
> end = true;
> }
>
> + if (sr & STM32H7_SPI_SR_UDR) {
> + static DEFINE_RATELIMIT_STATE(rs,
> + DEFAULT_RATELIMIT_INTERVAL * 10,
> + 1);
> + if (__ratelimit(&rs))
> + dev_dbg_ratelimited(spi->dev, "Underrun detected\n");
> + }
> +
> if (sr & STM32H7_SPI_SR_EOT) {
> if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
> stm32h7_spi_read_rxfifo(spi);
> @@ -1178,6 +1205,9 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
> if (spi->tx_buf)
> stm32h7_spi_write_txfifo(spi);
>
> + if (STM32_SPI_SLAVE_MODE(spi) && spi->slave_udr_mode != SPI_NO_ACTION)
> + ier |= STM32H7_SPI_IER_UDRIE;
> +
> if (STM32_SPI_MASTER_MODE(spi))
> stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
>
> @@ -1222,6 +1252,9 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
> if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX)
> ier |= STM32H7_SPI_IER_EOTIE | STM32H7_SPI_IER_TXTFIE;
>
> + if (STM32_SPI_SLAVE_MODE(spi) && spi->slave_udr_mode != SPI_NO_ACTION)
> + ier |= STM32H7_SPI_IER_UDRIE;
> +
> stm32_spi_set_bits(spi, STM32H7_SPI_IER, ier);
>
> stm32_spi_enable(spi);
> @@ -1530,6 +1563,53 @@ static int stm32h7_spi_number_of_data(struct stm32_spi *spi, u32 nb_words)
> return 0;
> }
>
> +/**
> + * stm32h7_spi_set_slave_udr - configure slave underrun detection and reaction
> + * @spi: pointer to the spi controller data structure
> + */
> +static void stm32h7_spi_set_slave_udr(struct stm32_spi *spi)
> +{
> + u32 max_udr_ptrn, udr_ptrn, cfg1_setb = 0;
> +
> + if (spi->slave_udr_mode == SPI_NO_ACTION)
> + return;
> +
> + switch (spi->slave_udr_mode) {
> + case SPI_SEND_PATTERN:
> + max_udr_ptrn = (1 << spi->cur_bpw) - 1;
> + if (spi->slave_udr_pattern > max_udr_ptrn) {
> + udr_ptrn = spi->slave_udr_pattern & max_udr_ptrn;
> + dev_warn(spi->dev,
> + "force slave underrun pattern to data width (> 0x%x, set 0x%x)\n",
> + max_udr_ptrn, udr_ptrn);
> + } else {
> + udr_ptrn = spi->slave_udr_pattern;
> + dev_dbg(spi->dev, "spi slave underrun: send pattern (0x%x)\n",
> + spi->slave_udr_pattern);
> + }
> + writel_relaxed(udr_ptrn, spi->base + STM32H7_SPI_UDRDR);
> + cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_PTRN);
> + break;
> + case SPI_REPEAT_LAST_RECEIVED_DATA:
> + cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_LAST_R);
> + dev_dbg(spi->dev, "spi slave underrun: repeat received data\n");
> + break;
> + case SPI_REPEAT_LAST_TRANSMITTED_DATA:
> + cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRCFG, STM32H7_SPI_CFG1_UDRCFG_LAST_T);
> + dev_dbg(spi->dev, "spi slave underrun: repeat transmitted data\n");
> + break;
> + default:
> + dev_warn(spi->dev, "slave underrun detection disabled\n");
> + spi->slave_udr_mode = SPI_NO_ACTION;
> + }
> +
> + if (spi->slave_udr_mode != SPI_NO_ACTION) {
> + cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_UDRDET, STM32H7_SPI_CFG1_UDRDET_LAST);
> +
> + stm32_spi_set_bits(spi, STM32H7_SPI_CFG1, cfg1_setb);
> + }
> +}
> +
> /**
> * stm32_spi_transfer_one_setup - common setup to transfer a single
> * spi_transfer either using DMA or
> @@ -1591,6 +1671,9 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
> goto out;
> }
>
> + if (STM32_SPI_SLAVE_MODE(spi) && spi->cfg->set_slave_udr)
> + spi->cfg->set_slave_udr(spi);
> +
> dev_dbg(spi->dev, "transfer communication mode set to %d\n",
> spi->cur_comm);
> dev_dbg(spi->dev,
> @@ -1774,6 +1857,7 @@ static const struct stm32_spi_cfg stm32h7_spi_cfg = {
> .baud_rate_div_min = STM32H7_SPI_MBR_DIV_MIN,
> .baud_rate_div_max = STM32H7_SPI_MBR_DIV_MAX,
> .has_fifo = true,
> + .set_slave_udr = stm32h7_spi_set_slave_udr,
> };
>
> static const struct of_device_id stm32_spi_of_match[] = {
> @@ -1789,6 +1873,31 @@ static int stm32h7_spi_slave_abort(struct spi_controller *ctrl)
> return 0;
> }
>
> +static void stm32h7_spi_parse_slave_config(struct stm32_spi *spi, struct device_node *np)
> +{
> + u32 udr_configs[2] = { 0, 0 };
> + int count, ret;
> +
> + count = of_property_count_elems_of_size(np, "st,spi-slave-underrun", sizeof(u32));
> + if (count <= 0) {
> + if (count != -EINVAL)
> + dev_err(spi->dev, "Invalid st,spi-slave-underrun property\n");
> + return;
> + }
> +
> + ret = of_property_read_u32_array(np, "st,spi-slave-underrun", udr_configs, count);
> + if (ret)
> + return;
> +
> + spi->slave_udr_mode = udr_configs[0];
> + if (spi->slave_udr_mode == SPI_SEND_PATTERN) {
> + if (count > 1)
> + spi->slave_udr_pattern = udr_configs[1];
> + else
> + dev_warn(spi->dev, "Missing pattern in st,spi-slave-underrun property\n");
> + }
> +}
> +
> static int stm32_spi_probe(struct platform_device *pdev)
> {
> struct spi_controller *ctrl;
> @@ -1842,6 +1951,9 @@ static int stm32_spi_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (STM32_SPI_SLAVE_MODE(spi))
> + stm32h7_spi_parse_slave_config(spi, np);
> +
> spi->clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(spi->clk)) {
> ret = PTR_ERR(spi->clk);
> --
> 2.25.1
>

Acked-by: Alain Volmat <[email protected]>

Regards,
Alain

2023-05-04 17:12:21

by Valentin Caron

[permalink] [raw]
Subject: Re: [PATCH 1/7] dt-bindings: spi: stm32: add address-cells and size-cells into yaml

Hi Rob,

On 4/28/23 23:41, Rob Herring wrote:
> On Fri, Apr 28, 2023 at 02:15:18PM +0200, Valentin Caron wrote:
>> Theses properties need to be described to satisfy dtbs_check.
> No, they are defined in spi-controller.yaml, so they should not be
> needed here.

Yes, you're right, I cannot remember why I need to add theses properties.

Thank you,
Valentin

>> Signed-off-by: Valentin Caron <[email protected]>
>> ---
>> Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> index 9ca1a843c820..c599eb359d56 100644
>> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> @@ -29,6 +29,9 @@ allOf:
>> st,spi-midi-ns: false
>>
>> properties:
>> + "#address-cells": true
>> + "#size-cells": true
>> +
>> compatible:
>> enum:
>> - st,stm32f4-spi
>> --
>> 2.25.1
>>

2023-05-04 17:32:22

by Valentin Caron

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: spi: stm32: add stm32h7 st,spi-slave-underrun property


On 4/28/23 23:56, Rob Herring wrote:
> On Fri, Apr 28, 2023 at 02:15:23PM +0200, Valentin Caron wrote:
>> This property is used to enable and configure stm32h7 SPI controller to
>> handle underrun that could appear in slave mode.
>>
>> Signed-off-by: Valentin Caron <[email protected]>
>> ---
>> .../devicetree/bindings/spi/st,stm32-spi.yaml | 8 ++++++++
>> MAINTAINERS | 1 +
>> include/dt-bindings/spi/spi-stm32.h | 15 +++++++++++++++
>> 3 files changed, 24 insertions(+)
>> create mode 100644 include/dt-bindings/spi/spi-stm32.h
>>
>> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> index 1d26fa2658c5..e946ea71a247 100644
>> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> @@ -28,6 +28,7 @@ allOf:
>> properties:
>> st,spi-midi-ns: false
>> spi-slave: false
>> + st,spi-slave-underrun: false
>>
>> properties:
>> "#address-cells": true
>> @@ -70,6 +71,13 @@ properties:
>> In case of spi-slave defined, if <0>, indicate that SS should be
>> detected via the dedicated HW pin
>>
>> + st,spi-slave-underrun:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description:
>> + First parameter enables and selects slave underrun reaction.
>> + Refer to "dt-bindings/spi/spi-stm32.h" for the supported values.
>> + Second parameter is the pattern in case of SPI_SEND_PATTERN mode.
> So, max 2 cells? Then:
>
> minItems: 1
> maxItems: 2
>
> Though I don't really think this belongs in DT. The driver implementing
> the SPI slave function defines all the rest of the protocol the slave
> implements. Why not this little bit? Perhaps there is no way for a SPI
> slave driver to tell the SPI controller which controller specific mode
> to use, so you abuse DT to configure the SPI controller. Also, with a
> controller specific response, then the slave driver is coupled to that
> SPI controller which isn't great either.
>
> Rob
I basically made this DT property to configure the controler and doesn't
give attention to the fact that this parameter can came from framework.

I will not rework this functionality, but simply let it down as we has
no demands on it.

Thanks for all your remarks,
Valentin