diff mbox

[U-Boot,11/11] cfi_flash: return device into read array mode after reading status

Message ID 1458524230-10827-11-git-send-email-anarsoul@gmail.com
State Accepted
Commit edc498c651d97e4b2cbd9a83a93a7f815a93e272
Delegated to: Tom Rini
Headers show

Commit Message

Vasily Khoruzhick March 21, 2016, 1:37 a.m. UTC
Otherwise flash remains in read status mode and it's not possible
to access data on flash.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/mtd/cfi_flash.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Marek Vasut March 21, 2016, 1:54 a.m. UTC | #1
On 03/21/2016 02:37 AM, Vasily Khoruzhick wrote:
> Otherwise flash remains in read status mode and it's not possible
> to access data on flash.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

CCing Stefan and Scott on this one.

> ---
>  drivers/mtd/cfi_flash.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 39932f4..18831c6 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -2203,6 +2203,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
>  						flash_isset (info, sect_cnt,
>  							     FLASH_OFFSET_PROTECT,
>  							     FLASH_STATUS_PROTECT);
> +					flash_write_cmd(info, sect_cnt, 0,
> +							FLASH_CMD_RESET);
>  					break;
>  				case CFI_CMDSET_AMD_EXTENDED:
>  				case CFI_CMDSET_AMD_STANDARD:
> 
s
Stefan Roese March 21, 2016, 4:19 p.m. UTC | #2
Hi Vasily,

On 21.03.2016 02:54, Marek Vasut wrote:
> On 03/21/2016 02:37 AM, Vasily Khoruzhick wrote:
>> Otherwise flash remains in read status mode and it's not possible
>> to access data on flash.
>>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> 
> CCing Stefan and Scott on this one.
> 
>> ---
>>   drivers/mtd/cfi_flash.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>> index 39932f4..18831c6 100644
>> --- a/drivers/mtd/cfi_flash.c
>> +++ b/drivers/mtd/cfi_flash.c
>> @@ -2203,6 +2203,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
>>   						flash_isset (info, sect_cnt,
>>   							     FLASH_OFFSET_PROTECT,
>>   							     FLASH_STATUS_PROTECT);
>> +					flash_write_cmd(info, sect_cnt, 0,
>> +							FLASH_CMD_RESET);
>>   					break;
>>   				case CFI_CMDSET_AMD_EXTENDED:
>>   				case CFI_CMDSET_AMD_STANDARD:
>>

I can't test this patch, since I don't have such a board here
available right now any more. But I'm wondering if this is really
necessary. The driver used to work just fine on Intel Strata
flash chips without this patch.

And looking at the code, a bit later (after the loop) the flash 
is actually reset:

...
		/* round up when converting to ms */
		info->write_tout = (tmp + 999) / 1000;
		info->flash_id = FLASH_MAN_CFI;
		if ((info->interface == FLASH_CFI_X8X16) &&
		    (info->chipwidth == FLASH_CFI_BY8)) {
			/* XXX - Need to test on x8/x16 in parallel. */
			info->portwidth >>= 1;
		}

		flash_write_cmd (info, 0, 0, info->cmd_reset);
	}
...

So what exactly does not work for you with the current version?
Which flash chip are you using?

Thanks,
Stefan
Vasily Khoruzhick March 21, 2016, 7:22 p.m. UTC | #3
On Mon, Mar 21, 2016 at 9:19 AM, Stefan Roese <sr@denx.de> wrote:
> Hi Vasily,

Hi Stefan,

> On 21.03.2016 02:54, Marek Vasut wrote:
>> On 03/21/2016 02:37 AM, Vasily Khoruzhick wrote:
>>> Otherwise flash remains in read status mode and it's not possible
>>> to access data on flash.
>>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>
>> CCing Stefan and Scott on this one.
>>
>>> ---
>>>   drivers/mtd/cfi_flash.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>>> index 39932f4..18831c6 100644
>>> --- a/drivers/mtd/cfi_flash.c
>>> +++ b/drivers/mtd/cfi_flash.c
>>> @@ -2203,6 +2203,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
>>>                                              flash_isset (info, sect_cnt,
>>>                                                           FLASH_OFFSET_PROTECT,
>>>                                                           FLASH_STATUS_PROTECT);
>>> +                                    flash_write_cmd(info, sect_cnt, 0,
>>> +                                                    FLASH_CMD_RESET);
>>>                                      break;
>>>                              case CFI_CMDSET_AMD_EXTENDED:
>>>                              case CFI_CMDSET_AMD_STANDARD:
>>>
>
> I can't test this patch, since I don't have such a board here
> available right now any more. But I'm wondering if this is really
> necessary. The driver used to work just fine on Intel Strata
> flash chips without this patch.
>
> And looking at the code, a bit later (after the loop) the flash
> is actually reset:

It sends status cmd for each erase block to check if it's protected or
not, and apparently this chip (Manufacturer ID 0x000089 Chip ID
0x008865) wants reset for each erase block as well.
Otherwise it returns status data instead of actual data. Btw, that is
exactly what Linux driver does.

> ...
>                 /* round up when converting to ms */
>                 info->write_tout = (tmp + 999) / 1000;
>                 info->flash_id = FLASH_MAN_CFI;
>                 if ((info->interface == FLASH_CFI_X8X16) &&
>                     (info->chipwidth == FLASH_CFI_BY8)) {
>                         /* XXX - Need to test on x8/x16 in parallel. */
>                         info->portwidth >>= 1;
>                 }
>
>                 flash_write_cmd (info, 0, 0, info->cmd_reset);
>         }
> ...
>
> So what exactly does not work for you with the current version?

It returns status page on read for each erase block instead of actual data.

> Which flash chip are you using?

Manufacturer ID 0x000089 Chip ID 0x008865

>
> Thanks,
> Stefan
>
Stefan Roese March 22, 2016, 6:40 a.m. UTC | #4
Hi Vasily,

On 21.03.2016 20:22, Vasily Khoruzhick wrote:
> On Mon, Mar 21, 2016 at 9:19 AM, Stefan Roese <sr@denx.de> wrote:
>> Hi Vasily,
>
> Hi Stefan,
>
>> On 21.03.2016 02:54, Marek Vasut wrote:
>>> On 03/21/2016 02:37 AM, Vasily Khoruzhick wrote:
>>>> Otherwise flash remains in read status mode and it's not possible
>>>> to access data on flash.
>>>>
>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>>
>>> CCing Stefan and Scott on this one.
>>>
>>>> ---
>>>>    drivers/mtd/cfi_flash.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>>>> index 39932f4..18831c6 100644
>>>> --- a/drivers/mtd/cfi_flash.c
>>>> +++ b/drivers/mtd/cfi_flash.c
>>>> @@ -2203,6 +2203,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
>>>>                                               flash_isset (info, sect_cnt,
>>>>                                                            FLASH_OFFSET_PROTECT,
>>>>                                                            FLASH_STATUS_PROTECT);
>>>> +                                    flash_write_cmd(info, sect_cnt, 0,
>>>> +                                                    FLASH_CMD_RESET);
>>>>                                       break;
>>>>                               case CFI_CMDSET_AMD_EXTENDED:
>>>>                               case CFI_CMDSET_AMD_STANDARD:
>>>>
>>
>> I can't test this patch, since I don't have such a board here
>> available right now any more. But I'm wondering if this is really
>> necessary. The driver used to work just fine on Intel Strata
>> flash chips without this patch.
>>
>> And looking at the code, a bit later (after the loop) the flash
>> is actually reset:
>
> It sends status cmd for each erase block to check if it's protected or
> not, and apparently this chip (Manufacturer ID 0x000089 Chip ID
> 0x008865) wants reset for each erase block as well.
> Otherwise it returns status data instead of actual data. Btw, that is
> exactly what Linux driver does.

I see. Okay, then:

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan
Tom Rini March 27, 2016, 10:30 p.m. UTC | #5
On Sun, Mar 20, 2016 at 06:37:10PM -0700, Vasily Khoruzhick wrote:

> Otherwise flash remains in read status mode and it's not possible
> to access data on flash.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> Acked-by: Stefan Roese <sr@denx.de>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 39932f4..18831c6 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -2203,6 +2203,8 @@  ulong flash_get_size (phys_addr_t base, int banknum)
 						flash_isset (info, sect_cnt,
 							     FLASH_OFFSET_PROTECT,
 							     FLASH_STATUS_PROTECT);
+					flash_write_cmd(info, sect_cnt, 0,
+							FLASH_CMD_RESET);
 					break;
 				case CFI_CMDSET_AMD_EXTENDED:
 				case CFI_CMDSET_AMD_STANDARD: