From: Patrice Chotard <[email protected]>
Add transfer_one_message() spi callback in order to use the QSPI interface
as a communication channel using up to 8 qspi lines (QSPI configured
in dual flash mode).
Signed-off-by: Patrice Chotard <[email protected]>
v2: _ use parallel-memories property
_ set auto_runtime_pm to true
_ remove pm_runtime_*() usage in transfer_one_message() callback
---
drivers/spi/spi-stm32-qspi.c | 103 ++++++++++++++++++++++++++++++++---
1 file changed, 96 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c
index f3fe92300639..a649b28ab111 100644
--- a/drivers/spi/spi-stm32-qspi.c
+++ b/drivers/spi/spi-stm32-qspi.c
@@ -15,6 +15,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_gpio.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pm_runtime.h>
#include <linux/platform_device.h>
@@ -355,10 +356,10 @@ static int stm32_qspi_get_mode(u8 buswidth)
return buswidth;
}
-static int stm32_qspi_send(struct spi_mem *mem, const struct spi_mem_op *op)
+static int stm32_qspi_send(struct spi_device *spi, const struct spi_mem_op *op)
{
- struct stm32_qspi *qspi = spi_controller_get_devdata(mem->spi->master);
- struct stm32_qspi_flash *flash = &qspi->flash[mem->spi->chip_select];
+ struct stm32_qspi *qspi = spi_controller_get_devdata(spi->master);
+ struct stm32_qspi_flash *flash = &qspi->flash[spi->chip_select];
u32 ccr, cr;
int timeout, err = 0, err_poll_status = 0;
@@ -465,7 +466,7 @@ static int stm32_qspi_poll_status(struct spi_mem *mem, const struct spi_mem_op *
qspi->fmode = CCR_FMODE_APM;
qspi->status_timeout = timeout_ms;
- ret = stm32_qspi_send(mem, op);
+ ret = stm32_qspi_send(mem->spi, op);
mutex_unlock(&qspi->lock);
pm_runtime_mark_last_busy(qspi->dev);
@@ -489,7 +490,7 @@ static int stm32_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
else
qspi->fmode = CCR_FMODE_INDW;
- ret = stm32_qspi_send(mem, op);
+ ret = stm32_qspi_send(mem->spi, op);
mutex_unlock(&qspi->lock);
pm_runtime_mark_last_busy(qspi->dev);
@@ -545,7 +546,7 @@ static ssize_t stm32_qspi_dirmap_read(struct spi_mem_dirmap_desc *desc,
else
qspi->fmode = CCR_FMODE_INDR;
- ret = stm32_qspi_send(desc->mem, &op);
+ ret = stm32_qspi_send(desc->mem->spi, &op);
mutex_unlock(&qspi->lock);
pm_runtime_mark_last_busy(qspi->dev);
@@ -554,6 +555,81 @@ static ssize_t stm32_qspi_dirmap_read(struct spi_mem_dirmap_desc *desc,
return ret ?: len;
}
+static int stm32_qspi_transfer_one_message(struct spi_controller *ctrl,
+ struct spi_message *msg)
+{
+ struct stm32_qspi *qspi = spi_controller_get_devdata(ctrl);
+ struct spi_transfer *transfer;
+ struct spi_device *spi = msg->spi;
+ struct spi_mem_op op;
+ int ret;
+
+ if (!spi->cs_gpiod)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&qspi->lock);
+
+ gpiod_set_value_cansleep(spi->cs_gpiod, true);
+
+ list_for_each_entry(transfer, &msg->transfers, transfer_list) {
+ u8 dummy_bytes = 0;
+
+ memset(&op, 0, sizeof(op));
+
+ dev_dbg(qspi->dev, "tx_buf:%p tx_nbits:%d rx_buf:%p rx_nbits:%d len:%d dummy_data:%d\n",
+ transfer->tx_buf, transfer->tx_nbits,
+ transfer->rx_buf, transfer->rx_nbits,
+ transfer->len, transfer->dummy_data);
+
+ /*
+ * QSPI hardware supports dummy bytes transfer.
+ * If current transfer is dummy byte, merge it with the next
+ * transfer in order to take into account QSPI block constraint
+ */
+ if (transfer->dummy_data) {
+ op.dummy.buswidth = transfer->tx_nbits;
+ op.dummy.nbytes = transfer->len;
+ dummy_bytes = transfer->len;
+
+ /* if happens, means that message is not correctly built */
+ if (list_is_last(&transfer->transfer_list, &msg->transfers))
+ goto end_of_transfer;
+
+ transfer = list_next_entry(transfer, transfer_list);
+ }
+
+ op.data.nbytes = transfer->len;
+
+ if (transfer->rx_buf) {
+ qspi->fmode = CCR_FMODE_INDR;
+ op.data.buswidth = transfer->rx_nbits;
+ op.data.dir = SPI_MEM_DATA_IN;
+ op.data.buf.in = transfer->rx_buf;
+ } else {
+ qspi->fmode = CCR_FMODE_INDW;
+ op.data.buswidth = transfer->tx_nbits;
+ op.data.dir = SPI_MEM_DATA_OUT;
+ op.data.buf.out = transfer->tx_buf;
+ }
+
+ ret = stm32_qspi_send(spi, &op);
+ if (ret)
+ goto end_of_transfer;
+
+ msg->actual_length += transfer->len + dummy_bytes;
+ }
+
+end_of_transfer:
+ gpiod_set_value_cansleep(spi->cs_gpiod, false);
+
+ mutex_unlock(&qspi->lock);
+
+ msg->status = ret;
+ spi_finalize_current_message(ctrl);
+
+ return ret;
+}
+
static int stm32_qspi_setup(struct spi_device *spi)
{
struct spi_controller *ctrl = spi->master;
@@ -579,7 +655,7 @@ static int stm32_qspi_setup(struct spi_device *spi)
flash->presc = presc;
mutex_lock(&qspi->lock);
- qspi->cr_reg = CR_APMS | 3 << CR_FTHRES_SHIFT | CR_SSHIFT | CR_EN;
+ qspi->cr_reg |= CR_APMS | 3 << CR_FTHRES_SHIFT | CR_SSHIFT | CR_EN;
writel_relaxed(qspi->cr_reg, qspi->io_base + QSPI_CR);
/* set dcr fsize to max address */
@@ -741,11 +817,24 @@ static int stm32_qspi_probe(struct platform_device *pdev)
mutex_init(&qspi->lock);
+ /*
+ * Dual flash mode is only enable in case "parallel-memories" and
+ * "cs-gpios" properties are found in DT
+ */
+ if (of_property_read_bool(dev->of_node, "parallel-memories") &&
+ of_gpio_named_count(dev->of_node, "cs-gpios")) {
+ qspi->cr_reg = CR_DFM;
+ dev_dbg(dev, "Dual flash mode enable");
+ }
+
ctrl->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD
| SPI_TX_DUAL | SPI_TX_QUAD;
ctrl->setup = stm32_qspi_setup;
ctrl->bus_num = -1;
ctrl->mem_ops = &stm32_qspi_mem_ops;
+ ctrl->use_gpio_descriptors = true;
+ ctrl->transfer_one_message = stm32_qspi_transfer_one_message;
+ ctrl->auto_runtime_pm = true;
ctrl->num_chipselect = STM32_QSPI_MAX_NORCHIP;
ctrl->dev.of_node = dev->of_node;
--
2.25.1
Hi Mark
On 8/10/22 15:06, Mark Brown wrote:
> On Wed, Aug 10, 2022 at 11:32:14AM +0200, [email protected] wrote:
>
>> Signed-off-by: Patrice Chotard <[email protected]>
>>
>> v2: _ use parallel-memories property
>> _ set auto_runtime_pm to true
>> _ remove pm_runtime_*() usage in transfer_one_message() callback
>> ---
>
> The changelog should come after the --- so that it gets automatically
> stripped out by tooling. No need to resend just for this.
>
>> + /*
>> + * Dual flash mode is only enable in case "parallel-memories" and
>> + * "cs-gpios" properties are found in DT
>> + */
>> + if (of_property_read_bool(dev->of_node, "parallel-memories") &&
>> + of_gpio_named_count(dev->of_node, "cs-gpios")) {
>> + qspi->cr_reg = CR_DFM;
>> + dev_dbg(dev, "Dual flash mode enable");
>> + }
>
> Do we need to add something to the DT bindings to indicate that
> parallel-memories is valid?
You mean in the st,stm32-qspi.yaml DT binding file ? Right i think it could be preferable to add it.
Patrice
On Wed, Aug 10, 2022 at 03:15:08PM +0200, Patrice CHOTARD wrote:
> On 8/10/22 15:06, Mark Brown wrote:
> > Do we need to add something to the DT bindings to indicate that
> > parallel-memories is valid?
> You mean in the st,stm32-qspi.yaml DT binding file ? Right i think it could be preferable to add it.
Yes. Though I'm not clear if the bindings actually want to enforce it
there, it's a device level property not a controller level one so it
might not be something where controller support gets validated.
On 8/10/22 15:23, Mark Brown wrote:
> On Wed, Aug 10, 2022 at 03:15:08PM +0200, Patrice CHOTARD wrote:
>> On 8/10/22 15:06, Mark Brown wrote:
>
>>> Do we need to add something to the DT bindings to indicate that
>>> parallel-memories is valid?
>
>> You mean in the st,stm32-qspi.yaml DT binding file ? Right i think it could be preferable to add it.
>
> Yes. Though I'm not clear if the bindings actually want to enforce it
> there, it's a device level property not a controller level one so it
> might not be something where controller support gets validated.
Ah yes, i see, parallel-memories should not be used in our qspi controller node.
So i can't reuse parallel-memories for my purpose.
So i need to add a new proprietary property at controller level as done in the v1 ?
On Wed, Aug 10, 2022 at 03:52:39PM +0200, Patrice CHOTARD wrote:
> On 8/10/22 15:40, Mark Brown wrote:
> > Can't the controller figure this out by looking at the properties of the
> > connected devices? You'd need to just return an error if we ever
> > triggered transfer_one_message() on a device that can't support the
> > operation.
> It should be a solution.
> I just noticed another point, property parallel-memories is an array of uint64 which represent device's size.
> In case a FPGA is connected to the qspi 8 line bus, parallel-memories property will be set with what ?
> simply random value to make dtbs_check happy ?
> IMHO, adding a new proprietary property would be cleaner.
I tend to agree that this is all rather unclear for things that aren't
actually storage.
On Wed, Aug 10, 2022 at 11:32:14AM +0200, [email protected] wrote:
> Signed-off-by: Patrice Chotard <[email protected]>
>
> v2: _ use parallel-memories property
> _ set auto_runtime_pm to true
> _ remove pm_runtime_*() usage in transfer_one_message() callback
> ---
The changelog should come after the --- so that it gets automatically
stripped out by tooling. No need to resend just for this.
> + /*
> + * Dual flash mode is only enable in case "parallel-memories" and
> + * "cs-gpios" properties are found in DT
> + */
> + if (of_property_read_bool(dev->of_node, "parallel-memories") &&
> + of_gpio_named_count(dev->of_node, "cs-gpios")) {
> + qspi->cr_reg = CR_DFM;
> + dev_dbg(dev, "Dual flash mode enable");
> + }
Do we need to add something to the DT bindings to indicate that
parallel-memories is valid?
On 8/10/22 15:40, Mark Brown wrote:
> On Wed, Aug 10, 2022 at 03:31:59PM +0200, Patrice CHOTARD wrote:
>> On 8/10/22 15:23, Mark Brown wrote:
>>> On Wed, Aug 10, 2022 at 03:15:08PM +0200, Patrice CHOTARD wrote:
>
>>> Yes. Though I'm not clear if the bindings actually want to enforce it
>>> there, it's a device level property not a controller level one so it
>>> might not be something where controller support gets validated.
>
>> Ah yes, i see, parallel-memories should not be used in our qspi controller node.
>> So i can't reuse parallel-memories for my purpose.
>
>> So i need to add a new proprietary property at controller level as done in the v1 ?
>
> Can't the controller figure this out by looking at the properties of the
> connected devices? You'd need to just return an error if we ever
> triggered transfer_one_message() on a device that can't support the
> operation.
It should be a solution.
I just noticed another point, property parallel-memories is an array of uint64 which represent device's size.
In case a FPGA is connected to the qspi 8 line bus, parallel-memories property will be set with what ?
simply random value to make dtbs_check happy ?
IMHO, adding a new proprietary property would be cleaner.
On Wed, Aug 10, 2022 at 03:31:59PM +0200, Patrice CHOTARD wrote:
> On 8/10/22 15:23, Mark Brown wrote:
> > On Wed, Aug 10, 2022 at 03:15:08PM +0200, Patrice CHOTARD wrote:
> > Yes. Though I'm not clear if the bindings actually want to enforce it
> > there, it's a device level property not a controller level one so it
> > might not be something where controller support gets validated.
> Ah yes, i see, parallel-memories should not be used in our qspi controller node.
> So i can't reuse parallel-memories for my purpose.
> So i need to add a new proprietary property at controller level as done in the v1 ?
Can't the controller figure this out by looking at the properties of the
connected devices? You'd need to just return an error if we ever
triggered transfer_one_message() on a device that can't support the
operation.