diff mbox series

[U-Boot,v2] sf: bar: Clean BA24 Bank Address Register bit after read/write/erase operation

Message ID 1506336008-21852-1-git-send-email-lukma@denx.de
State Accepted
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series [U-Boot,v2] sf: bar: Clean BA24 Bank Address Register bit after read/write/erase operation | expand

Commit Message

Lukasz Majewski Sept. 25, 2017, 10:40 a.m. UTC
The content of Bank Address Register (BAR) is volatile. It is cleared
after power cycle or reset command (RESET F0h).

Some memories (like e.g. s25fl256s) use it to access memory larger than
0x1000000 (16 MiB).

The problem shows up when one:

1. Reads/writes/erases memory > 16 MiB
2. Calls "reset" u-boot command (which is not causing BAR to be cleared)

In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
Unfortunately, the BA24 bit is still set and hence it receives content
from 0x1000000 (16 MiB) memory address.
As a result the SoC aborts and we hang. Only power cycle can take the
SoC out of this state.

How to reproduce/test:

sf probe; sf erase 0x1200000 0x800000; reset
sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 0x800000; reset
sf probe; sf read 0x11000000 0x1200000 0x800000; reset

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---
Changes in v2:

- Rename cleanup_bar() to clean_bar()
- Rewrite in-code comments
---
 drivers/mtd/spi/spi_flash.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Jagan Teki Sept. 26, 2017, 5:40 a.m. UTC | #1
On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
> The content of Bank Address Register (BAR) is volatile. It is cleared
> after power cycle or reset command (RESET F0h).
>
> Some memories (like e.g. s25fl256s) use it to access memory larger than
> 0x1000000 (16 MiB).
>
> The problem shows up when one:
>
> 1. Reads/writes/erases memory > 16 MiB
> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>
> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
> Unfortunately, the BA24 bit is still set and hence it receives content
> from 0x1000000 (16 MiB) memory address.
> As a result the SoC aborts and we hang. Only power cycle can take the
> SoC out of this state.
>
> How to reproduce/test:
>
> sf probe; sf erase 0x1200000 0x800000; reset
> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 0x800000; reset
> sf probe; sf read 0x11000000 0x1200000 0x800000; reset

Sorry, I didn't follow this test.. are you writing on at 18MiB offset
and doing reset how it will reproduce? because we can write it 0x0 and
after reset the ROM look for 16MiB on this flash this is what you
concern about right?

thanks!
Lukasz Majewski Sept. 26, 2017, 7:07 a.m. UTC | #2
On 09/26/2017 07:40 AM, Jagan Teki wrote:
> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>> The content of Bank Address Register (BAR) is volatile. It is cleared
>> after power cycle or reset command (RESET F0h).
>>
>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>> 0x1000000 (16 MiB).
>>
>> The problem shows up when one:
>>
>> 1. Reads/writes/erases memory > 16 MiB
>> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>>
>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>> Unfortunately, the BA24 bit is still set and hence it receives content
>> from 0x1000000 (16 MiB) memory address.
>> As a result the SoC aborts and we hang. Only power cycle can take the
>> SoC out of this state.
>>
>> How to reproduce/test:
>>
>> sf probe; sf erase 0x1200000 0x800000; reset
>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 0x800000; reset
>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
> 
> Sorry, I didn't follow this test.. are you writing on at 18MiB offset
> and doing reset how it will reproduce? 

Please start with erase test case first.

The problem shows up when you try to access SPI-NOR memory > 16 MiB.
It works with current code, since BAR is adjusted properly.

Then after this access (without removing power), please type 'reset' 
command.

You will observe, that the board will not boot (if SPI-NOR is a booting 
device).

The problem is that ROM bootloader tries to access 0x0 address, but BA24 
(in the SPI-NOR controller)  is programmed from last operation (to 0x1). 
Hence, the ROM Boot reads from 0x1000000 address.


> because we can write it 0x0 and
> after reset the ROM look for 16MiB on this flash this is what you
> concern about right?

The problem is with not clearing bit BA24 after each operation. This 
causes hangs after "reset" command.


> 
> thanks!
>
Lukasz Majewski Sept. 26, 2017, 8:01 a.m. UTC | #3
Hi Jagan,

> On 09/26/2017 07:40 AM, Jagan Teki wrote:
>> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>>> The content of Bank Address Register (BAR) is volatile. It is cleared
>>> after power cycle or reset command (RESET F0h).
>>>
>>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>>> 0x1000000 (16 MiB).
>>>
>>> The problem shows up when one:
>>>
>>> 1. Reads/writes/erases memory > 16 MiB
>>> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>>>
>>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>>> Unfortunately, the BA24 bit is still set and hence it receives content
>>> from 0x1000000 (16 MiB) memory address.
>>> As a result the SoC aborts and we hang. Only power cycle can take the
>>> SoC out of this state.
>>>
>>> How to reproduce/test:
>>>
>>> sf probe; sf erase 0x1200000 0x800000; reset
>>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 
>>> 0x800000; reset
>>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>>
>> Sorry, I didn't follow this test.. are you writing on at 18MiB offset
>> and doing reset how it will reproduce? 
> 
> Please start with erase test case first.
> 
> The problem shows up when you try to access SPI-NOR memory > 16 MiB.
> It works with current code, since BAR is adjusted properly.
> 
> Then after this access (without removing power), please type 'reset' 
> command.
> 
> You will observe, that the board will not boot (if SPI-NOR is a booting 
> device).
> 
> The problem is that ROM bootloader tries to access 0x0 address, but BA24 
> (in the SPI-NOR controller)  is programmed from last operation (to 0x1). 
> Hence, the ROM Boot reads from 0x1000000 address.

And maybe some explanation why and how it was discovered.

I was storing in u-boot rootfs on SPI-NOR. This data crossed 16 MiB 
boundary on my Spansion memory.
After this operation I wanted to move forward with factory setup 
procedure and executed "reset".

The board hanged and I had to power cycle it to have it working again.

> 
> 
>> because we can write it 0x0 and
>> after reset the ROM look for 16MiB on this flash this is what you
>> concern about right?
> 
> The problem is with not clearing bit BA24 after each operation. This 
> causes hangs after "reset" command.
> 
> 
>>
>> thanks!
>>
> 
>
Jagan Teki Sept. 27, 2017, 7:14 a.m. UTC | #4
On Tue, Sep 26, 2017 at 12:37 PM, Łukasz Majewski <lukma@denx.de> wrote:
> On 09/26/2017 07:40 AM, Jagan Teki wrote:
>>
>> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>>>
>>> The content of Bank Address Register (BAR) is volatile. It is cleared
>>> after power cycle or reset command (RESET F0h).
>>>
>>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>>> 0x1000000 (16 MiB).
>>>
>>> The problem shows up when one:
>>>
>>> 1. Reads/writes/erases memory > 16 MiB
>>> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>>>
>>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>>> Unfortunately, the BA24 bit is still set and hence it receives content
>>> from 0x1000000 (16 MiB) memory address.
>>> As a result the SoC aborts and we hang. Only power cycle can take the
>>> SoC out of this state.
>>>
>>> How to reproduce/test:
>>>
>>> sf probe; sf erase 0x1200000 0x800000; reset
>>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000
>>> 0x800000; reset
>>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>>
>>
>> Sorry, I didn't follow this test.. are you writing on at 18MiB offset
>> and doing reset how it will reproduce?
>
>
> Please start with erase test case first.
>
> The problem shows up when you try to access SPI-NOR memory > 16 MiB.
> It works with current code, since BAR is adjusted properly.
>
> Then after this access (without removing power), please type 'reset'
> command.
>
> You will observe, that the board will not boot (if SPI-NOR is a booting
> device).
>
> The problem is that ROM bootloader tries to access 0x0 address, but BA24 (in
> the SPI-NOR controller)  is programmed from last operation (to 0x1). Hence,
> the ROM Boot reads from 0x1000000 address.
>
>
>> because we can write it 0x0 and
>> after reset the ROM look for 16MiB on this flash this is what you
>> concern about right?

Yes, I understand ROM look at 16MiB, but the test-case you have erase
is from 18MiB right, it should be 0x0 on the test case right?

thanks!
Lukasz Majewski Sept. 27, 2017, 7:54 a.m. UTC | #5
Hi Jagan,

> On Tue, Sep 26, 2017 at 12:37 PM, Łukasz Majewski <lukma@denx.de> wrote:
>> On 09/26/2017 07:40 AM, Jagan Teki wrote:
>>>
>>> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>>>>
>>>> The content of Bank Address Register (BAR) is volatile. It is cleared
>>>> after power cycle or reset command (RESET F0h).
>>>>
>>>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>>>> 0x1000000 (16 MiB).
>>>>
>>>> The problem shows up when one:
>>>>
>>>> 1. Reads/writes/erases memory > 16 MiB
>>>> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>>>>
>>>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>>>> Unfortunately, the BA24 bit is still set and hence it receives content
>>>> from 0x1000000 (16 MiB) memory address.
>>>> As a result the SoC aborts and we hang. Only power cycle can take the
>>>> SoC out of this state.
>>>>
>>>> How to reproduce/test:
>>>>
>>>> sf probe; sf erase 0x1200000 0x800000; reset
>>>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000
>>>> 0x800000; reset
>>>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>>>
>>>
>>> Sorry, I didn't follow this test.. are you writing on at 18MiB offset
>>> and doing reset how it will reproduce?
>>
>>
>> Please start with erase test case first.
>>
>> The problem shows up when you try to access SPI-NOR memory > 16 MiB.
>> It works with current code, since BAR is adjusted properly.
>>
>> Then after this access (without removing power), please type 'reset'
>> command.
>>
>> You will observe, that the board will not boot (if SPI-NOR is a booting
>> device).
>>
>> The problem is that ROM bootloader tries to access 0x0 address, but BA24 (in
>> the SPI-NOR controller)  is programmed from last operation (to 0x1). Hence,
>> the ROM Boot reads from 0x1000000 address.
>>
>>
>>> because we can write it 0x0 and
>>> after reset the ROM look for 16MiB on this flash this is what you
>>> concern about right?
> 
> Yes, I understand ROM look at 16MiB, but the test-case you have erase
> is from 18MiB right, it should be 0x0 on the test case right?

No it is correct.

The value 0x12000000 must only be > 16 MiB. After accessing this 
location, the BA24 bit is set.

Then after "reset" the BA24 is still set and hence BOOT ROM reads from 
wrong address.

> 
> thanks!
>
Jagan Teki Sept. 27, 2017, 8:03 a.m. UTC | #6
On Wed, Sep 27, 2017 at 1:24 PM, Łukasz Majewski <lukma@denx.de> wrote:
> Hi Jagan,
>
>
>> On Tue, Sep 26, 2017 at 12:37 PM, Łukasz Majewski <lukma@denx.de> wrote:
>>>
>>> On 09/26/2017 07:40 AM, Jagan Teki wrote:
>>>>
>>>>
>>>> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>>>>>
>>>>>
>>>>> The content of Bank Address Register (BAR) is volatile. It is cleared
>>>>> after power cycle or reset command (RESET F0h).
>>>>>
>>>>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>>>>> 0x1000000 (16 MiB).
>>>>>
>>>>> The problem shows up when one:
>>>>>
>>>>> 1. Reads/writes/erases memory > 16 MiB
>>>>> 2. Calls "reset" u-boot command (which is not causing BAR to be
>>>>> cleared)
>>>>>
>>>>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>>>>> Unfortunately, the BA24 bit is still set and hence it receives content
>>>>> from 0x1000000 (16 MiB) memory address.
>>>>> As a result the SoC aborts and we hang. Only power cycle can take the
>>>>> SoC out of this state.
>>>>>
>>>>> How to reproduce/test:
>>>>>
>>>>> sf probe; sf erase 0x1200000 0x800000; reset
>>>>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000
>>>>> 0x800000; reset
>>>>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>>>>
>>>>
>>>>
>>>> Sorry, I didn't follow this test.. are you writing on at 18MiB offset
>>>> and doing reset how it will reproduce?
>>>
>>>
>>>
>>> Please start with erase test case first.
>>>
>>> The problem shows up when you try to access SPI-NOR memory > 16 MiB.
>>> It works with current code, since BAR is adjusted properly.
>>>
>>> Then after this access (without removing power), please type 'reset'
>>> command.
>>>
>>> You will observe, that the board will not boot (if SPI-NOR is a booting
>>> device).
>>>
>>> The problem is that ROM bootloader tries to access 0x0 address, but BA24
>>> (in
>>> the SPI-NOR controller)  is programmed from last operation (to 0x1).
>>> Hence,
>>> the ROM Boot reads from 0x1000000 address.
>>>
>>>
>>>> because we can write it 0x0 and
>>>> after reset the ROM look for 16MiB on this flash this is what you
>>>> concern about right?
>>
>>
>> Yes, I understand ROM look at 16MiB, but the test-case you have erase
>> is from 18MiB right, it should be 0x0 on the test case right?
>
>
> No it is correct.
>
> The value 0x12000000 must only be > 16 MiB. After accessing this location,
> the BA24 bit is set.
>
> Then after "reset" the BA24 is still set and hence BOOT ROM reads from wrong
> address.

OK, I was thinking in other way around even that has similar issue.

thanks!
Jagan Teki Sept. 27, 2017, 8:04 a.m. UTC | #7
On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
> The content of Bank Address Register (BAR) is volatile. It is cleared
> after power cycle or reset command (RESET F0h).
>
> Some memories (like e.g. s25fl256s) use it to access memory larger than
> 0x1000000 (16 MiB).
>
> The problem shows up when one:
>
> 1. Reads/writes/erases memory > 16 MiB
> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>
> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
> Unfortunately, the BA24 bit is still set and hence it receives content
> from 0x1000000 (16 MiB) memory address.
> As a result the SoC aborts and we hang. Only power cycle can take the
> SoC out of this state.
>
> How to reproduce/test:
>
> sf probe; sf erase 0x1200000 0x800000; reset
> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 0x800000; reset
> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ---
> Changes in v2:
>
> - Rename cleanup_bar() to clean_bar()
> - Rewrite in-code comments
> ---
>  drivers/mtd/spi/spi_flash.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 34f6888..5b3c974 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -113,6 +113,27 @@ static int write_cr(struct spi_flash *flash, u8 wc)
>  #endif
>
>  #ifdef CONFIG_SPI_FLASH_BAR
> +/*
> + * This "cleanup" is necessary in a situation when one was accessing

Update cleanup with clean_bar

Applied to u-boot-spi/master

thanks!
Lukasz Majewski Sept. 27, 2017, 8:07 a.m. UTC | #8
On 09/27/2017 10:04 AM, Jagan Teki wrote:
> On Mon, Sep 25, 2017 at 4:10 PM, Lukasz Majewski <lukma@denx.de> wrote:
>> The content of Bank Address Register (BAR) is volatile. It is cleared
>> after power cycle or reset command (RESET F0h).
>>
>> Some memories (like e.g. s25fl256s) use it to access memory larger than
>> 0x1000000 (16 MiB).
>>
>> The problem shows up when one:
>>
>> 1. Reads/writes/erases memory > 16 MiB
>> 2. Calls "reset" u-boot command (which is not causing BAR to be cleared)
>>
>> In the above scenario, the SoC ROM sends 0x000000 address to read SPL.
>> Unfortunately, the BA24 bit is still set and hence it receives content
>> from 0x1000000 (16 MiB) memory address.
>> As a result the SoC aborts and we hang. Only power cycle can take the
>> SoC out of this state.
>>
>> How to reproduce/test:
>>
>> sf probe; sf erase 0x1200000 0x800000; reset
>> sf probe; sf erase 0x1200000 0x800000; sf write 0x11000000 0x1200000 0x800000; reset
>> sf probe; sf read 0x11000000 0x1200000 0x800000; reset
>>
>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>
>> ---
>> Changes in v2:
>>
>> - Rename cleanup_bar() to clean_bar()
>> - Rewrite in-code comments
>> ---
>>   drivers/mtd/spi/spi_flash.c | 33 +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>
>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>> index 34f6888..5b3c974 100644
>> --- a/drivers/mtd/spi/spi_flash.c
>> +++ b/drivers/mtd/spi/spi_flash.c
>> @@ -113,6 +113,27 @@ static int write_cr(struct spi_flash *flash, u8 wc)
>>   #endif
>>
>>   #ifdef CONFIG_SPI_FLASH_BAR
>> +/*
>> + * This "cleanup" is necessary in a situation when one was accessing
> 
> Update cleanup with clean_bar
> 
> Applied to u-boot-spi/master

Thanks :-)

> 
> thanks!
>
diff mbox series

Patch

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 34f6888..5b3c974 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -113,6 +113,27 @@  static int write_cr(struct spi_flash *flash, u8 wc)
 #endif
 
 #ifdef CONFIG_SPI_FLASH_BAR
+/*
+ * This "cleanup" is necessary in a situation when one was accessing
+ * spi flash memory > 16 MiB by using Bank Address Register's BA24 bit.
+ *
+ * After it the BA24 bit shall be cleared to allow access to correct
+ * memory region after SW reset (by calling "reset" command).
+ *
+ * Otherwise, the BA24 bit may be left set and then after reset, the
+ * ROM would read/write/erase SPL from 16 MiB * bank_sel address.
+ */
+static int clean_bar(struct spi_flash *flash)
+{
+	u8 cmd, bank_sel = 0;
+
+	if (flash->bank_curr == 0)
+		return 0;
+	cmd = flash->bank_write_cmd;
+
+	return spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1);
+}
+
 static int write_bar(struct spi_flash *flash, u32 offset)
 {
 	u8 cmd, bank_sel;
@@ -339,6 +360,10 @@  int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 		len -= erase_size;
 	}
 
+#ifdef CONFIG_SPI_FLASH_BAR
+	ret = clean_bar(flash);
+#endif
+
 	return ret;
 }
 
@@ -397,6 +422,10 @@  int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		offset += chunk_len;
 	}
 
+#ifdef CONFIG_SPI_FLASH_BAR
+	ret = clean_bar(flash);
+#endif
+
 	return ret;
 }
 
@@ -500,6 +529,10 @@  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		data += read_len;
 	}
 
+#ifdef CONFIG_SPI_FLASH_BAR
+	ret = clean_bar(flash);
+#endif
+
 	free(cmd);
 	return ret;
 }