Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1193884 > unrolled thread

[RFC PATCH 0/5] Add memory mapped read support for TI QSPI.

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/5] Add memory mapped read support for TI QSPI. Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200
    [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200
      Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries  for qspi Vignesh R <vigneshr@ti.com> - 2015-08-03 07:10 +0200
        Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries  for qspi Mark Brown <broonie@kernel.org> - 2015-08-04 18:00 +0200
      Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries  for qspi Vignesh R <vigneshr@ti.com> - 2015-08-03 07:10 +0200
      Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries  for qspi Vignesh R <vigneshr@ti.com> - 2015-08-03 07:20 +0200
    [RFC PATCH 2/5] spi: spi-ti-qspi: Add memory mapped read support Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200
    [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200
      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-03 07:00 +0200
        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-04 18:00 +0200
          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read "R, Vignesh" <vigneshr@ti.com> - 2015-08-04 20:10 +0200
            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 07:30 +0200
              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-05 07:40 +0200
                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 08:00 +0200
            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-05 14:00 +0200
              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 14:50 +0200
                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-05 14:50 +0200
                  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 15:00 +0200
                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 11:10 +0200
                      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 12:10 +0200
                        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 12:30 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 13:10 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 13:10 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-06 14:30 +0200
                            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 16:00 +0200
                              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Geert Uytterhoeven <geert@linux-m68k.org> - 2015-08-06 18:20 +0200
                                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 20:30 +0200
                                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 23:40 +0200
                                  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-07 09:40 +0200
                                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-07 10:40 +0200
                                  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Martin Sperl <martin@sperl.org> - 2015-08-07 10:30 +0200
                                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-07 12:20 +0200
                                      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-12 11:30 +0200
                              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 18:50 +0200
                            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 20:30 +0200
                        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 13:30 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 13:50 +0200
                            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 18:10 +0200
    [RFC PATCH 5/5] ARM: dts: AM4372: Add memory map region entries for qspi Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200
    [RFC PATCH 3/5] mtd: devices: m25p80: set flag to request memory mapped read Vignesh R <vigneshr@ti.com> - 2015-07-28 10:50 +0200

Page 1 of 2  [1] 2  Next page →

#1193884 — [RFC PATCH 0/5] Add memory mapped read support for TI QSPI.

This patch series adds support for memory mapped reads for TI QSPI
driver.

TI QSPI controller has memory mapped port (SFI translator interface [1])
through which SPI flash memories can be read using memcpy call. SFI
translator takes care of generating appropriate SPI signals to read data
from flash. This interface works only with SPI flash memories and cannot
be used with other SPI devices. To use memory mapped port, the
controller is switched to memory mapped interface by writing to
QSPI_SPI_SWITCH_REG. The read_opcode, read mode, dummy bytes are set in
QSPI_SPI_SETUPx_REG. Once switched, the SPI flash is available to SoC to
read at specific address. This interface is disabled once memory mapped
read is complete. For write, erase and interaction with non-flash SPI
devices normal SPI interface is used. 

The m25p80 driver sets use_mmap_read flag in spi-message struct passed
to spi-ti-qspi so as to indicate the read request is from mtd layer.
spi-ti-qspi driver switches to memory mapped mode and does memcpy based
on use_mmap_read flag.

The read performace increased from ~100kB/s to ~2.5MB/s on DRA74 EVM.

Tested on DRA74 EVM with spansion S25FL256S flash.
Tested on AM437x sk evm with macronix MX66l51235l flash.

[1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf Section 24.5.4 QSPI
Functional Description

Vignesh R (5):
  spi: introduce flag for memory mapped read
  spi: spi-ti-qspi: Add memory mapped read support
  mtd: devices: m25p80: set flag to request memory mapped read
  ARM: dts: DRA7: Add memory map region entries for qspi
  ARM: dts: AM4372: Add memory map region entries for qspi

 arch/arm/boot/dts/am4372.dtsi |   4 +-
 arch/arm/boot/dts/dra7.dtsi   |   6 +-
 drivers/mtd/devices/m25p80.c  |   3 +
 drivers/spi/spi-ti-qspi.c     | 129 ++++++++++++++++++++++++++++++++++++++++--
 include/linux/spi/spi.h       |   3 +
 5 files changed, 138 insertions(+), 7 deletions(-)

-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1193887 — [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi

Add qspi memory mapped region entries for DRA7xx based SoCs.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ade28c790f4b..5317a0f24ab9 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -902,7 +902,9 @@
 
 		qspi: qspi@47900000 {
 			compatible = "ti,am4372-qspi";
-			reg = <0x47900000 0x100>;
+			reg = <0x47900000 0x100>,
+			      <0x30000000 0x3ffffff>;
+			reg-names = "qspi_base", "qspi_mmap";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "qspi";
-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198547 — Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi

On 07/31/2015 11:49 PM, Mark Brown wrote:
> On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
>>  		qspi: qspi@47900000 {
>>  			compatible = "ti,am4372-qspi";
>> -			reg = <0x47900000 0x100>;
>> +			reg = <0x47900000 0x100>,
>> +			      <0x30000000 0x3ffffff>;
>> +			reg-names = "qspi_base", "qspi_mmap";
> 
> The DT binding document for the controller needs to be extended to
> document this change in the binding.
> 

DT bindings are already documented at
Documentation/devicetree/bindings/spi/ti_qspi.txt. Did you mean to add
this node as an example in that file?

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200114 — Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi

[Multipart message — attachments visible in raw view] — view raw

On Mon, Aug 03, 2015 at 10:32:09AM +0530, Vignesh R wrote:
> On 07/31/2015 11:49 PM, Mark Brown wrote:

> >> -			reg = <0x47900000 0x100>;
> >> +			reg = <0x47900000 0x100>,
> >> +			      <0x30000000 0x3ffffff>;
> >> +			reg-names = "qspi_base", "qspi_mmap";

> > The DT binding document for the controller needs to be extended to
> > document this change in the binding.

> DT bindings are already documented at
> Documentation/devicetree/bindings/spi/ti_qspi.txt. Did you mean to add
> this node as an example in that file?

No, I mean you're changing the binding to add this new memory region -
that new region needs to be added to the documentation.

[toc] | [prev] | [next] | [standalone]

#1198549 — Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi

On 08/01/2015 02:58 AM, Brian Norris wrote:
> On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index ade28c790f4b..5317a0f24ab9 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -902,7 +902,9 @@
>>  
>>  		qspi: qspi@47900000 {
>>  			compatible = "ti,am4372-qspi";
>> -			reg = <0x47900000 0x100>;
>> +			reg = <0x47900000 0x100>,
>> +			      <0x30000000 0x3ffffff>;
> 
> Are you sure this region is 1 byte shy of 64MB in length? Same question
> for patch 5.
> 

Oops, my bad... Its 64MB in length, I entered offset of last byte
instead of length. Will correct this in the actual patch.

>> +			reg-names = "qspi_base", "qspi_mmap";
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>>  			ti,hwmods = "qspi";
>> -- 
>> 2.4.6
>>

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198550 — Re: [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi

On 07/31/2015 07:18 PM, Sekhar Nori wrote:
> On Tuesday 28 July 2015 02:11 PM, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index ade28c790f4b..5317a0f24ab9 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
> 
> The patch talks about DRA7x in subject and description but you modify
> AM437x here. You have got commit text mixed up between 4/5 and 5/5.
> 
> Probably not the kind of feedback you are looking for an RFC, but since
> I noticed it..

Oh, my bad.. I will correct $subject when I submit actual patch series.

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193888 — [RFC PATCH 2/5] spi: spi-ti-qspi: Add memory mapped read support

TI QSPI controller has memory mapped port through which SPI flash
memories can be read using memcpy call. This patch adds support for
memory mapped read based on use_mmap_read flag.
When use_mmap_read flag is set, the controller is switched to memory
mapped interface by writing to QSPI_SPI_SWITCH_REG. The read_opcode,
read mode, dummy bytes are configured in QSPI_SPI_SETUPx_REG, then
memcpy is called to copy the requested data from flash to the rx_buf.
With this patch, the read speed increased from ~100kB/s to ~2.5MB/s on
DRA74 EVM.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/spi-ti-qspi.c | 129 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 125 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 5c0616870358..45844a227c5e 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -71,11 +71,8 @@ struct ti_qspi {
 #define QSPI_SPI_CMD_REG		(0x48)
 #define QSPI_SPI_STATUS_REG		(0x4c)
 #define QSPI_SPI_DATA_REG		(0x50)
-#define QSPI_SPI_SETUP0_REG		(0x54)
+#define QSPI_SPI_SETUP_REG(n)		(0x54 + 4 * n)
 #define QSPI_SPI_SWITCH_REG		(0x64)
-#define QSPI_SPI_SETUP1_REG		(0x58)
-#define QSPI_SPI_SETUP2_REG		(0x5c)
-#define QSPI_SPI_SETUP3_REG		(0x60)
 #define QSPI_SPI_DATA_REG_1		(0x68)
 #define QSPI_SPI_DATA_REG_2		(0x6c)
 #define QSPI_SPI_DATA_REG_3		(0x70)
@@ -118,6 +115,16 @@ struct ti_qspi {
 
 #define QSPI_AUTOSUSPEND_TIMEOUT         2000
 
+#define MEM_CS_EN(n)			((n + 1) << 8)
+
+#define MM_SWITCH			0x1
+
+#define QSPI_SETUP_RD_NORMAL		(0x0 << 12)
+#define QSPI_SETUP_RD_DUAL		(0x1 << 12)
+#define QSPI_SETUP_RD_QUAD		(0x3 << 12)
+#define QSPI_SETUP_ADDR_SHIFT		8
+#define QSPI_SETUP_DUMMY_SHIFT		10
+
 static inline unsigned long ti_qspi_read(struct ti_qspi *qspi,
 		unsigned long reg)
 {
@@ -335,6 +342,117 @@ static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
 	return 0;
 }
 
+static void ti_qspi_enable_memory_map(struct spi_device *spi)
+{
+	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
+	u32 val;
+
+	ti_qspi_write(qspi, MM_SWITCH, QSPI_SPI_SWITCH_REG);
+	if (qspi->ctrl_mod) {
+		val = readl(qspi->ctrl_base);
+		val |= MEM_CS_EN(spi->chip_select);
+		writel(val, qspi->ctrl_base);
+	}
+}
+
+static void ti_qspi_disable_memory_map(struct spi_device *spi)
+{
+	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
+	u32 val;
+
+	ti_qspi_write(qspi, 0, QSPI_SPI_SWITCH_REG);
+	if (qspi->ctrl_mod) {
+		val = readl(qspi->ctrl_base);
+		val &= ~MEM_CS_EN(spi->chip_select);
+		writel(val, qspi->ctrl_base);
+	}
+}
+
+static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8
+				    read_opcode, u8 addr_width,
+				    u8 dummy_bytes)
+{
+	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
+	u32 mode = spi->mode & (SPI_RX_DUAL | SPI_RX_QUAD);
+	u32 memval = read_opcode;
+
+	switch (mode) {
+	case SPI_RX_QUAD:
+		memval |= QSPI_SETUP_RD_QUAD;
+		break;
+	case SPI_RX_DUAL:
+		memval |= QSPI_SETUP_RD_DUAL;
+		break;
+	default:
+		memval |= QSPI_SETUP_RD_NORMAL;
+		break;
+	}
+	memval |= ((addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
+		   dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
+	ti_qspi_write(qspi, memval,
+		      QSPI_SPI_SETUP_REG(spi->chip_select));
+}
+
+static unsigned int ti_qspi_cmd2addr(u8 *cmd, u8 addr_width)
+{
+	u32 addr;
+
+	/* cmd[0] is read opcode */
+	addr = cmd[1] << ((addr_width - 1) * 8);
+	addr |= cmd[2] << ((addr_width - 2) * 8);
+	addr |= cmd[3] << ((addr_width - 3) * 8);
+	addr |= cmd[4] << ((addr_width - 4) * 8);
+
+	return addr;
+}
+
+static int ti_qspi_mmap_read(struct spi_master *master,
+			     struct spi_message *m)
+{
+	struct ti_qspi *qspi = spi_master_get_devdata(master);
+	struct spi_device *spi = m->spi;
+	struct spi_transfer *t;
+	u8 read_opcode = 0x3;	/* Default normal read */
+	void *to = NULL;
+	u8 addr_width = 4, dummy_bytes = 0;
+	unsigned int len = 0, from = 0;
+	int status = 0;
+
+	mutex_lock(&qspi->list_lock);
+
+	/* disable WC interrupt during memcpy */
+	ti_qspi_write(qspi, QSPI_WC_INT_DISABLE, QSPI_INTR_ENABLE_CLEAR_REG);
+	ti_qspi_enable_memory_map(spi);
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->tx_buf) {
+			read_opcode = *((u8 *)t->tx_buf);
+			dummy_bytes = t->len - (addr_width + 1);
+			from = ti_qspi_cmd2addr((u8 *)t->tx_buf, addr_width);
+		}
+		if (t->rx_buf) {
+			to = ((void *)t->rx_buf);
+			len = t->len;
+		}
+		m->actual_length += t->len;
+	}
+	ti_qspi_setup_mmap_read(spi, read_opcode, addr_width,
+				dummy_bytes);
+
+	if (qspi_is_busy(qspi)) {
+		status = -EBUSY;
+		goto err;
+	}
+	memcpy(to, qspi->mmap_base + from, len);
+
+err:
+	ti_qspi_disable_memory_map(spi);
+	mutex_unlock(&qspi->list_lock);
+	m->status = status;
+	spi_finalize_current_message(master);
+
+	return status;
+}
+
 static int ti_qspi_start_transfer_one(struct spi_master *master,
 		struct spi_message *m)
 {
@@ -344,6 +462,9 @@ static int ti_qspi_start_transfer_one(struct spi_master *master,
 	int status = 0, ret;
 	int frame_length;
 
+	if (m->use_mmap_mode)
+		return ti_qspi_mmap_read(master, m);
+
 	/* setup device control reg */
 	qspi->dc = 0;
 
-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193893 — [RFC PATCH 1/5] spi: introduce flag for memory mapped read

TI QSPI controller has SFI translator which exposes entire flash memory
as memory mapped region for read. With this interface, the CPU
can copy data from flash using normal memcpy call. SFI translator
takes care of generating appropriate SPI signals to read data from
flash. This interface works only with SPI flash memories and cannot be
used with other SPI devices.
Introduce use_mmap_read field in spi_message struct. This can be set by
mtd devices (m25p80) to indicate to spi-master (ti-qspi) to perform
memory mapped read. This helps to distinguish whether the spi-message is
from mtd layer(hence mmap read is possible) or by other spi devices.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 include/linux/spi/spi.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index d673072346f2..f1a0329ee63f 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -640,6 +640,8 @@ struct spi_transfer {
  * @actual_length: the total number of bytes that were transferred in all
  *	successful segments
  * @status: zero for success, else negative errno
+ * @use_mmap_mode: Indicate to spi master to perform memory mapped
+ *	read if possible.
  * @queue: for use by whichever driver currently owns the message
  * @state: for use by whichever driver currently owns the message
  *
@@ -681,6 +683,7 @@ struct spi_message {
 	unsigned		frame_length;
 	unsigned		actual_length;
 	int			status;
+	bool			use_mmap_mode;
 
 	/* for optional use by whatever driver currently owns the
 	 * spi_message ...  between calls to spi_async and then later
-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198538 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

Hi,

On 7/31/2015 11:47 PM, Mark Brown wrote:
> On Tue, Jul 28, 2015 at 02:11:12PM +0530, Vignesh R wrote:
> 
>> Introduce use_mmap_read field in spi_message struct. This can be set by
>> mtd devices (m25p80) to indicate to spi-master (ti-qspi) to perform
>> memory mapped read. This helps to distinguish whether the spi-message is
>> from mtd layer(hence mmap read is possible) or by other spi devices.
> 
> Based on this description and...
> 
>> + * @use_mmap_mode: Indicate to spi master to perform memory mapped
>> + *	read if possible.
> 
> ...the internal documentation I unable to tell what is meant by "perform
> a memory mapped read", at least to the extent where it is visible
> outside of the driver.  This means we can't really use this as a generic
> API since other people won't be able to tell what it does.
> 


Will the following documentation provide better idea regarding the flag:

@use_mmap_mode: Some SPI controller chips are optimized for interacting
with serial flash memories. These chips have memory mapped interface,
through which entire serial flash memory slave can be read/written as if
though they are physical memories (like RAM). Using this interface,
flash can be accessed using memcpy() function and the spi controller
hardware will take care of communicating with serial flash over SPI.
Setting this flag will indicate the SPI controller driver that the
spi_message is from mtd layer to read from/write to flash. The SPI
master driver can then appropriately switch the controller to memory
mapped interface to read from/write to flash, based on this flag (See
drivers/spi/spi-ti-qspi.c for example).
NOTE: If the SPI controller chip lacks memory mapped interface, then the
driver will ignore this flag and use normal SPI protocol to read
from/write to flash. Communication with non-flash SPI devices is not
possible using the memory mapped interface.

I can update the patch commit message and documentation accordingly?

Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200116 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

[Multipart message — attachments visible in raw view] — view raw

On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:

> @use_mmap_mode: Some SPI controller chips are optimized for interacting
> with serial flash memories. These chips have memory mapped interface,
> through which entire serial flash memory slave can be read/written as if
> though they are physical memories (like RAM). Using this interface,
> flash can be accessed using memcpy() function and the spi controller
> hardware will take care of communicating with serial flash over SPI.
> Setting this flag will indicate the SPI controller driver that the
> spi_message is from mtd layer to read from/write to flash. The SPI
> master driver can then appropriately switch the controller to memory
> mapped interface to read from/write to flash, based on this flag (See
> drivers/spi/spi-ti-qspi.c for example).
> NOTE: If the SPI controller chip lacks memory mapped interface, then the
> driver will ignore this flag and use normal SPI protocol to read
> from/write to flash. Communication with non-flash SPI devices is not
> possible using the memory mapped interface.

I still can't tell from the above what this interface is supposed to do.
It sounds like the use of memory mapped mode is supposed to be
transparent to users, it should just affect how the controller interacts
with the hardware, but if that's the case why do we need to expose it to
users at all?  Shouldn't the driver just use memory mapped mode if it's
faster?

[toc] | [prev] | [next] | [standalone]

#1200198 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 8/4/2015 9:21 PM, Mark Brown wrote:
> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
> 
>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>> with serial flash memories. These chips have memory mapped interface,
>> through which entire serial flash memory slave can be read/written as if
>> though they are physical memories (like RAM). Using this interface,
>> flash can be accessed using memcpy() function and the spi controller
>> hardware will take care of communicating with serial flash over SPI.
>> Setting this flag will indicate the SPI controller driver that the
>> spi_message is from mtd layer to read from/write to flash. The SPI
>> master driver can then appropriately switch the controller to memory
>> mapped interface to read from/write to flash, based on this flag (See
>> drivers/spi/spi-ti-qspi.c for example).
>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>> driver will ignore this flag and use normal SPI protocol to read
>> from/write to flash. Communication with non-flash SPI devices is not
>> possible using the memory mapped interface.
> 
> I still can't tell from the above what this interface is supposed to do.
> It sounds like the use of memory mapped mode is supposed to be
> transparent to users, it should just affect how the controller interacts
> with the hardware, but if that's the case why do we need to expose it to
> users at all?  Shouldn't the driver just use memory mapped mode if it's
> faster?
> 

TI QSPI controller has two blocks:
1. SPI_CORE: This is generic(normal) spi mode. This can be used to
communicate with any SPI devices (serial flashes as well as non-flash
devices like touchscreen).
2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
allows reading and writing to an SPI flash device only. Used to speed up
flash reads. It _cannot_ be used to communicate with non flash devices.
Now, the spi_message that ti-qspi receives in transfer_one() callback
can be from mtd device(in which case SFI_MM_IF can be used) or from any
other non flash SPI device (in which case SFI_MM_IF must not be used
instead SPI_CORE is to be used) but there is no way(is there?) to
distinguish where spi_message is from. Therefore I introduced flag
(use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
this helps the ti-qspi driver to determine that the user is flash device
and thus can do read via SFI_MM_IF. If this flag is not set then the
user is assumed to be non flash SPI driver and will use SPI_CORE block
to communicate.

On the whole, I just need a way to determine that the user is a flash
device in order to switch to memory mapped interface.

Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200418 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

Hello,

On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>
>
> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>
>>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>>> with serial flash memories. These chips have memory mapped interface,
>>> through which entire serial flash memory slave can be read/written as if
>>> though they are physical memories (like RAM). Using this interface,
>>> flash can be accessed using memcpy() function and the spi controller
>>> hardware will take care of communicating with serial flash over SPI.
>>> Setting this flag will indicate the SPI controller driver that the
>>> spi_message is from mtd layer to read from/write to flash. The SPI
>>> master driver can then appropriately switch the controller to memory
>>> mapped interface to read from/write to flash, based on this flag (See
>>> drivers/spi/spi-ti-qspi.c for example).
>>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>>> driver will ignore this flag and use normal SPI protocol to read
>>> from/write to flash. Communication with non-flash SPI devices is not
>>> possible using the memory mapped interface.
>>
>> I still can't tell from the above what this interface is supposed to do.
>> It sounds like the use of memory mapped mode is supposed to be
>> transparent to users, it should just affect how the controller interacts
>> with the hardware, but if that's the case why do we need to expose it to
>> users at all?  Shouldn't the driver just use memory mapped mode if it's
>> faster?
>>
>
> TI QSPI controller has two blocks:
> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
> communicate with any SPI devices (serial flashes as well as non-flash
> devices like touchscreen).
> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
> allows reading and writing to an SPI flash device only. Used to speed up
> flash reads. It _cannot_ be used to communicate with non flash devices.
> Now, the spi_message that ti-qspi receives in transfer_one() callback
> can be from mtd device(in which case SFI_MM_IF can be used) or from any
> other non flash SPI device (in which case SFI_MM_IF must not be used
> instead SPI_CORE is to be used) but there is no way(is there?) to
> distinguish where spi_message is from. Therefore I introduced flag
> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
> this helps the ti-qspi driver to determine that the user is flash device
> and thus can do read via SFI_MM_IF. If this flag is not set then the
> user is assumed to be non flash SPI driver and will use SPI_CORE block
> to communicate.
>
> On the whole, I just need a way to determine that the user is a flash
> device in order to switch to memory mapped interface.
>

Maybe it can be set on the SPI slave rather than each message.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200422 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 08/05/2015 10:51 AM, Michal Suchanek wrote:
> Hello,
> 
> On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>>
>>
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>>
>>>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>>>> with serial flash memories. These chips have memory mapped interface,
>>>> through which entire serial flash memory slave can be read/written as if
>>>> though they are physical memories (like RAM). Using this interface,
>>>> flash can be accessed using memcpy() function and the spi controller
>>>> hardware will take care of communicating with serial flash over SPI.
>>>> Setting this flag will indicate the SPI controller driver that the
>>>> spi_message is from mtd layer to read from/write to flash. The SPI
>>>> master driver can then appropriately switch the controller to memory
>>>> mapped interface to read from/write to flash, based on this flag (See
>>>> drivers/spi/spi-ti-qspi.c for example).
>>>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>>>> driver will ignore this flag and use normal SPI protocol to read
>>>> from/write to flash. Communication with non-flash SPI devices is not
>>>> possible using the memory mapped interface.
>>>
>>> I still can't tell from the above what this interface is supposed to do.
>>> It sounds like the use of memory mapped mode is supposed to be
>>> transparent to users, it should just affect how the controller interacts
>>> with the hardware, but if that's the case why do we need to expose it to
>>> users at all?  Shouldn't the driver just use memory mapped mode if it's
>>> faster?
>>>
>>
>> TI QSPI controller has two blocks:
>> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
>> communicate with any SPI devices (serial flashes as well as non-flash
>> devices like touchscreen).
>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>> allows reading and writing to an SPI flash device only. Used to speed up
>> flash reads. It _cannot_ be used to communicate with non flash devices.
>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>> other non flash SPI device (in which case SFI_MM_IF must not be used
>> instead SPI_CORE is to be used) but there is no way(is there?) to
>> distinguish where spi_message is from. Therefore I introduced flag
>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>> this helps the ti-qspi driver to determine that the user is flash device
>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>> to communicate.
>>
>> On the whole, I just need a way to determine that the user is a flash
>> device in order to switch to memory mapped interface.
>>
> 
> Maybe it can be set on the SPI slave rather than each message.

You mean to add flag to spi_device struct? That's ok for me.

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200426 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 5 August 2015 at 07:35, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On 08/05/2015 10:51 AM, Michal Suchanek wrote:
>> Hello,
>>
>> On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>>>
>>>
>>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>>>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>>>

>>>
>>> TI QSPI controller has two blocks:
>>> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
>>> communicate with any SPI devices (serial flashes as well as non-flash
>>> devices like touchscreen).
>>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>>> allows reading and writing to an SPI flash device only. Used to speed up
>>> flash reads. It _cannot_ be used to communicate with non flash devices.
>>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>>> other non flash SPI device (in which case SFI_MM_IF must not be used
>>> instead SPI_CORE is to be used) but there is no way(is there?) to
>>> distinguish where spi_message is from. Therefore I introduced flag
>>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>>> this helps the ti-qspi driver to determine that the user is flash device
>>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>>> to communicate.
>>>
>>> On the whole, I just need a way to determine that the user is a flash
>>> device in order to switch to memory mapped interface.
>>>
>>
>> Maybe it can be set on the SPI slave rather than each message.
>
> You mean to add flag to spi_device struct? That's ok for me.
>

There are already mode flags so you can just add one more.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200664 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

[Multipart message — attachments visible in raw view] — view raw

On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote:
> On 8/4/2015 9:21 PM, Mark Brown wrote:
> > On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:

> > I still can't tell from the above what this interface is supposed to do.
> > It sounds like the use of memory mapped mode is supposed to be
> > transparent to users, it should just affect how the controller interacts
> > with the hardware, but if that's the case why do we need to expose it to
> > users at all?  Shouldn't the driver just use memory mapped mode if it's
> > faster?

> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
> allows reading and writing to an SPI flash device only. Used to speed up
> flash reads. It _cannot_ be used to communicate with non flash devices.
> Now, the spi_message that ti-qspi receives in transfer_one() callback
> can be from mtd device(in which case SFI_MM_IF can be used) or from any
> other non flash SPI device (in which case SFI_MM_IF must not be used
> instead SPI_CORE is to be used) but there is no way(is there?) to
> distinguish where spi_message is from. Therefore I introduced flag
> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
> this helps the ti-qspi driver to determine that the user is flash device
> and thus can do read via SFI_MM_IF. If this flag is not set then the
> user is assumed to be non flash SPI driver and will use SPI_CORE block
> to communicate.

So if you're trying to do this you need to document it adequately so
that other people can understand what it is supposed to do and how to
use and implement it.  People can't really tell how the interface is
supposed to work based on what was in the patch and the above isn't
really helping.  For example, how does this change or restrict what the
contents of the spi_message are?

> On the whole, I just need a way to determine that the user is a flash
> device in order to switch to memory mapped interface.

As far as I can tell you want to set a per spi_message flag saying that
the message is a flash read command?  If that's what this is trying to
do then why do you need to set the flag at all?  If the message is in a
clearly defined format and it's more efficient to use this mmap mode
then surely the driver can just recognise that the format is approprate
and switch into mmap mode without being explicitly told - I'm not clear
what the flag adds here.

[toc] | [prev] | [next] | [standalone]

#1200702 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote:
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> > On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>
>> > I still can't tell from the above what this interface is supposed to do.
>> > It sounds like the use of memory mapped mode is supposed to be
>> > transparent to users, it should just affect how the controller interacts
>> > with the hardware, but if that's the case why do we need to expose it to
>> > users at all?  Shouldn't the driver just use memory mapped mode if it's
>> > faster?
>
>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>> allows reading and writing to an SPI flash device only. Used to speed up
>> flash reads. It _cannot_ be used to communicate with non flash devices.
>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>> other non flash SPI device (in which case SFI_MM_IF must not be used
>> instead SPI_CORE is to be used) but there is no way(is there?) to
>> distinguish where spi_message is from. Therefore I introduced flag
>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>> this helps the ti-qspi driver to determine that the user is flash device
>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>> to communicate.
>
> So if you're trying to do this you need to document it adequately so
> that other people can understand what it is supposed to do and how to
> use and implement it.  People can't really tell how the interface is
> supposed to work based on what was in the patch and the above isn't
> really helping.  For example, how does this change or restrict what the
> contents of the spi_message are?
>
>> On the whole, I just need a way to determine that the user is a flash
>> device in order to switch to memory mapped interface.
>
> As far as I can tell you want to set a per spi_message flag saying that
> the message is a flash read command?  If that's what this is trying to
> do then why do you need to set the flag at all?  If the message is in a
> clearly defined format and it's more efficient to use this mmap mode
> then surely the driver can just recognise that the format is approprate
> and switch into mmap mode without being explicitly told - I'm not clear
> what the flag adds here.

ehm, the read command is just one byte.

I don't think sending 03 or other random byte as the first byte of a
SPI transfer can be used as reliable detection that we are talking to
a SPI flash memory.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200703 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
> On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:

> > As far as I can tell you want to set a per spi_message flag saying that
> > the message is a flash read command?  If that's what this is trying to
> > do then why do you need to set the flag at all?  If the message is in a
> > clearly defined format and it's more efficient to use this mmap mode
> > then surely the driver can just recognise that the format is approprate
> > and switch into mmap mode without being explicitly told - I'm not clear
> > what the flag adds here.

> ehm, the read command is just one byte.

> I don't think sending 03 or other random byte as the first byte of a
> SPI transfer can be used as reliable detection that we are talking to
> a SPI flash memory.

Why care - if something is physically in the same format as a flash read
command how would a device be able to tell that it wasn't actually a
flash read command?  The signals sent on the bus are going to be
identical anyway.

[toc] | [prev] | [next] | [standalone]

#1200709 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
>> On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:
>
>> > As far as I can tell you want to set a per spi_message flag saying that
>> > the message is a flash read command?  If that's what this is trying to
>> > do then why do you need to set the flag at all?  If the message is in a
>> > clearly defined format and it's more efficient to use this mmap mode
>> > then surely the driver can just recognise that the format is approprate
>> > and switch into mmap mode without being explicitly told - I'm not clear
>> > what the flag adds here.
>
>> ehm, the read command is just one byte.
>
>> I don't think sending 03 or other random byte as the first byte of a
>> SPI transfer can be used as reliable detection that we are talking to
>> a SPI flash memory.
>
> Why care - if something is physically in the same format as a flash read
> command how would a device be able to tell that it wasn't actually a
> flash read command?  The signals sent on the bus are going to be
> identical anyway.

Not only must the command be the same but also the response must be tha same.

The flash chip responds by sending arbitrary amount of data. Given
that transfer_one gets only the part that sends the read command and
the part to do the actual read may or may not follow this is getting a
bit hairy. Add in dummy bytes due to fast-read lag and page write
wrap-around and you get something that you definitely do not want
unless you are really sure that there is a flash memory on the other
end of the wire.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201596 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:
> On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:

> >> I don't think sending 03 or other random byte as the first byte of a
> >> SPI transfer can be used as reliable detection that we are talking to
> >> a SPI flash memory.

> > Why care - if something is physically in the same format as a flash read
> > command how would a device be able to tell that it wasn't actually a
> > flash read command?  The signals sent on the bus are going to be
> > identical anyway.

> Not only must the command be the same but also the response must be tha same.

What difference would that make?  The caller is sending a single SPI
operation and this is a user visible thing...

> The flash chip responds by sending arbitrary amount of data. Given
> that transfer_one gets only the part that sends the read command and
> the part to do the actual read may or may not follow this is getting a
> bit hairy. Add in dummy bytes due to fast-read lag and page write
> wrap-around and you get something that you definitely do not want
> unless you are really sure that there is a flash memory on the other
> end of the wire.

So if you're doing this you may have a good reason to implement
transfer_one_message() instead.  Or perhaps implement it in the core and
provide operations to do the map and unmap.  And of course if this sort
of requirement exists that's an obvious thing that must be documented
in the interfaces but isn't.

We need a lot more thought about the interface here, the lack of any
explanation of what the interface is supposed to be and the fact that
all questions about it are being answered in terms of describing the
specific system are both a bit worrying.

[toc] | [prev] | [next] | [standalone]

#1201614 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On 6 August 2015 at 11:02, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:
>> On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
>> > On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
>
>> >> I don't think sending 03 or other random byte as the first byte of a
>> >> SPI transfer can be used as reliable detection that we are talking to
>> >> a SPI flash memory.
>
>> > Why care - if something is physically in the same format as a flash read
>> > command how would a device be able to tell that it wasn't actually a
>> > flash read command?  The signals sent on the bus are going to be
>> > identical anyway.
>
>> Not only must the command be the same but also the response must be tha same.
>
> What difference would that make?  The caller is sending a single SPI
> operation and this is a user visible thing...
>
>> The flash chip responds by sending arbitrary amount of data. Given
>> that transfer_one gets only the part that sends the read command and
>> the part to do the actual read may or may not follow this is getting a
>> bit hairy. Add in dummy bytes due to fast-read lag and page write
>> wrap-around and you get something that you definitely do not want
>> unless you are really sure that there is a flash memory on the other
>> end of the wire.
>
> So if you're doing this you may have a good reason to implement
> transfer_one_message() instead.  Or perhaps implement it in the core and
> provide operations to do the map and unmap.  And of course if this sort
> of requirement exists that's an obvious thing that must be documented
> in the interfaces but isn't.
>
> We need a lot more thought about the interface here, the lack of any
> explanation of what the interface is supposed to be and the fact that
> all questions about it are being answered in terms of describing the
> specific system are both a bit worrying.

Disclaimer: I am not familiar with the hardware for which this patch
adds support.

However, I am familiar m25p80.c and as I understand it the controller
is basically supposed to implement m25p80.c in hardware when this flag
is set.

If I was using m25p80.c to talk to anything but an actual flash chip
it would get me quite worried.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web