Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-02-05 Thread Vignesh R 



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.amba...@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 

> [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 
>>>
>>> Looks good for me:
>>> Reviewed-by: Tudor Ambarus 
>>>
>>> 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(, txbuf, n_tx);
 +  write_len = (n_tx > 4) ? 4 : n_tx;
 +  memcpy(, 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(, txbuf, write_len);
 +  writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
 +  }
 +  }
ret = cqspi_exec_flash_cmd(cqspi, reg);
return ret;
   }

>>

-- 
Regards
Vignesh

Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-02-04 Thread Mandal, Purna Chandra 




On 04-Feb-19 7:07 PM, Vignesh R wrote:

Hi,

On 03/02/19 5:50 PM, tudor.amba...@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 


Looks good for me:
Reviewed-by: Tudor Ambarus 

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(, txbuf, n_tx);
+   write_len = (n_tx > 4) ? 4 : n_tx;
+   memcpy(, 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(, txbuf, write_len);
+   writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
+   }
+   }
ret = cqspi_exec_flash_cmd(cqspi, reg);
return ret;
  }

Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-02-04 Thread Vignesh R 
Hi,

On 03/02/19 5:50 PM, tudor.amba...@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 
> 
> Looks good for me:
> Reviewed-by: Tudor Ambarus 
> 
> 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(, txbuf, n_tx);
>> +write_len = (n_tx > 4) ? 4 : n_tx;
>> +memcpy(, 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(, txbuf, write_len);
>> +writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
>> +}
>> +}
>>  ret = cqspi_exec_flash_cmd(cqspi, reg);
>>  return ret;
>>  }
>>

-- 
Regards
Vignesh

Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-02-03 Thread Tudor.Ambarus 
+ 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 

Looks good for me:
Reviewed-by: Tudor Ambarus 

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(, txbuf, n_tx);
> + write_len = (n_tx > 4) ? 4 : n_tx;
> + memcpy(, 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(, txbuf, write_len);
> + writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
> + }
> + }
>   ret = cqspi_exec_flash_cmd(cqspi, reg);
>   return ret;
>  }
>

[PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-01-27 Thread Purna Chandra Mandal 
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 
---

 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(, txbuf, n_tx);
+   write_len = (n_tx > 4) ? 4 : n_tx;
+   memcpy(, 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(, txbuf, write_len);
+   writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
+   }
+   }
ret = cqspi_exec_flash_cmd(cqspi, reg);
return ret;
 }
-- 
2.13.0

Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-01-27 Thread Mandal, Purna Chandra 



On 21-Jan-19 3:07 PM, tudor.amba...@microchip.com wrote:

Hi, Purna,

On 12/14/2018 04:15 AM, Purna Chandra Mandal wrote:

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


Is this supported by all versions of the IP?


Yes, it is supported on all versions of IP.


This patch allows all the 8 bytes to be written.

Signed-off-by: Purna Chandra Mandal 
---

  drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c 
b/drivers/mtd/spi-nor/cadence-quadspi.c
index 04cedd3a2bf6..990934387fea 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -420,7 +420,7 @@ static int cqspi_command_write(struct spi_nor *nor, const 
u8 opcode,
unsigned int data;


I know that this is not your change, but data should be u32 and not unsigned
int. reg too ... probably one should submit a patch to change this, I see that
the same error is done in cqspi_command_read().

ack. will be done in separate patch.



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);
@@ -435,8 +435,13 @@ static int cqspi_command_write(struct spi_nor *nor, const 
u8 opcode,
data = 0;>   memcpy(, txbuf, n_tx);


You should limit what is copied in data, similar to what is done in
cqspi_command_read():

write_len = (n_tx > 4) ? 4 : n_tx;
memcpy(, txbuf, write_len);
txbuf += write_len;

ack.





writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
-   }
  
+		if (n_tx > 4) {

+   data = 0;

and then:
write_len = n_tx - 4;
memcpy(, txbuf, write_len);


ack.

Cheers,
ta


+   memcpy(, txbuf + 4, n_tx - 4);
+   writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
+   }
+   }
ret = cqspi_exec_flash_cmd(cqspi, reg);
return ret;
  }

Re: [PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2019-01-21 Thread Tudor.Ambarus 
Hi, Purna,

On 12/14/2018 04:15 AM, Purna Chandra Mandal wrote:
> cadence-quadspi controller allows upto eight bytes of data to be transferred
> in software Triggered Instruction generator (STIG) mode of operation. Lower
> 4 bytes are written through writedatalower and upper 4 bytes by
> writedataupper register.

Is this supported by all versions of the IP?

> 
> This patch allows all the 8 bytes to be written.
> 
> Signed-off-by: Purna Chandra Mandal 
> ---
> 
>  drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c 
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 04cedd3a2bf6..990934387fea 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -420,7 +420,7 @@ static int cqspi_command_write(struct spi_nor *nor, const 
> u8 opcode,
>   unsigned int data;

I know that this is not your change, but data should be u32 and not unsigned
int. reg too ... probably one should submit a patch to change this, I see that
the same error is done in cqspi_command_read().

>   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);
> @@ -435,8 +435,13 @@ static int cqspi_command_write(struct spi_nor *nor, 
> const u8 opcode,
>   data = 0;>  memcpy(, txbuf, n_tx);

You should limit what is copied in data, similar to what is done in
cqspi_command_read():

write_len = (n_tx > 4) ? 4 : n_tx;
memcpy(, txbuf, write_len);
txbuf += write_len;



>   writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
> - }
>  
> + if (n_tx > 4) {
> + data = 0;
and then:
write_len = n_tx - 4;
memcpy(, txbuf, write_len);

Cheers,
ta

> + memcpy(, txbuf + 4, n_tx - 4);
> + writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
> + }
> + }
>   ret = cqspi_exec_flash_cmd(cqspi, reg);
>   return ret;
>  }
>

[PATCH] mtd: spi-nor: cadence-quadspi: write upto 8-bytes data in STIG mode

2018-12-13 Thread Purna Chandra Mandal 
cadence-quadspi controller allows upto eight bytes of data to be transferred
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 
---

 drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c 
b/drivers/mtd/spi-nor/cadence-quadspi.c
index 04cedd3a2bf6..990934387fea 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -420,7 +420,7 @@ static int cqspi_command_write(struct spi_nor *nor, const 
u8 opcode,
unsigned int data;
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);
@@ -435,8 +435,13 @@ static int cqspi_command_write(struct spi_nor *nor, const 
u8 opcode,
data = 0;
memcpy(, txbuf, n_tx);
writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
-   }
 
+   if (n_tx > 4) {
+   data = 0;
+   memcpy(, txbuf + 4, n_tx - 4);
+   writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
+   }
+   }
ret = cqspi_exec_flash_cmd(cqspi, reg);
return ret;
 }
-- 
2.13.0