linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel
@ 2022-08-10  9:32 patrice.chotard
  2022-08-10  9:32 ` [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback patrice.chotard
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: patrice.chotard @ 2022-08-10  9:32 UTC (permalink / raw)
  To: Mark Brown, Alexandre Torgue, robh+dt, krzysztof.kozlowski+dt
  Cc: linux-spi, linux-stm32, linux-arm-kernel, linux-kernel,
	christophe.kerello, patrice.chotard, devicetree

From: Patrice Chotard <patrice.chotard@foss.st.com>


The goal of this series is to allow to use QSPI bus as a 8 lines communication 
channel for specific purpose.

The QSPI block offers the possibility to communicate with 2 flashes in 
parrallel using the dual flash mode, 8 data lines are then used.
Usage of DT parallel-memories property is needed to enable dual flash mode.

The addition of the legacy transfer_one_message() spi callback is also needed
as currently the stm32-qspi driver only supports spi_controller_mem_ops API.


 arch/arm/boot/dts/stm32mp15-pinctrl.dtsi |  50 +++++++----
 arch/arm/boot/dts/stm32mp157c-ev1.dts    |  12 ++-
 drivers/spi/spi-stm32-qspi.c             | 103 +++++++++++++++++++++--
 3 files changed, 139 insertions(+), 26 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10  9:32 [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel patrice.chotard
@ 2022-08-10  9:32 ` patrice.chotard
  2022-08-10 13:06   ` Mark Brown
  2022-08-10  9:32 ` [PATCH v2 2/2] ARM: dts: stm32: Create separate pinmux for qspi cs pin in stm32mp15-pinctrl.dtsi patrice.chotard
  2022-08-22 16:05 ` (subset) [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel Mark Brown
  2 siblings, 1 reply; 11+ messages in thread
From: patrice.chotard @ 2022-08-10  9:32 UTC (permalink / raw)
  To: Mark Brown, Alexandre Torgue
  Cc: linux-spi, linux-stm32, linux-arm-kernel, linux-kernel,
	christophe.kerello, patrice.chotard

From: Patrice Chotard <patrice.chotard@foss.st.com>

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 <patrice.chotard@foss.st.com>

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] ARM: dts: stm32: Create separate pinmux for qspi cs pin in stm32mp15-pinctrl.dtsi
  2022-08-10  9:32 [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel patrice.chotard
  2022-08-10  9:32 ` [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback patrice.chotard
@ 2022-08-10  9:32 ` patrice.chotard
  2022-08-22 16:05 ` (subset) [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel Mark Brown
  2 siblings, 0 replies; 11+ messages in thread
From: patrice.chotard @ 2022-08-10  9:32 UTC (permalink / raw)
  To: Mark Brown, Alexandre Torgue
  Cc: linux-spi, linux-stm32, linux-arm-kernel, linux-kernel,
	christophe.kerello, patrice.chotard

From: Patrice Chotard <patrice.chotard@foss.st.com>

Create a separate pinmux for qspi chip select in stm32mp15-pinctrl.dtsi.
In the case we want to use transfer_one() API to communicate with a SPI
device, chip select signal must be driven individually.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
 arch/arm/boot/dts/stm32mp15-pinctrl.dtsi | 50 ++++++++++++++++--------
 arch/arm/boot/dts/stm32mp157c-ev1.dts    | 12 +++++-
 2 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
index 6052243ad81c..ade4fab45f14 100644
--- a/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
@@ -1189,7 +1189,7 @@ pins {
 	};
 
 	qspi_bk1_pins_a: qspi-bk1-0 {
-		pins1 {
+		pins {
 			pinmux = <STM32_PINMUX('F', 8, AF10)>, /* QSPI_BK1_IO0 */
 				 <STM32_PINMUX('F', 9, AF10)>, /* QSPI_BK1_IO1 */
 				 <STM32_PINMUX('F', 7, AF9)>, /* QSPI_BK1_IO2 */
@@ -1198,12 +1198,6 @@ pins1 {
 			drive-push-pull;
 			slew-rate = <1>;
 		};
-		pins2 {
-			pinmux = <STM32_PINMUX('B', 6, AF10)>; /* QSPI_BK1_NCS */
-			bias-pull-up;
-			drive-push-pull;
-			slew-rate = <1>;
-		};
 	};
 
 	qspi_bk1_sleep_pins_a: qspi-bk1-sleep-0 {
@@ -1211,13 +1205,12 @@ pins {
 			pinmux = <STM32_PINMUX('F', 8, ANALOG)>, /* QSPI_BK1_IO0 */
 				 <STM32_PINMUX('F', 9, ANALOG)>, /* QSPI_BK1_IO1 */
 				 <STM32_PINMUX('F', 7, ANALOG)>, /* QSPI_BK1_IO2 */
-				 <STM32_PINMUX('F', 6, ANALOG)>, /* QSPI_BK1_IO3 */
-				 <STM32_PINMUX('B', 6, ANALOG)>; /* QSPI_BK1_NCS */
+				 <STM32_PINMUX('F', 6, ANALOG)>; /* QSPI_BK1_IO3 */
 		};
 	};
 
 	qspi_bk2_pins_a: qspi-bk2-0 {
-		pins1 {
+		pins {
 			pinmux = <STM32_PINMUX('H', 2, AF9)>, /* QSPI_BK2_IO0 */
 				 <STM32_PINMUX('H', 3, AF9)>, /* QSPI_BK2_IO1 */
 				 <STM32_PINMUX('G', 10, AF11)>, /* QSPI_BK2_IO2 */
@@ -1226,7 +1219,34 @@ pins1 {
 			drive-push-pull;
 			slew-rate = <1>;
 		};
-		pins2 {
+	};
+
+	qspi_bk2_sleep_pins_a: qspi-bk2-sleep-0 {
+		pins {
+			pinmux = <STM32_PINMUX('H', 2, ANALOG)>, /* QSPI_BK2_IO0 */
+				 <STM32_PINMUX('H', 3, ANALOG)>, /* QSPI_BK2_IO1 */
+				 <STM32_PINMUX('G', 10, ANALOG)>, /* QSPI_BK2_IO2 */
+				 <STM32_PINMUX('G', 7, ANALOG)>; /* QSPI_BK2_IO3 */
+		};
+	};
+
+	qspi_cs1_pins_a: qspi-cs1-0 {
+		pins {
+			pinmux = <STM32_PINMUX('B', 6, AF10)>; /* QSPI_BK1_NCS */
+			bias-pull-up;
+			drive-push-pull;
+			slew-rate = <1>;
+		};
+	};
+
+	qspi_cs1_sleep_pins_a: qspi-cs1-sleep-0 {
+		pins {
+			pinmux = <STM32_PINMUX('B', 6, ANALOG)>; /* QSPI_BK1_NCS */
+		};
+	};
+
+	qspi_cs2_pins_a: qspi-cs2-0 {
+		pins {
 			pinmux = <STM32_PINMUX('C', 0, AF10)>; /* QSPI_BK2_NCS */
 			bias-pull-up;
 			drive-push-pull;
@@ -1234,13 +1254,9 @@ pins2 {
 		};
 	};
 
-	qspi_bk2_sleep_pins_a: qspi-bk2-sleep-0 {
+	qspi_cs2_sleep_pins_a: qspi-cs2-sleep-0 {
 		pins {
-			pinmux = <STM32_PINMUX('H', 2, ANALOG)>, /* QSPI_BK2_IO0 */
-				 <STM32_PINMUX('H', 3, ANALOG)>, /* QSPI_BK2_IO1 */
-				 <STM32_PINMUX('G', 10, ANALOG)>, /* QSPI_BK2_IO2 */
-				 <STM32_PINMUX('G', 7, ANALOG)>, /* QSPI_BK2_IO3 */
-				 <STM32_PINMUX('C', 0, ANALOG)>; /* QSPI_BK2_NCS */
+			pinmux = <STM32_PINMUX('C', 0, ANALOG)>; /* QSPI_BK2_NCS */
 		};
 	};
 
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index d142dd30e16b..050c3c27a420 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -255,8 +255,16 @@ &m_can1 {
 
 &qspi {
 	pinctrl-names = "default", "sleep";
-	pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>;
-	pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>;
+	pinctrl-0 = <&qspi_clk_pins_a
+		     &qspi_bk1_pins_a
+		     &qspi_cs1_pins_a
+		     &qspi_bk2_pins_a
+		     &qspi_cs2_pins_a>;
+	pinctrl-1 = <&qspi_clk_sleep_pins_a
+		     &qspi_bk1_sleep_pins_a
+		     &qspi_cs1_sleep_pins_a
+		     &qspi_bk2_sleep_pins_a
+		     &qspi_cs2_sleep_pins_a>;
 	reg = <0x58003000 0x1000>, <0x70000000 0x4000000>;
 	#address-cells = <1>;
 	#size-cells = <0>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10  9:32 ` [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback patrice.chotard
@ 2022-08-10 13:06   ` Mark Brown
  2022-08-10 13:15     ` Patrice CHOTARD
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2022-08-10 13:06 UTC (permalink / raw)
  To: patrice.chotard
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Wed, Aug 10, 2022 at 11:32:14AM +0200, patrice.chotard@foss.st.com wrote:

> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
> 
> 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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:06   ` Mark Brown
@ 2022-08-10 13:15     ` Patrice CHOTARD
  2022-08-10 13:23       ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Patrice CHOTARD @ 2022-08-10 13:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello

Hi Mark

On 8/10/22 15:06, Mark Brown wrote:
> On Wed, Aug 10, 2022 at 11:32:14AM +0200, patrice.chotard@foss.st.com wrote:
> 
>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
>>
>> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:15     ` Patrice CHOTARD
@ 2022-08-10 13:23       ` Mark Brown
  2022-08-10 13:31         ` Patrice CHOTARD
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2022-08-10 13:23 UTC (permalink / raw)
  To: Patrice CHOTARD
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:23       ` Mark Brown
@ 2022-08-10 13:31         ` Patrice CHOTARD
  2022-08-10 13:40           ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Patrice CHOTARD @ 2022-08-10 13:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello



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 ?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:31         ` Patrice CHOTARD
@ 2022-08-10 13:40           ` Mark Brown
  2022-08-10 13:52             ` Patrice CHOTARD
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2022-08-10 13:40 UTC (permalink / raw)
  To: Patrice CHOTARD
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:40           ` Mark Brown
@ 2022-08-10 13:52             ` Patrice CHOTARD
  2022-08-10 13:57               ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Patrice CHOTARD @ 2022-08-10 13:52 UTC (permalink / raw)
  To: Mark Brown
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello



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.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
  2022-08-10 13:52             ` Patrice CHOTARD
@ 2022-08-10 13:57               ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2022-08-10 13:57 UTC (permalink / raw)
  To: Patrice CHOTARD
  Cc: Alexandre Torgue, linux-spi, linux-stm32, linux-arm-kernel,
	linux-kernel, christophe.kerello

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: (subset) [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel
  2022-08-10  9:32 [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel patrice.chotard
  2022-08-10  9:32 ` [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback patrice.chotard
  2022-08-10  9:32 ` [PATCH v2 2/2] ARM: dts: stm32: Create separate pinmux for qspi cs pin in stm32mp15-pinctrl.dtsi patrice.chotard
@ 2022-08-22 16:05 ` Mark Brown
  2 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2022-08-22 16:05 UTC (permalink / raw)
  To: patrice.chotard, krzysztof.kozlowski+dt, Alexandre Torgue, robh+dt
  Cc: christophe.kerello, linux-kernel, linux-spi, devicetree,
	linux-arm-kernel, linux-stm32

On Wed, 10 Aug 2022 11:32:13 +0200, patrice.chotard@foss.st.com wrote:
> From: Patrice Chotard <patrice.chotard@foss.st.com>
> 
> 
> The goal of this series is to allow to use QSPI bus as a 8 lines communication
> channel for specific purpose.
> 
> The QSPI block offers the possibility to communicate with 2 flashes in
> parrallel using the dual flash mode, 8 data lines are then used.
> Usage of DT parallel-memories property is needed to enable dual flash mode.
> 
> [...]

Applied to

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

Thanks!

[1/2] spi: stm32_qspi: Add transfer_one_message() spi callback
      commit: b051161f44d414e736fa2b011245441bae9babd7

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-08-22 16:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-10  9:32 [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel patrice.chotard
2022-08-10  9:32 ` [PATCH v2 1/2] spi: stm32_qspi: Add transfer_one_message() spi callback patrice.chotard
2022-08-10 13:06   ` Mark Brown
2022-08-10 13:15     ` Patrice CHOTARD
2022-08-10 13:23       ` Mark Brown
2022-08-10 13:31         ` Patrice CHOTARD
2022-08-10 13:40           ` Mark Brown
2022-08-10 13:52             ` Patrice CHOTARD
2022-08-10 13:57               ` Mark Brown
2022-08-10  9:32 ` [PATCH v2 2/2] ARM: dts: stm32: Create separate pinmux for qspi cs pin in stm32mp15-pinctrl.dtsi patrice.chotard
2022-08-22 16:05 ` (subset) [PATCH v2 0/2] spi: stm32_qspi: use QSPI bus as 8 lines communication channel Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).