[LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[P L Sai Krishna]

This patch adds dummy_cycles entry in the spi_transfer structure.
len field in the transfer structure contains dummy bytes along with
actual data bytes, controllers which requires dummy bytes use len
field and simply Ignore the dummy_cycles field. Controllers which
expects dummy cycles won't work directly by using len field because
host driver doesn't know that len field of a particular transfer
includes dummy bytes or not (and also number of dummy bytes included
in len field). In such cases host driver use this dummy_cycles field
to identify the number of dummy cycles and based on that it will send
the required number of dummy cycles.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
v2: 
 - Changed the structure member name from dummy to dummy_cycles.
 - Updated the documentation of dummy_cycles.
 - m25p80 changes split into another patch.

 include/linux/spi/spi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 857a9a1..63135b3 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master *master,
  * @len: size of rx and tx buffers (in bytes)
  * @speed_hz: Select a speed other than the device default for this
  *      transfer. If 0 the default (from @spi_device) is used.
+ * @dummy_cycles: number of dummy cycles. If host controller requires
+ * 	dummy cycles rather than dummy bytes which send along with Cmd
+ *	and address then this dummy_cycles is used.
  * @bits_per_word: select a bits_per_word other than the device default
  *      for this transfer. If 0 the default (from @spi_device) is used.
  * @cs_change: affects chipselect after this transfer completes
@@ -752,6 +755,7 @@ struct spi_transfer {
 	u8		bits_per_word;
 	u16		delay_usecs;
 	u32		speed_hz;
+	u32		dummy_cycles;
 
 	struct list_head transfer_list;
 };
-- 
2.1.2

[LINUX PATCH v2 2/3] mtd:m25p80: Assigned number of dummy cycles to dummy_cycles.

[P L Sai Krishna]

Assigned number of dummy cycles to dummy_cycles member of
spi_transfer structure in m25p80_read API.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
v2: 
 - New Patch.
 
 drivers/mtd/devices/m25p80.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c9c3b7f..7c9fac9 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -139,6 +139,7 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 
 	t[0].tx_buf = flash->command;
 	t[0].len = m25p_cmdsz(nor) + dummy;
+	t[0].dummy = nor->read_dummy;
 	spi_message_add_tail(&t[0], &m);
 
 	t[1].rx_buf = buf;
-- 
2.1.2

Re: [LINUX PATCH v2 2/3] mtd:m25p80: Assigned number of dummy cycles to dummy_cycles.

[kbuild test robot]

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

Hi L,

[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.6-rc2 next-20160407]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/P-L-Sai-Krishna/spi-Added-dummy_cycle-entry-in-the-spi_transfer-structure/20160407-225040
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi for-next
config: x86_64-randconfig-x014-201614 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/mtd/devices/m25p80.c: In function 'm25p80_read':
>> drivers/mtd/devices/m25p80.c:142:6: error: 'struct spi_transfer' has no member named 'dummy'
     t[0].dummy = nor->read_dummy;
         ^

vim +142 drivers/mtd/devices/m25p80.c

   136	
   137		flash->command[0] = nor->read_opcode;
   138		m25p_addr2cmd(nor, from, flash->command);
   139	
   140		t[0].tx_buf = flash->command;
   141		t[0].len = m25p_cmdsz(nor) + dummy;
 > 142		t[0].dummy = nor->read_dummy;
   143		spi_message_add_tail(&t[0], &m);
   144	
   145		t[1].rx_buf = buf;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24963 bytes --]

[LINUX PATCH v2 3/3] spi:zynqmp:gqspi: Added separate dummy entry.

[P L Sai Krishna]

This patch sends dummy as a separate entry.
Break the Address+Cmd+dummy transfer into multiple transfers.
Address+Cmd as one transfer.
Dummy cycles as another transfer.
As per the controller spec, immediate data field of dummy entry
in the GenFifo represent dummy cycles.
Bus width for dummy cycles transfer should be same as
Rx bus width.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
v2:
 - Replaced dummy with dummy_cycles.
 
 drivers/spi/spi-zynqmp-gqspi.c | 57 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index aab9b49..a4b109e 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -134,6 +134,9 @@
 #define GQSPI_SELECT_MODE_QUADSPI	0x4
 #define GQSPI_DMA_UNALIGN		0x3
 #define GQSPI_DEFAULT_NUM_CS	1	/* Default number of chip selects */
+#define GQSPI_RX_BUS_WIDTH_QUAD		0x4
+#define GQSPI_RX_BUS_WIDTH_DUAL		0x2
+#define GQSPI_RX_BUS_WIDTH_SINGLE	0x1
 
 enum mode_type {GQSPI_MODE_IO, GQSPI_MODE_DMA};
 
@@ -169,6 +172,7 @@ struct zynqmp_qspi {
 	u32 genfifobus;
 	u32 dma_rx_bytes;
 	dma_addr_t dma_addr;
+	u32 rx_bus_width;
 	u32 genfifoentry;
 	enum mode_type mode;
 };
@@ -541,6 +545,35 @@ static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
 }
 
 /**
+ * zynqmp_qspi_preparedummy:	Prepares the dummy entry
+ *
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @transfer:	It is a pointer to the structure containing transfer data.
+ * @genfifoentry:	genfifoentry is pointer to the variable in which
+ *			GENFIFO	mask is returned to calling function
+ */
+static void zynqmp_qspi_preparedummy(struct zynqmp_qspi *xqspi,
+					struct spi_transfer *transfer,
+					u32 *genfifoentry)
+{
+	/* For dummy Tx and Rx are NULL */
+	*genfifoentry &= ~(GQSPI_GENFIFO_TX | GQSPI_GENFIFO_RX);
+
+	/* SPI mode */
+	*genfifoentry &= ~GQSPI_GENFIFO_MODE_QUADSPI;
+	if (xqspi->rx_bus_width == GQSPI_RX_BUS_WIDTH_QUAD)
+		*genfifoentry |= GQSPI_GENFIFO_MODE_QUADSPI;
+	else if (xqspi->rx_bus_width == GQSPI_RX_BUS_WIDTH_DUAL)
+		*genfifoentry |= GQSPI_GENFIFO_MODE_DUALSPI;
+	else
+		*genfifoentry |= GQSPI_GENFIFO_MODE_SPI;
+
+	/* Immediate data */
+	*genfifoentry &= ~GQSPI_GENFIFO_IMM_DATA_MASK;
+	*genfifoentry |= transfer->dummy_cycles;
+}
+
+/**
  * zynqmp_qspi_readrxfifo:	Fills the RX FIFO as long as there is room in
  *				the FIFO.
  * @xqspi:	Pointer to the zynqmp_qspi structure
@@ -771,7 +804,7 @@ static void zynqmp_qspi_txrxsetup(struct zynqmp_qspi *xqspi,
 		*genfifoentry |= GQSPI_GENFIFO_TX;
 		*genfifoentry |=
 			zynqmp_qspi_selectspimode(xqspi, transfer->tx_nbits);
-		xqspi->bytes_to_transfer = transfer->len;
+		xqspi->bytes_to_transfer = transfer->len - (transfer->dummy_cycles/8);
 		if (xqspi->mode == GQSPI_MODE_DMA) {
 			config_reg = zynqmp_gqspi_read(xqspi,
 							GQSPI_CONFIG_OFST);
@@ -832,13 +865,19 @@ static int zynqmp_qspi_start_transfer(struct spi_master *master,
 	if (xqspi->mode == GQSPI_MODE_DMA)
 		transfer_len = xqspi->dma_rx_bytes;
 	else
-		transfer_len = transfer->len;
+		transfer_len = transfer->len - (transfer->dummy_cycles/8);
 
 	xqspi->genfifoentry = genfifoentry;
 	if ((transfer_len) < GQSPI_GENFIFO_IMM_DATA_MASK) {
 		genfifoentry &= ~GQSPI_GENFIFO_IMM_DATA_MASK;
 		genfifoentry |= transfer_len;
 		zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST, genfifoentry);
+		if (transfer->dummy_cycles) {
+			zynqmp_qspi_preparedummy(xqspi, transfer,
+					&genfifoentry);
+			zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST,
+					genfifoentry);
+		}
 	} else {
 		int tempcount = transfer_len;
 		u32 exponent = 8;	/* 2^8 = 256 */
@@ -979,6 +1018,8 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
 	struct zynqmp_qspi *xqspi;
 	struct resource *res;
 	struct device *dev = &pdev->dev;
+	struct device_node *nc;
+	u32 rx_bus_width;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(*xqspi));
 	if (!master)
@@ -1039,6 +1080,18 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
 		goto clk_dis_all;
 	}
 
+	xqspi->rx_bus_width = GQSPI_RX_BUS_WIDTH_SINGLE;
+	for_each_available_child_of_node(pdev->dev.of_node, nc) {
+		ret = of_property_read_u32(nc, "spi-rx-bus-width",
+					&rx_bus_width);
+		if (!ret) {
+			xqspi->rx_bus_width = rx_bus_width;
+			break;
+		}
+	}
+	if (ret)
+		dev_err(dev, "rx bus width not found\n");
+
 	master->num_chipselect = GQSPI_DEFAULT_NUM_CS;
 
 	master->setup = zynqmp_qspi_setup;
-- 
2.1.2

Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Cyrille Pitchen]

Hi all,

Le 07/04/2016 16:39, P L Sai Krishna a écrit :
> This patch adds dummy_cycles entry in the spi_transfer structure.
> len field in the transfer structure contains dummy bytes along with
> actual data bytes, controllers which requires dummy bytes use len
> field and simply Ignore the dummy_cycles field. Controllers which
> expects dummy cycles won't work directly by using len field because
> host driver doesn't know that len field of a particular transfer
> includes dummy bytes or not (and also number of dummy bytes included
> in len field). In such cases host driver use this dummy_cycles field
> to identify the number of dummy cycles and based on that it will send
> the required number of dummy cycles.

Dummy cycles are only used with SPI NOR flashes, aren't they?
I guess so since there is also a patch dedicated to the m25p80 driver.

So why not using the spi_flash_read() API already introduced by Vignesh
in the SPI layer?

struct spi_flash_read_message already includes a 'dummy_bytes' member.

> 
> Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
> ---
> v2: 
>  - Changed the structure member name from dummy to dummy_cycles.
>  - Updated the documentation of dummy_cycles.
>  - m25p80 changes split into another patch.
> 
>  include/linux/spi/spi.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 857a9a1..63135b3 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master *master,
>   * @len: size of rx and tx buffers (in bytes)
>   * @speed_hz: Select a speed other than the device default for this
>   *      transfer. If 0 the default (from @spi_device) is used.
> + * @dummy_cycles: number of dummy cycles. If host controller requires
> + * 	dummy cycles rather than dummy bytes which send along with Cmd
> + *	and address then this dummy_cycles is used.
>   * @bits_per_word: select a bits_per_word other than the device default
>   *      for this transfer. If 0 the default (from @spi_device) is used.
>   * @cs_change: affects chipselect after this transfer completes
> @@ -752,6 +755,7 @@ struct spi_transfer {
>  	u8		bits_per_word;
>  	u16		delay_usecs;
>  	u32		speed_hz;
> +	u32		dummy_cycles;
>  
>  	struct list_head transfer_list;
>  };
> 

Best regards,

Cyrille

RE: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Lakshmi Sai Krishna Potthuri]

Hi Cyrille and Mark,

>-----Original Message-----
>From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
>Sent: Thursday, April 07, 2016 8:33 PM
>To: Lakshmi Sai Krishna Potthuri <lakshmis@xilinx.com>; Michal Simek
><michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>; David
>Woodhouse <dwmw2@infradead.org>; Brian Norris
><computersforpeace@gmail.com>; Mark Brown <broonie@kernel.org>;
>Javier Martinez Canillas <javier@osg.samsung.com>; Boris Brezillon
><boris.brezillon@free-electrons.com>; Stephen Warren
><swarren@nvidia.com>; Geert Uytterhoeven <geert+renesas@glider.be>;
>Andrew F. Davis <afd@ti.com>; Marek Vasut <marex@denx.de>; Jagan Teki
><jteki@openedev.com>; Rafał Miłecki <zajec5@gmail.com>
>Cc: linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Harini Katakam
><harinik@xilinx.com>; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
>Anirudha Sarangi <anirudh@xilinx.com>; Lakshmi Sai Krishna Potthuri
><lakshmis@xilinx.com>; R, Vignesh <vigneshr@ti.com>
>Subject: Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the
>spi_transfer structure.
>
>Hi all,
>
>Le 07/04/2016 16:39, P L Sai Krishna a écrit :
>> This patch adds dummy_cycles entry in the spi_transfer structure.
>> len field in the transfer structure contains dummy bytes along with
>> actual data bytes, controllers which requires dummy bytes use len
>> field and simply Ignore the dummy_cycles field. Controllers which
>> expects dummy cycles won't work directly by using len field because
>> host driver doesn't know that len field of a particular transfer
>> includes dummy bytes or not (and also number of dummy bytes included
>> in len field). In such cases host driver use this dummy_cycles field
>> to identify the number of dummy cycles and based on that it will send
>> the required number of dummy cycles.
>
>Dummy cycles are only used with SPI NOR flashes, aren't they?

Yes, with SPI NOR flashes.

>I guess so since there is also a patch dedicated to the m25p80 driver.
>
>So why not using the spi_flash_read() API already introduced by Vignesh
>in the SPI layer?

ZynqMP GQSPI controller driver use "transfer_one" hook. Since this is generic
Controller majorly interfaced to flash devices and it might extend to NAND
flashes in future. This patch does not disturb the existing implementation of
accessing the flash or other slave devices. It adds an additional optional
feature. So there is no harm to controllers that don't need dummy cycles -
they can ignore it and still work with the existing implementation.

Regards
Sai Krishna
>
>struct spi_flash_read_message already includes a 'dummy_bytes' member.
>
>>
>> Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
>> ---
>> v2:
>>  - Changed the structure member name from dummy to dummy_cycles.
>>  - Updated the documentation of dummy_cycles.
>>  - m25p80 changes split into another patch.
>>
>>  include/linux/spi/spi.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>> index 857a9a1..63135b3 100644
>> --- a/include/linux/spi/spi.h
>> +++ b/include/linux/spi/spi.h
>> @@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master
>*master,
>>   * @len: size of rx and tx buffers (in bytes)
>>   * @speed_hz: Select a speed other than the device default for this
>>   *      transfer. If 0 the default (from @spi_device) is used.
>> + * @dummy_cycles: number of dummy cycles. If host controller requires
>> + *  dummy cycles rather than dummy bytes which send along with Cmd
>> + *  and address then this dummy_cycles is used.
>>   * @bits_per_word: select a bits_per_word other than the device default
>>   *      for this transfer. If 0 the default (from @spi_device) is used.
>>   * @cs_change: affects chipselect after this transfer completes
>> @@ -752,6 +755,7 @@ struct spi_transfer {
>>      u8              bits_per_word;
>>      u16             delay_usecs;
>>      u32             speed_hz;
>> +    u32             dummy_cycles;
>>
>>      struct list_head transfer_list;
>>  };
>>
>
>Best regards,
>
>Cyrille


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Cyrille Pitchen]

Hi Sai Krishna,

Le 13/04/2016 07:21, Lakshmi Sai Krishna Potthuri a écrit :
> Hi Cyrille and Mark,
> 
>> -----Original Message-----
>> From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
>> Sent: Thursday, April 07, 2016 8:33 PM
>> To: Lakshmi Sai Krishna Potthuri <lakshmis@xilinx.com>; Michal Simek
>> <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>; David
>> Woodhouse <dwmw2@infradead.org>; Brian Norris
>> <computersforpeace@gmail.com>; Mark Brown <broonie@kernel.org>;
>> Javier Martinez Canillas <javier@osg.samsung.com>; Boris Brezillon
>> <boris.brezillon@free-electrons.com>; Stephen Warren
>> <swarren@nvidia.com>; Geert Uytterhoeven <geert+renesas@glider.be>;
>> Andrew F. Davis <afd@ti.com>; Marek Vasut <marex@denx.de>; Jagan Teki
>> <jteki@openedev.com>; Rafał Miłecki <zajec5@gmail.com>
>> Cc: linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>> spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Harini Katakam
>> <harinik@xilinx.com>; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
>> Anirudha Sarangi <anirudh@xilinx.com>; Lakshmi Sai Krishna Potthuri
>> <lakshmis@xilinx.com>; R, Vignesh <vigneshr@ti.com>
>> Subject: Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the
>> spi_transfer structure.
>>
>> Hi all,
>>
>> Le 07/04/2016 16:39, P L Sai Krishna a écrit :
>>> This patch adds dummy_cycles entry in the spi_transfer structure.
>>> len field in the transfer structure contains dummy bytes along with
>>> actual data bytes, controllers which requires dummy bytes use len
>>> field and simply Ignore the dummy_cycles field. Controllers which
>>> expects dummy cycles won't work directly by using len field because
>>> host driver doesn't know that len field of a particular transfer
>>> includes dummy bytes or not (and also number of dummy bytes included
>>> in len field). In such cases host driver use this dummy_cycles field
>>> to identify the number of dummy cycles and based on that it will send
>>> the required number of dummy cycles.
>>
>> Dummy cycles are only used with SPI NOR flashes, aren't they?
> 
> Yes, with SPI NOR flashes.
> 
>> I guess so since there is also a patch dedicated to the m25p80 driver.
>>
>> So why not using the spi_flash_read() API already introduced by Vignesh
>> in the SPI layer?
> 
> ZynqMP GQSPI controller driver use "transfer_one" hook. Since this is generic
> Controller majorly interfaced to flash devices and it might extend to NAND
> flashes in future. This patch does not disturb the existing implementation of
> accessing the flash or other slave devices. It adds an additional optional
> feature. So there is no harm to controllers that don't need dummy cycles -
> they can ignore it and still work with the existing implementation.
> 
> Regards
> Sai Krishna

I understand but you propose to patch both the SPI layer and the m25p80 driver
to introduce some support which is already provided by the "spi_flash_read"
hook: struct spi_flash_read_message has already a "dummy_bytes" field.
IMHO, it looks redundant.

If you really want to use something close to the "transfer_one" hook, I guess
you could build spi_transfer structures from the content of the struct
spi_flash_read_message argument of spi_flash_read() then pass those
spi_transfer structures to your custom "transfer_one_ex":

int transfer_one_ex(struct spi_master *master, struct spi_device *spi,
		    struct spi_transfer *transfer, u32 dummy_cycles);

You just need to set dummy_cycles to 0 to implement the regular "transfer_one"
hook.

Best regards,

Cyrille

>>
>> struct spi_flash_read_message already includes a 'dummy_bytes' member.
>>
>>>
>>> Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
>>> ---
>>> v2:
>>>  - Changed the structure member name from dummy to dummy_cycles.
>>>  - Updated the documentation of dummy_cycles.
>>>  - m25p80 changes split into another patch.
>>>
>>>  include/linux/spi/spi.h | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>>> index 857a9a1..63135b3 100644
>>> --- a/include/linux/spi/spi.h
>>> +++ b/include/linux/spi/spi.h
>>> @@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master
>> *master,
>>>   * @len: size of rx and tx buffers (in bytes)
>>>   * @speed_hz: Select a speed other than the device default for this
>>>   *      transfer. If 0 the default (from @spi_device) is used.
>>> + * @dummy_cycles: number of dummy cycles. If host controller requires
>>> + *  dummy cycles rather than dummy bytes which send along with Cmd
>>> + *  and address then this dummy_cycles is used.
>>>   * @bits_per_word: select a bits_per_word other than the device default
>>>   *      for this transfer. If 0 the default (from @spi_device) is used.
>>>   * @cs_change: affects chipselect after this transfer completes
>>> @@ -752,6 +755,7 @@ struct spi_transfer {
>>>      u8              bits_per_word;
>>>      u16             delay_usecs;
>>>      u32             speed_hz;
>>> +    u32             dummy_cycles;
>>>
>>>      struct list_head transfer_list;
>>>  };
>>>
>>
>> Best regards,
>>
>> Cyrille
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 

Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Mark Brown]

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

On Thu, Apr 14, 2016 at 10:06:55AM +0200, Cyrille Pitchen wrote:

> I understand but you propose to patch both the SPI layer and the m25p80 driver
> to introduce some support which is already provided by the "spi_flash_read"
> hook: struct spi_flash_read_message has already a "dummy_bytes" field.
> IMHO, it looks redundant.

My understanding is that this is intended for dummy bits rather than
dummy bytes.

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

Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Cyrille Pitchen]

Le 14/04/2016 10:57, Mark Brown a écrit :
> On Thu, Apr 14, 2016 at 10:06:55AM +0200, Cyrille Pitchen wrote:
> 
>> I understand but you propose to patch both the SPI layer and the m25p80 driver
>> to introduce some support which is already provided by the "spi_flash_read"
>> hook: struct spi_flash_read_message has already a "dummy_bytes" field.
>> IMHO, it looks redundant.
> 
> My understanding is that this is intended for dummy bits rather than
> dummy bytes.
> 

dummy_bits == (dummy_bytes * 8) and
dummy_cycles == ((dummy_bytes * 8) / addr_nbits)

witch addr_nbits in {1, 2, 4}

the struct_flash_read_message has both dummy_bytes and addr_nbits members.

The spi-nor framework seems to always provide a multiple of 8 for dummy *bits*.
I guess because the m25p80 driver only supports such number of dummy bits but
also because all SPI memories can be configured so their number of dummy cycles keeps the byte alignment for the data to follow.

It still allows to use less than 8 dummy *cycles*, for instance the factory
settings for Macronix Quad SPI memories are:
- 4 dummy cycles for Fast Read 1-2-2 (hence 8 dummy bits)
- 6 dummy cycles for Fast Read x-4-4 (hence 24 dummy bits)

AFAIK, only Micron QSPI memories could be configured so the number of dummy
cycles doesn't result in a multiple of 8 bits but theirs are not the
recommanded timings provided by the datasheet.

Micron factory settings are:
- 10 dummy cycles for Fast Read x-4-4 (hence 40 dummy bits)
- 8 dummy cycles for other Fast Reads.


Best regards,

Cyrille

Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

[Mark Brown]

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

On Thu, Apr 07, 2016 at 08:09:27PM +0530, P L Sai Krishna wrote:

> + * @dummy_cycles: number of dummy cycles. If host controller requires
> + * 	dummy cycles rather than dummy bytes which send along with Cmd
> + *	and address then this dummy_cycles is used.

I'm having a very hard time parsing this, partly because it is entirely
flash specific (the whole concept of cmd and address is missing from
SPI).  As Cyrille said perhaps this should go in a flash specific
interface?

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