linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
@ 2019-01-28  5:02 Purna Chandra Mandal
  2019-02-03 12:20 ` Tudor.Ambarus
  2019-02-10 18:56 ` Boris Brezillon
  0 siblings, 2 replies; 6+ messages in thread
From: Purna Chandra Mandal @ 2019-01-28  5:02 UTC (permalink / raw)
  To: Fred Bloggs, Boris Brezillon, Tudor Ambarus
  Cc: Richard Weinberger, Purna Chandra Mandal, linux-kernel,
	Marek Vasut, linux-mtd, Brian Norris, David Woodhouse

cadence-quadspi controller allows upto eight bytes of data to
be written in software Triggered Instruction generator (STIG) mode
of operation. Lower 4 bytes are written through writedatalower and
upper 4 bytes by writedataupper register.

This patch allows all the 8 bytes to be written.

Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>
---

 drivers/mtd/spi-nor/cadence-quadspi.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 04cedd3a2bf6..7f78f9409ddd 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -418,9 +418,10 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
 	void __iomem *reg_base = cqspi->iobase;
 	unsigned int reg;
 	unsigned int data;
+	u32 write_len;
 	int ret;
 
-	if (n_tx > 4 || (n_tx && !txbuf)) {
+	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
 		dev_err(nor->dev,
 			"Invalid input argument, cmdlen %d txbuf 0x%p\n",
 			n_tx, txbuf);
@@ -433,10 +434,18 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
 		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
 			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
 		data = 0;
-		memcpy(&data, txbuf, n_tx);
+		write_len = (n_tx > 4) ? 4 : n_tx;
+		memcpy(&data, txbuf, write_len);
+		txbuf += write_len;
 		writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
-	}
 
+		if (n_tx > 4) {
+			data = 0;
+			write_len = n_tx - 4;
+			memcpy(&data, txbuf, write_len);
+			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
+		}
+	}
 	ret = cqspi_exec_flash_cmd(cqspi, reg);
 	return ret;
 }
-- 
2.13.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
  2019-01-28  5:02 [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode Purna Chandra Mandal
@ 2019-02-03 12:20 ` Tudor.Ambarus
  2019-02-04 13:37   ` Vignesh R
  2019-02-10 18:56 ` Boris Brezillon
  1 sibling, 1 reply; 6+ messages in thread
From: Tudor.Ambarus @ 2019-02-03 12:20 UTC (permalink / raw)
  To: purna.chandra.mandal, f.blogs, boris.brezillon, richard, vigneshr
  Cc: marek.vasut, dwmw2, computersforpeace, linux-mtd, linux-kernel

+ Vignesh

On 01/28/2019 07:02 AM, Purna Chandra Mandal wrote:
> cadence-quadspi controller allows upto eight bytes of data to
> be written in software Triggered Instruction generator (STIG) mode
> of operation. Lower 4 bytes are written through writedatalower and
> upper 4 bytes by writedataupper register.
> 
> This patch allows all the 8 bytes to be written.
> 
> Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>

Looks good for me:
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Vignesh, can we have your R-b or T-b tag?

Cheers,
ta

> ---
> 
>  drivers/mtd/spi-nor/cadence-quadspi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 04cedd3a2bf6..7f78f9409ddd 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -418,9 +418,10 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>  	void __iomem *reg_base = cqspi->iobase;
>  	unsigned int reg;
>  	unsigned int data;
> +	u32 write_len;
>  	int ret;
>  
> -	if (n_tx > 4 || (n_tx && !txbuf)) {
> +	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
>  		dev_err(nor->dev,
>  			"Invalid input argument, cmdlen %d txbuf 0x%p\n",
>  			n_tx, txbuf);
> @@ -433,10 +434,18 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>  		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
>  			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
>  		data = 0;
> -		memcpy(&data, txbuf, n_tx);
> +		write_len = (n_tx > 4) ? 4 : n_tx;
> +		memcpy(&data, txbuf, write_len);
> +		txbuf += write_len;
>  		writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
> -	}
>  
> +		if (n_tx > 4) {
> +			data = 0;
> +			write_len = n_tx - 4;
> +			memcpy(&data, txbuf, write_len);
> +			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
> +		}
> +	}
>  	ret = cqspi_exec_flash_cmd(cqspi, reg);
>  	return ret;
>  }
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
  2019-02-03 12:20 ` Tudor.Ambarus
@ 2019-02-04 13:37   ` Vignesh R
  2019-02-05  7:00     ` Mandal, Purna Chandra
  0 siblings, 1 reply; 6+ messages in thread
From: Vignesh R @ 2019-02-04 13:37 UTC (permalink / raw)
  To: Tudor.Ambarus, purna.chandra.mandal, f.blogs, boris.brezillon, richard
  Cc: marek.vasut, dwmw2, computersforpeace, linux-mtd, linux-kernel

Hi,

On 03/02/19 5:50 PM, Tudor.Ambarus@microchip.com wrote:
> + Vignesh
> 

Thanks for looping in.

> On 01/28/2019 07:02 AM, Purna Chandra Mandal wrote:
>> cadence-quadspi controller allows upto eight bytes of data to
>> be written in software Triggered Instruction generator (STIG) mode
>> of operation. Lower 4 bytes are written through writedatalower and
>> upper 4 bytes by writedataupper register.
>>
>> This patch allows all the 8 bytes to be written.
>>

Code as such looks fine. But, how was this tested? How can I trigger
this new code path with current linux-next? AFAICS, STIG mode write is
used to in nor->write_reg() path, and I dont see any nor->write_reg()
call with >4bytes len.

>> Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>
> 
> Looks good for me:
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Vignesh, can we have your R-b or T-b tag?
> 
> Cheers,
> ta
> 
>> ---
>>
>>  drivers/mtd/spi-nor/cadence-quadspi.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 04cedd3a2bf6..7f78f9409ddd 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -418,9 +418,10 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>  	void __iomem *reg_base = cqspi->iobase;
>>  	unsigned int reg;
>>  	unsigned int data;
>> +	u32 write_len;
>>  	int ret;
>>  
>> -	if (n_tx > 4 || (n_tx && !txbuf)) {
>> +	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
>>  		dev_err(nor->dev,
>>  			"Invalid input argument, cmdlen %d txbuf 0x%p\n",
>>  			n_tx, txbuf);
>> @@ -433,10 +434,18 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>  		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
>>  			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
>>  		data = 0;
>> -		memcpy(&data, txbuf, n_tx);
>> +		write_len = (n_tx > 4) ? 4 : n_tx;
>> +		memcpy(&data, txbuf, write_len);
>> +		txbuf += write_len;
>>  		writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
>> -	}
>>  
>> +		if (n_tx > 4) {
>> +			data = 0;
>> +			write_len = n_tx - 4;
>> +			memcpy(&data, txbuf, write_len);
>> +			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
>> +		}
>> +	}
>>  	ret = cqspi_exec_flash_cmd(cqspi, reg);
>>  	return ret;
>>  }
>>

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
  2019-02-04 13:37   ` Vignesh R
@ 2019-02-05  7:00     ` Mandal, Purna Chandra
  2019-02-05 10:00       ` Vignesh R
  0 siblings, 1 reply; 6+ messages in thread
From: Mandal, Purna Chandra @ 2019-02-05  7:00 UTC (permalink / raw)
  To: Vignesh R, Tudor.Ambarus, f.blogs, boris.brezillon, richard
  Cc: marek.vasut, dwmw2, computersforpeace, linux-mtd, linux-kernel



On 04-Feb-19 7:07 PM, Vignesh R wrote:
> Hi,
> 
> On 03/02/19 5:50 PM, Tudor.Ambarus@microchip.com wrote:
>> + Vignesh
>>
> 
> Thanks for looping in.
> 
>> On 01/28/2019 07:02 AM, Purna Chandra Mandal wrote:
>>> cadence-quadspi controller allows upto eight bytes of data to
>>> be written in software Triggered Instruction generator (STIG) mode
>>> of operation. Lower 4 bytes are written through writedatalower and
>>> upper 4 bytes by writedataupper register.
>>>
>>> This patch allows all the 8 bytes to be written.
>>>
> 
> Code as such looks fine. But, how was this tested? How can I trigger
> this new code path with current linux-next? AFAICS, STIG mode write is
> used to in nor->write_reg() path, and I dont see any nor->write_reg()
> call with >4bytes len.
Currently there is no linux user of write_reg() for write_len > 4byte.

For volatile and non-volatile sector locking [1], we have one out of 
tree implementation and that is specific to flash chip "mt25qu02g". In 
this implementation we need additional sector address (4 byte) to be 
provided for each lock-bit write/erase operation. So total write len in 
write_reg() will be 6 bytes (=1 for opcode, 4 for sect addr, 1 for 
data). We are finalizing the patch for review.

Since cadence qspi controller do support the 8-byte read/write in STIG 
mode, I have tried here to enable that in write_reg(), similar to 
read_reg().

[1] 
https://www.micron.com/~/media/documents/products/data-sheet/nor-flash/serial-nor/n25q/n25q_128mb_3v_65nm.pdf

> 
>>> Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>
>>
>> Looks good for me:
>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Vignesh, can we have your R-b or T-b tag?
>>
>> Cheers,
>> ta
>>
>>> ---
>>>
>>>   drivers/mtd/spi-nor/cadence-quadspi.c | 15 ++++++++++++---
>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 04cedd3a2bf6..7f78f9409ddd 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -418,9 +418,10 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>>   	void __iomem *reg_base = cqspi->iobase;
>>>   	unsigned int reg;
>>>   	unsigned int data;
>>> +	u32 write_len;
>>>   	int ret;
>>>   
>>> -	if (n_tx > 4 || (n_tx && !txbuf)) {
>>> +	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
>>>   		dev_err(nor->dev,
>>>   			"Invalid input argument, cmdlen %d txbuf 0x%p\n",
>>>   			n_tx, txbuf);
>>> @@ -433,10 +434,18 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>>   		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
>>>   			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
>>>   		data = 0;
>>> -		memcpy(&data, txbuf, n_tx);
>>> +		write_len = (n_tx > 4) ? 4 : n_tx;
>>> +		memcpy(&data, txbuf, write_len);
>>> +		txbuf += write_len;
>>>   		writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
>>> -	}
>>>   
>>> +		if (n_tx > 4) {
>>> +			data = 0;
>>> +			write_len = n_tx - 4;
>>> +			memcpy(&data, txbuf, write_len);
>>> +			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
>>> +		}
>>> +	}
>>>   	ret = cqspi_exec_flash_cmd(cqspi, reg);
>>>   	return ret;
>>>   }
>>>
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
  2019-02-05  7:00     ` Mandal, Purna Chandra
@ 2019-02-05 10:00       ` Vignesh R
  0 siblings, 0 replies; 6+ messages in thread
From: Vignesh R @ 2019-02-05 10:00 UTC (permalink / raw)
  To: Mandal, Purna Chandra, Tudor.Ambarus, f.blogs, boris.brezillon, richard
  Cc: marek.vasut, dwmw2, computersforpeace, linux-mtd, linux-kernel



On 05/02/19 12:30 PM, Mandal, Purna Chandra wrote:
> 
> 
> On 04-Feb-19 7:07 PM, Vignesh R wrote:
>> Hi,
>>
>> On 03/02/19 5:50 PM, Tudor.Ambarus@microchip.com wrote:
>>> + Vignesh
>>>
>>
>> Thanks for looping in.
>>
>>> On 01/28/2019 07:02 AM, Purna Chandra Mandal wrote:
>>>> cadence-quadspi controller allows upto eight bytes of data to
>>>> be written in software Triggered Instruction generator (STIG) mode
>>>> of operation. Lower 4 bytes are written through writedatalower and
>>>> upper 4 bytes by writedataupper register.
>>>>
>>>> This patch allows all the 8 bytes to be written.
>>>>
>>
>> Code as such looks fine. But, how was this tested? How can I trigger
>> this new code path with current linux-next? AFAICS, STIG mode write is
>> used to in nor->write_reg() path, and I dont see any nor->write_reg()
>> call with >4bytes len.
> Currently there is no linux user of write_reg() for write_len > 4byte.
> 
> For volatile and non-volatile sector locking [1], we have one out of 
> tree implementation and that is specific to flash chip "mt25qu02g". In 
> this implementation we need additional sector address (4 byte) to be 
> provided for each lock-bit write/erase operation. So total write len in 
> write_reg() will be 6 bytes (=1 for opcode, 4 for sect addr, 1 for 
> data). We are finalizing the patch for review.
> 
> Since cadence qspi controller do support the 8-byte read/write in STIG 
> mode, I have tried here to enable that in write_reg(), similar to 
> read_reg().
> 

Sounds good.

Reviewed-by: Vignesh R <vigneshr@ti.com>

> [1] 
> https://www.micron.com/~/media/documents/products/data-sheet/nor-flash/serial-nor/n25q/n25q_128mb_3v_65nm.pdf
> 
>>
>>>> Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>
>>>
>>> Looks good for me:
>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>> Vignesh, can we have your R-b or T-b tag?
>>>
>>> Cheers,
>>> ta
>>>
>>>> ---
>>>>
>>>>   drivers/mtd/spi-nor/cadence-quadspi.c | 15 ++++++++++++---
>>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> index 04cedd3a2bf6..7f78f9409ddd 100644
>>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>> @@ -418,9 +418,10 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>>>   	void __iomem *reg_base = cqspi->iobase;
>>>>   	unsigned int reg;
>>>>   	unsigned int data;
>>>> +	u32 write_len;
>>>>   	int ret;
>>>>   
>>>> -	if (n_tx > 4 || (n_tx && !txbuf)) {
>>>> +	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
>>>>   		dev_err(nor->dev,
>>>>   			"Invalid input argument, cmdlen %d txbuf 0x%p\n",
>>>>   			n_tx, txbuf);
>>>> @@ -433,10 +434,18 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>>>>   		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
>>>>   			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
>>>>   		data = 0;
>>>> -		memcpy(&data, txbuf, n_tx);
>>>> +		write_len = (n_tx > 4) ? 4 : n_tx;
>>>> +		memcpy(&data, txbuf, write_len);
>>>> +		txbuf += write_len;
>>>>   		writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
>>>> -	}
>>>>   
>>>> +		if (n_tx > 4) {
>>>> +			data = 0;
>>>> +			write_len = n_tx - 4;
>>>> +			memcpy(&data, txbuf, write_len);
>>>> +			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
>>>> +		}
>>>> +	}
>>>>   	ret = cqspi_exec_flash_cmd(cqspi, reg);
>>>>   	return ret;
>>>>   }
>>>>
>>

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode
  2019-01-28  5:02 [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode Purna Chandra Mandal
  2019-02-03 12:20 ` Tudor.Ambarus
@ 2019-02-10 18:56 ` Boris Brezillon
  1 sibling, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2019-02-10 18:56 UTC (permalink / raw)
  To: Purna Chandra Mandal, Fred Bloggs, Boris Brezillon, Tudor Ambarus
  Cc: Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut,
	linux-mtd, Brian Norris, David Woodhouse

From: Boris Brezillon <bbrezillon@kernel.org>

On Mon, 2019-01-28 at 05:02:29 UTC, Purna Chandra Mandal wrote:
> cadence-quadspi controller allows upto eight bytes of data to
> be written in software Triggered Instruction generator (STIG) mode
> of operation. Lower 4 bytes are written through writedatalower and
> upper 4 bytes by writedataupper register.
> 
> This patch allows all the 8 bytes to be written.
> 
> Signed-off-by: Purna Chandra Mandal <purna.chandra.mandal@intel.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Reviewed-by: Vignesh R <vigneshr@ti.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next, thanks.

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2019-02-10 18:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28  5:02 [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode Purna Chandra Mandal
2019-02-03 12:20 ` Tudor.Ambarus
2019-02-04 13:37   ` Vignesh R
2019-02-05  7:00     ` Mandal, Purna Chandra
2019-02-05 10:00       ` Vignesh R
2019-02-10 18:56 ` Boris Brezillon

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).