diff mbox series

[RFC,v2,1/5] mtd: cfi_cmdset_0002: Add support for polling status register

Message ID 20190321174548.9288-2-vigneshr@ti.com
State Superseded
Headers show
Series MTD: Add Initial Hyperbus support | expand

Commit Message

Raghavendra, Vignesh March 21, 2019, 5:45 p.m. UTC
HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
can be use as is. But these devices do not support DQ polling method of
determining chip ready/good status. These flashes provide Status
Register whose bits can be polled to know status of flash operation.

Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
Extended Query version 1.5. Bit 0 of "Software Features supported" field
of CFI Primary Vendor-Specific Extended Query table indicates
presence/absence of status register and Bit 1 indicates whether or not
DQ polling is supported. Using these bits, its possible to determine
whether flash supports DQ polling or need to use Status Register.

Add support for polling status register to know device ready/status of
erase/write operations when DQ polling is not supported.

[1] https://www.cypress.com/file/213346/download

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 50 +++++++++++++++++++++++++++++
 include/linux/mtd/cfi.h             |  5 +++
 2 files changed, 55 insertions(+)

Comments

Joakim Tjernlund March 21, 2019, 6:11 p.m. UTC | #1
On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
> 
> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
> can be use as is. But these devices do not support DQ polling method of
> determining chip ready/good status. These flashes provide Status
> Register whose bits can be polled to know status of flash operation.
> 
> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
> Extended Query version 1.5. Bit 0 of "Software Features supported" field
> of CFI Primary Vendor-Specific Extended Query table indicates
> presence/absence of status register and Bit 1 indicates whether or not
> DQ polling is supported. Using these bits, its possible to determine
> whether flash supports DQ polling or need to use Status Register.
> 
> Add support for polling status register to know device ready/status of
> erase/write operations when DQ polling is not supported.

Isn't this new Status scheme just a copy of Intels(cmdset_0001)?
If so I think the new status impl. in 0002 should borrow from 0001 as this is a
hardened and battle tested impl.

I know other modern 0002 chips supports both old and new impl. of Status and I world
guess that we will see more chips with new Status only.

 Jocke
Sergei Shtylyov March 24, 2019, 4:23 p.m. UTC | #2
Hello!

On 03/21/2019 08:45 PM, Vignesh Raghavendra wrote:

> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
> can be use as is. But these devices do not support DQ polling method of
> determining chip ready/good status. These flashes provide Status
> Register whose bits can be polled to know status of flash operation.
> 
> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
> Extended Query version 1.5. Bit 0 of "Software Features supported" field
> of CFI Primary Vendor-Specific Extended Query table indicates
> presence/absence of status register and Bit 1 indicates whether or not
> DQ polling is supported. Using these bits, its possible to determine
> whether flash supports DQ polling or need to use Status Register.

   We need?

> Add support for polling status register to know device ready/status of
> erase/write operations when DQ polling is not supported.
> 
> [1] https://www.cypress.com/file/213346/download
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 50 +++++++++++++++++++++++++++++
>  include/linux/mtd/cfi.h             |  5 +++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 72428b6bfc47..29987d8e6c6e 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
[...]
> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
> index cbf77168658c..92ac82ac2329 100644
> --- a/include/linux/mtd/cfi.h
> +++ b/include/linux/mtd/cfi.h
> @@ -233,6 +233,11 @@ struct cfi_pri_amdstd {
>  	uint8_t  VppMin;
>  	uint8_t  VppMax;
>  	uint8_t  TopBottom;
> +	/* Below field are added from version 1.5 */

   The below fields are.

> +	uint8_t  ProgramSuspend;
> +	uint8_t  UnlockBypass;
> +	uint8_t  SecureSiliconSector;
> +	uint8_t  SoftwareFeatures;
>  } __packed;
>  
>  /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */
> 

MBR, Sergei
Raghavendra, Vignesh March 25, 2019, 12:57 p.m. UTC | #3
Hi,

On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
> On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
>>
>> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
>> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
>> can be use as is. But these devices do not support DQ polling method of
>> determining chip ready/good status. These flashes provide Status
>> Register whose bits can be polled to know status of flash operation.
>>
>> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
>> Extended Query version 1.5. Bit 0 of "Software Features supported" field
>> of CFI Primary Vendor-Specific Extended Query table indicates
>> presence/absence of status register and Bit 1 indicates whether or not
>> DQ polling is supported. Using these bits, its possible to determine
>> whether flash supports DQ polling or need to use Status Register.
>>
>> Add support for polling status register to know device ready/status of
>> erase/write operations when DQ polling is not supported.
> 
> Isn't this new Status scheme just a copy of Intels(cmdset_0001)?

Yes, but with one difference: At the end of program/erase operation,
device directly enters status register mode and  starts reflecting
status register content at any address.
The device remains in the read status register state until another
command is written to the device. Therefore there is notion of device is
in "status register read mode" (FL_STATUS) state

But in case of cfi_cmdset_0002, once program/erase operation is
complete, device returns to previous address space overlay from which
operation was started from (mostly read mode)

In order to enter status register overlay mode, Read Status command is
to be written to addr_unlock1(0x555) address. The overlay is in effect
for one read access, specifically the next read access that follows the
Status Register Read command
Therefore code around FL_STATUS state in cfi_cmdset_0001 is not
applicable to cfi_cmdset_0002 as is.


> If so I think the new status impl. in 0002 should borrow from 0001 as this is a
> hardened and battle tested impl.
>

In case of cfi_cmdset_0001.c, program/erase is followed by
inval_cache_and_wait_for_operation() to poll ready bit and based on
status register value, success or the error handling is done.

Most of the code corresponding to inval_cache_and_wait_for_operation()
is already in cfi_cmdset_0002.c. So, whats missing in this patch is
handling and reporting of errors as reflected in status register after
write/erase failures. I will add that in the next version.

But, I don't see much to borrow apart from error handling sequence.
Please, let me know if I missed something.

> I know other modern 0002 chips supports both old and new impl. of Status and I world
> guess that we will see more chips with new Status only.
> 

Agreed. Newer devices would mostly be CFI 1.5.
Joakim Tjernlund March 25, 2019, 1:51 p.m. UTC | #4
On Mon, 2019-03-25 at 18:27 +0530, Vignesh Raghavendra wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Hi,
> 
> On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
> > On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
> > > HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
> > > Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
> > > can be use as is. But these devices do not support DQ polling method of
> > > determining chip ready/good status. These flashes provide Status
> > > Register whose bits can be polled to know status of flash operation.
> > > 
> > > Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
> > > Extended Query version 1.5. Bit 0 of "Software Features supported" field
> > > of CFI Primary Vendor-Specific Extended Query table indicates
> > > presence/absence of status register and Bit 1 indicates whether or not
> > > DQ polling is supported. Using these bits, its possible to determine
> > > whether flash supports DQ polling or need to use Status Register.
> > > 
> > > Add support for polling status register to know device ready/status of
> > > erase/write operations when DQ polling is not supported.
> > 
> > Isn't this new Status scheme just a copy of Intels(cmdset_0001)?
> 
> Yes, but with one difference: At the end of program/erase operation,
> device directly enters status register mode and  starts reflecting
> status register content at any address.
> The device remains in the read status register state until another
> command is written to the device. Therefore there is notion of device is
> in "status register read mode" (FL_STATUS) state

That seems to vary and long time ago RMK added this:
		/* If the flash has finished erasing, then 'erase suspend'
		 * appears to make some (28F320) flash devices switch to
		 * 'read' mode.  Make sure that we switch to 'read status'
		 * mode so we get the right data. --rmk
		 */
		map_write(map, CMD(0x70), chip->in_progress_block_addr);

> 
> But in case of cfi_cmdset_0002, once program/erase operation is
> complete, device returns to previous address space overlay from which
> operation was started from (mostly read mode)

I hope you can do the same as Intel here, issue an explicit Status CMD or you will be in trouble.

Also, I think you need to use the various map_word_xxx as in:
status = map_read(map, chip->in_progress_block_addr);
if (map_word_andequal(map, status, status_OK, status_OK))
	break;
otherwise you will break interleaved setups(like two 8-bit flashes in parallel to
form one 16 bit bus). Maybe this is not supported for CMDSET 0002 ?

 Jocke

> 
> In order to enter status register overlay mode, Read Status command is
> to be written to addr_unlock1(0x555) address. The overlay is in effect
> for one read access, specifically the next read access that follows the
> Status Register Read command
> Therefore code around FL_STATUS state in cfi_cmdset_0001 is not
> applicable to cfi_cmdset_0002 as is.
> 
> 
> > If so I think the new status impl. in 0002 should borrow from 0001 as this is a
> > hardened and battle tested impl.
> > 
> 
> In case of cfi_cmdset_0001.c, program/erase is followed by
> inval_cache_and_wait_for_operation() to poll ready bit and based on
> status register value, success or the error handling is done.
> 
> Most of the code corresponding to inval_cache_and_wait_for_operation()
> is already in cfi_cmdset_0002.c. So, whats missing in this patch is
> handling and reporting of errors as reflected in status register after
> write/erase failures. I will add that in the next version.
> 
> But, I don't see much to borrow apart from error handling sequence.
> Please, let me know if I missed something.
> 
> > I know other modern 0002 chips supports both old and new impl. of Status and I world
> > guess that we will see more chips with new Status only.
> > 
> 
> Agreed. Newer devices would mostly be CFI 1.5.
> 
> --
> Regards
> Vignesh
Raghavendra, Vignesh March 25, 2019, 5:06 p.m. UTC | #5
On 25/03/19 7:21 PM, Joakim Tjernlund wrote:
> On Mon, 2019-03-25 at 18:27 +0530, Vignesh Raghavendra wrote:
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>>
>> Hi,
>>
>> On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
>>> On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
>>>> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
>>>> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
>>>> can be use as is. But these devices do not support DQ polling method of
>>>> determining chip ready/good status. These flashes provide Status
>>>> Register whose bits can be polled to know status of flash operation.
>>>>
>>>> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
>>>> Extended Query version 1.5. Bit 0 of "Software Features supported" field
>>>> of CFI Primary Vendor-Specific Extended Query table indicates
>>>> presence/absence of status register and Bit 1 indicates whether or not
>>>> DQ polling is supported. Using these bits, its possible to determine
>>>> whether flash supports DQ polling or need to use Status Register.
>>>>
>>>> Add support for polling status register to know device ready/status of
>>>> erase/write operations when DQ polling is not supported.
>>>
>>> Isn't this new Status scheme just a copy of Intels(cmdset_0001)?
>>
>> Yes, but with one difference: At the end of program/erase operation,
>> device directly enters status register mode and  starts reflecting
>> status register content at any address.
>> The device remains in the read status register state until another
>> command is written to the device. Therefore there is notion of device is
>> in "status register read mode" (FL_STATUS) state
> 
> That seems to vary and long time ago RMK added this:
> 		/* If the flash has finished erasing, then 'erase suspend'
> 		 * appears to make some (28F320) flash devices switch to
> 		 * 'read' mode.  Make sure that we switch to 'read status'
> 		 * mode so we get the right data. --rmk
> 		 */
> 		map_write(map, CMD(0x70), chip->in_progress_block_addr);
> 

This behavior is expected with cmdset_0001. Because "The device remains
in the read status register state until another command is written",
therefore "erase suspend' command after erase completion will switch
device to read mode. And therefore read status is safe thing to do for
cmdset_0001.

But in case of cmdset_0002 erase completion will not put device to read
status mode and therefore no special status tracking is required.

>>
>> But in case of cfi_cmdset_0002, once program/erase operation is
>> complete, device returns to previous address space overlay from which
>> operation was started from (mostly read mode)
> 
> I hope you can do the same as Intel here, issue an explicit Status CMD or you will be in trouble.

Even if we issue Read Status command to enter read status mode, any
single subsequent read will put device back to read mode. So, sending
explicit Status CMD is of not much use.

As long as cmdset_0002 driver ensures sending Read Status cmd and next
single read can be done in one go (ie. mutex held), I don't see any
trouble here. This is already take care off.

> 
> Also, I think you need to use the various map_word_xxx as in:
> status = map_read(map, chip->in_progress_block_addr);
> if (map_word_andequal(map, status, status_OK, status_OK))
> 	break;

Yes, I will fixup this patch to use map_word_* wherever necessary in the
next revision.


> otherwise you will break interleaved setups(like two 8-bit flashes in parallel to
> form one 16 bit bus). Maybe this is not supported for CMDSET 0002 ?
> 

Interleaved is indeed supported by cmdset_0002. Thanks for pointing that
out!

>  Jocke
> 
>>
>> In order to enter status register overlay mode, Read Status command is
>> to be written to addr_unlock1(0x555) address. The overlay is in effect
>> for one read access, specifically the next read access that follows the
>> Status Register Read command
>> Therefore code around FL_STATUS state in cfi_cmdset_0001 is not
>> applicable to cfi_cmdset_0002 as is.
>>
>>
>>> If so I think the new status impl. in 0002 should borrow from 0001 as this is a
>>> hardened and battle tested impl.
>>>
>>
>> In case of cfi_cmdset_0001.c, program/erase is followed by
>> inval_cache_and_wait_for_operation() to poll ready bit and based on
>> status register value, success or the error handling is done.
>>
>> Most of the code corresponding to inval_cache_and_wait_for_operation()
>> is already in cfi_cmdset_0002.c. So, whats missing in this patch is
>> handling and reporting of errors as reflected in status register after
>> write/erase failures. I will add that in the next version.
>>
>> But, I don't see much to borrow apart from error handling sequence.
>> Please, let me know if I missed something.
>>
>>> I know other modern 0002 chips supports both old and new impl. of Status and I world
>>> guess that we will see more chips with new Status only.
>>>
>>
>> Agreed. Newer devices would mostly be CFI 1.5.
>>
>> --
>> Regards
>> Vignesh
>
Joakim Tjernlund March 25, 2019, 5:24 p.m. UTC | #6
On Mon, 2019-03-25 at 22:36 +0530, Vignesh Raghavendra wrote:
> 
> On 25/03/19 7:21 PM, Joakim Tjernlund wrote:
> > On Mon, 2019-03-25 at 18:27 +0530, Vignesh Raghavendra wrote:
> > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > > 
> > > 
> > > Hi,
> > > 
> > > On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
> > > > On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
> > > > > HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
> > > > > Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
> > > > > can be use as is. But these devices do not support DQ polling method of
> > > > > determining chip ready/good status. These flashes provide Status
> > > > > Register whose bits can be polled to know status of flash operation.
> > > > > 
> > > > > Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
> > > > > Extended Query version 1.5. Bit 0 of "Software Features supported" field
> > > > > of CFI Primary Vendor-Specific Extended Query table indicates
> > > > > presence/absence of status register and Bit 1 indicates whether or not
> > > > > DQ polling is supported. Using these bits, its possible to determine
> > > > > whether flash supports DQ polling or need to use Status Register.
> > > > > 
> > > > > Add support for polling status register to know device ready/status of
> > > > > erase/write operations when DQ polling is not supported.
> > > > 
> > > > Isn't this new Status scheme just a copy of Intels(cmdset_0001)?
> > > 
> > > Yes, but with one difference: At the end of program/erase operation,
> > > device directly enters status register mode and  starts reflecting
> > > status register content at any address.
> > > The device remains in the read status register state until another
> > > command is written to the device. Therefore there is notion of device is
> > > in "status register read mode" (FL_STATUS) state
> > 
> > That seems to vary and long time ago RMK added this:
> > 		/* If the flash has finished erasing, then 'erase suspend'
> > 		 * appears to make some (28F320) flash devices switch to
> > 		 * 'read' mode.  Make sure that we switch to 'read status'
> > 		 * mode so we get the right data. --rmk
> > 		 */
> > 		map_write(map, CMD(0x70), chip->in_progress_block_addr);
> > 
> 
> This behavior is expected with cmdset_0001. Because "The device remains
> in the read status register state until another command is written",
> therefore "erase suspend' command after erase completion will switch
> device to read mode. And therefore read status is safe thing to do for
> cmdset_0001.
> 
> But in case of cmdset_0002 erase completion will not put device to read
> status mode and therefore no special status tracking is required.
> 
> > > But in case of cfi_cmdset_0002, once program/erase operation is
> > > complete, device returns to previous address space overlay from which
> > > operation was started from (mostly read mode)
> > 
> > I hope you can do the same as Intel here, issue an explicit Status CMD or you will be in trouble.
> 
> Even if we issue Read Status command to enter read status mode, any
> single subsequent read will put device back to read mode. So, sending
> explicit Status CMD is of not much use.
> 
> As long as cmdset_0002 driver ensures sending Read Status cmd and next
> single read can be done in one go (ie. mutex held), I don't see any
> trouble here. This is already take care off.

Ouch, a non sticky Status sounds borken. Are you sure that nothing can change the
chip between you issue the Status CMD and read out of status bits?
Like if an erase/suspend/resume completes just after Status CMD but before Status readout?

 Jocke

> 
> > Also, I think you need to use the various map_word_xxx as in:
> > status = map_read(map, chip->in_progress_block_addr);
> > if (map_word_andequal(map, status, status_OK, status_OK))
> > 	break;
> 
> Yes, I will fixup this patch to use map_word_* wherever necessary in the
> next revision.
> 
> 
> > otherwise you will break interleaved setups(like two 8-bit flashes in parallel to
> > form one 16 bit bus). Maybe this is not supported for CMDSET 0002 ?
> > 
> 
> Interleaved is indeed supported by cmdset_0002. Thanks for pointing that
> out!
> 
> >  Jocke
> > 
> > > In order to enter status register overlay mode, Read Status command is
> > > to be written to addr_unlock1(0x555) address. The overlay is in effect
> > > for one read access, specifically the next read access that follows the
> > > Status Register Read command
> > > Therefore code around FL_STATUS state in cfi_cmdset_0001 is not
> > > applicable to cfi_cmdset_0002 as is.
> > > 
> > > 
> > > > If so I think the new status impl. in 0002 should borrow from 0001 as this is a
> > > > hardened and battle tested impl.
> > > > 
> > > 
> > > In case of cfi_cmdset_0001.c, program/erase is followed by
> > > inval_cache_and_wait_for_operation() to poll ready bit and based on
> > > status register value, success or the error handling is done.
> > > 
> > > Most of the code corresponding to inval_cache_and_wait_for_operation()
> > > is already in cfi_cmdset_0002.c. So, whats missing in this patch is
> > > handling and reporting of errors as reflected in status register after
> > > write/erase failures. I will add that in the next version.
> > > 
> > > But, I don't see much to borrow apart from error handling sequence.
> > > Please, let me know if I missed something.
> > > 
> > > > I know other modern 0002 chips supports both old and new impl. of Status and I world
> > > > guess that we will see more chips with new Status only.
> > > > 
> > > 
> > > Agreed. Newer devices would mostly be CFI 1.5.
> > > 
> > > --
> > > Regards
> > > Vignesh
Raghavendra, Vignesh April 2, 2019, 9:03 a.m. UTC | #7
On 25/03/19 10:54 PM, Joakim Tjernlund wrote:
> On Mon, 2019-03-25 at 22:36 +0530, Vignesh Raghavendra wrote:
>>
>> On 25/03/19 7:21 PM, Joakim Tjernlund wrote:
>>> On Mon, 2019-03-25 at 18:27 +0530, Vignesh Raghavendra wrote:
>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
>>>>> On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
>>>>>> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
>>>>>> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
>>>>>> can be use as is. But these devices do not support DQ polling method of
>>>>>> determining chip ready/good status. These flashes provide Status
>>>>>> Register whose bits can be polled to know status of flash operation.
>>>>>>
>>>>>> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
>>>>>> Extended Query version 1.5. Bit 0 of "Software Features supported" field
>>>>>> of CFI Primary Vendor-Specific Extended Query table indicates
>>>>>> presence/absence of status register and Bit 1 indicates whether or not
>>>>>> DQ polling is supported. Using these bits, its possible to determine
>>>>>> whether flash supports DQ polling or need to use Status Register.
>>>>>>
>>>>>> Add support for polling status register to know device ready/status of
>>>>>> erase/write operations when DQ polling is not supported.
>>>>>
>>>>> Isn't this new Status scheme just a copy of Intels(cmdset_0001)?
>>>>
>>>> Yes, but with one difference: At the end of program/erase operation,
>>>> device directly enters status register mode and  starts reflecting
>>>> status register content at any address.
>>>> The device remains in the read status register state until another
>>>> command is written to the device. Therefore there is notion of device is
>>>> in "status register read mode" (FL_STATUS) state
>>>
>>> That seems to vary and long time ago RMK added this:
>>> 		/* If the flash has finished erasing, then 'erase suspend'
>>> 		 * appears to make some (28F320) flash devices switch to
>>> 		 * 'read' mode.  Make sure that we switch to 'read status'
>>> 		 * mode so we get the right data. --rmk
>>> 		 */
>>> 		map_write(map, CMD(0x70), chip->in_progress_block_addr);
>>>
>>
>> This behavior is expected with cmdset_0001. Because "The device remains
>> in the read status register state until another command is written",
>> therefore "erase suspend' command after erase completion will switch
>> device to read mode. And therefore read status is safe thing to do for
>> cmdset_0001.
>>
>> But in case of cmdset_0002 erase completion will not put device to read
>> status mode and therefore no special status tracking is required.
>>
>>>> But in case of cfi_cmdset_0002, once program/erase operation is
>>>> complete, device returns to previous address space overlay from which
>>>> operation was started from (mostly read mode)
>>>
>>> I hope you can do the same as Intel here, issue an explicit Status CMD or you will be in trouble.
>>
>> Even if we issue Read Status command to enter read status mode, any
>> single subsequent read will put device back to read mode. So, sending
>> explicit Status CMD is of not much use.
>>
>> As long as cmdset_0002 driver ensures sending Read Status cmd and next
>> single read can be done in one go (ie. mutex held), I don't see any
>> trouble here. This is already take care off.
> 
> Ouch, a non sticky Status sounds borken. Are you sure that nothing can change the
> chip between you issue the Status CMD and read out of status bits?
> Like if an erase/suspend/resume completes just after Status CMD but before Status readout?
> 

Yes, I did some tests(with HyperFlash) and erase/program
completion/suspend in b/w issue of Status CMD but before status readout
does not result in exiting status read address space overlay. So we are
safe here with non sticky Status.
diff mbox series

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 72428b6bfc47..29987d8e6c6e 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -49,6 +49,14 @@ 
 #define SST49LF008A		0x005a
 #define AT49BV6416		0x00d6
 
+/*
+ * Bits of Status Register definition for flash devices that don't
+ * support DQ polling (Eg.: Hyperflash)
+ */
+#define CFI_SR_DRB		BIT(7)
+#define CFI_SR_ESB		BIT(5)
+#define CFI_SR_PSB		BIT(4)
+
 static int cfi_amdstd_read (struct mtd_info *, loff_t, size_t, size_t *, u_char *);
 static int cfi_amdstd_write_words(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
 static int cfi_amdstd_write_buffers(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
@@ -97,6 +105,18 @@  static struct mtd_chip_driver cfi_amdstd_chipdrv = {
 	.module		= THIS_MODULE
 };
 
+/*
+ * Use status register to poll for Erase/write completion when DQ is not
+ * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in
+ * CFI Primary Vendor-Specific Extended Query table 1.5
+ */
+static int cfi_use_status_reg(struct cfi_private *cfi)
+{
+	struct cfi_pri_amdstd *extp = cfi->cmdset_priv;
+
+	return (extp->MinorVersion >= '5') &&
+		(extp->SoftwareFeatures & 0x11) == 1;
+}
 
 /* #define DEBUG_CFI_FEATURES */
 
@@ -744,8 +764,21 @@  static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
  */
 static int __xipram chip_ready(struct map_info *map, unsigned long addr)
 {
+	struct cfi_private *cfi = map->fldrv_priv;
 	map_word d, t;
 
+	if (cfi_use_status_reg(cfi)) {
+		/*
+		 * For chips that support status register, check device
+		 * ready bit
+		 */
+		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
+				 cfi->device_type, NULL);
+		d = map_read(map, addr);
+
+		return d.x[0] & CFI_SR_DRB;
+	}
+
 	d = map_read(map, addr);
 	t = map_read(map, addr);
 
@@ -769,8 +802,25 @@  static int __xipram chip_ready(struct map_info *map, unsigned long addr)
  */
 static int __xipram chip_good(struct map_info *map, unsigned long addr, map_word expected)
 {
+	struct cfi_private *cfi = map->fldrv_priv;
 	map_word oldd, curd;
 
+	if (cfi_use_status_reg(cfi)) {
+		/*
+		 * For chips that support status register, check device
+		 * ready bit and Erase/Program status bit to know if
+		 * operation succeeded.
+		 */
+		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, 0, map, cfi,
+				 cfi->device_type, NULL);
+		curd = map_read(map, addr);
+
+		if (curd.x[0] & CFI_SR_DRB)
+			return !(curd.x[0] & (CFI_SR_PSB | CFI_SR_ESB));
+
+		return 0;
+	}
+
 	oldd = map_read(map, addr);
 	curd = map_read(map, addr);
 
diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
index cbf77168658c..92ac82ac2329 100644
--- a/include/linux/mtd/cfi.h
+++ b/include/linux/mtd/cfi.h
@@ -233,6 +233,11 @@  struct cfi_pri_amdstd {
 	uint8_t  VppMin;
 	uint8_t  VppMax;
 	uint8_t  TopBottom;
+	/* Below field are added from version 1.5 */
+	uint8_t  ProgramSuspend;
+	uint8_t  UnlockBypass;
+	uint8_t  SecureSiliconSector;
+	uint8_t  SoftwareFeatures;
 } __packed;
 
 /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */