[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

+ Lukasz (as a reviewer of my patch[1])

On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> This is the patch[1] for 4-byte addressing, but I would wonder how can proceed
> operations with 4-byte if we disable during probe.
> 
> [1] http://git.denx.de/?p=u-boot-
> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca

OK, so your patch does something different than what I did.

I was trying to keep the change to U-Boot as small as possible, only
fixing this issue I was seeing:

After a soft-reboot where the SPI chip was not reset, it is left in
4-byte addressing mode (linux uses this mode, obviously). Remember
that 4-byte mode is not a permanent setting, so we can enter and
leave it any time we like by issuing a command.

U-Boot uses the Bank Address Register (BAR) for spi flash chips with
more than 16 MByte, so it impclitly assumes that the chip is in
3-byte address mode. As I see it, your patch is worth a discussion
named "should we use 4-byte addressing mode on spi flash chips?".
I do think this is a better alternative than writing BAR! But this
change probably needs discussion and testing.

Until we discussed and tested that, could we push my patch[1] into
v2018.01? This is really a rather tiny bugfix I need for soft reboot,
compared to using 4-byte address mode.

[1] https://patchwork.ozlabs.org/patch/826919/

Thanks,
Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> + Lukasz (as a reviewer of my patch[1])
>
> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>> This is the patch[1] for 4-byte addressing, but I would wonder how can proceed
>> operations with 4-byte if we disable during probe.
>>
>> [1] http://git.denx.de/?p=u-boot-
>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>
> OK, so your patch does something different than what I did.
>
> I was trying to keep the change to U-Boot as small as possible, only
> fixing this issue I was seeing:
>
> After a soft-reboot where the SPI chip was not reset, it is left in
> 4-byte addressing mode (linux uses this mode, obviously). Remember
> that 4-byte mode is not a permanent setting, so we can enter and
> leave it any time we like by issuing a command.
>
> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
> more than 16 MByte, so it impclitly assumes that the chip is in
> 3-byte address mode. As I see it, your patch is worth a discussion
> named "should we use 4-byte addressing mode on spi flash chips?".
> I do think this is a better alternative than writing BAR! But this
> change probably needs discussion and testing.

OK, will review your patch.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Prabhakar Kushwaha]

Dear Jagan, Simon,

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Jagan Teki
> Sent: Thursday, December 07, 2017 11:19 AM
> To: Goldschmidt Simon <sgoldschmidt@de.pepperl-fuchs.com>
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode
> 
> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> > + Lukasz (as a reviewer of my patch[1])
> >
> > On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> >> This is the patch[1] for 4-byte addressing, but I would wonder how can
> proceed
> >> operations with 4-byte if we disable during probe.
> >>
> >> [1]
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.denx
> .de%2F%3Fp%3Du-boot-
> &data=02%7C01%7Cprabhakar.kushwaha%40nxp.com%7Ca37e67c0f5fd431396
> 5f08d53d3649b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364
> 82225771650679&sdata=CBQkKDXTE1g1mvEbYuyiBApW2NTxQFCirGeJV9uzX8E
> %3D&reserved=0
> >> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
> >
> > OK, so your patch does something different than what I did.
> >
> > I was trying to keep the change to U-Boot as small as possible, only
> > fixing this issue I was seeing:
> >
> > After a soft-reboot where the SPI chip was not reset, it is left in
> > 4-byte addressing mode (linux uses this mode, obviously). Remember
> > that 4-byte mode is not a permanent setting, so we can enter and
> > leave it any time we like by issuing a command.
> >
> > U-Boot uses the Bank Address Register (BAR) for spi flash chips with
> > more than 16 MByte, so it impclitly assumes that the chip is in
> > 3-byte address mode. As I see it, your patch is worth a discussion
> > named "should we use 4-byte addressing mode on spi flash chips?".
> > I do think this is a better alternative than writing BAR! But this
> > change probably needs discussion and testing.
> 
> OK, will review your patch.
> 

Other solution to this problem could have been "adding support of 4byte addressing". 

There will always be a requirement of supporting >16MB flash.  

--pk 

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

+ Marek for the socfpga platform, see below

On 07.12.2017 06:49, Jagan Teki wrote:
> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>> + Lukasz (as a reviewer of my patch[1])
>>
>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>> This is the patch[1] for 4-byte addressing, but I would wonder how can proceed
>>> operations with 4-byte if we disable during probe.
>>>
>>> [1] http://git.denx.de/?p=u-boot-
>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>
>> OK, so your patch does something different than what I did.
>>
>> I was trying to keep the change to U-Boot as small as possible, only
>> fixing this issue I was seeing:
>>
>> After a soft-reboot where the SPI chip was not reset, it is left in
>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>> that 4-byte mode is not a permanent setting, so we can enter and
>> leave it any time we like by issuing a command.
>>
>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>> more than 16 MByte, so it impclitly assumes that the chip is in
>> 3-byte address mode. As I see it, your patch is worth a discussion
>> named "should we use 4-byte addressing mode on spi flash chips?".
>> I do think this is a better alternative than writing BAR! But this
>> change probably needs discussion and testing.
> 
> OK, will review your patch.

Any update here? The last input on this is about five months old! This 
is the last patch I need to run my socfpga board from qspi.

I added Marek to the discussion as at least the SoCrates board does not 
have a reset connected to the qspi chip and needs this as well. Note 
that the system boot rom does not have a problem with the chip being in 
4-byte mode but SPL fails to load U-Boot from qspi.

I'm open to other solutions for this problem (like using 4-byte 
opcodes), but since they did not emerge, yet, can we please push this 
fix to get it working? Also, I don't want to wait for the long pending 
Linux backport of spi flash support: this is a bug right now, let's fix 
it independently of new features!

Thanks,
Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> + Marek for the socfpga platform, see below
>
> On 07.12.2017 06:49, Jagan Teki wrote:
>>
>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>
>>> + Lukasz (as a reviewer of my patch[1])
>>>
>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>
>>>> This is the patch[1] for 4-byte addressing, but I would wonder how can
>>>> proceed
>>>> operations with 4-byte if we disable during probe.
>>>>
>>>> [1] http://git.denx.de/?p=u-boot-
>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>
>>>
>>> OK, so your patch does something different than what I did.
>>>
>>> I was trying to keep the change to U-Boot as small as possible, only
>>> fixing this issue I was seeing:
>>>
>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>> that 4-byte mode is not a permanent setting, so we can enter and
>>> leave it any time we like by issuing a command.
>>>
>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>> I do think this is a better alternative than writing BAR! But this
>>> change probably needs discussion and testing.
>>
>>
>> OK, will review your patch.
>
>
> Any update here? The last input on this is about five months old! This is
> the last patch I need to run my socfpga board from qspi.
>
> I added Marek to the discussion as at least the SoCrates board does not have
> a reset connected to the qspi chip and needs this as well. Note that the
> system boot rom does not have a problem with the chip being in 4-byte mode
> but SPL fails to load U-Boot from qspi.

Does Linux do this stuff? say my flash in 4-byte and flashed SPL and rebooted.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 14.05.2018 09:22, Jagan Teki wrote:
> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>> + Marek for the socfpga platform, see below
>>
>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>
>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>> + Lukasz (as a reviewer of my patch[1])
>>>>
>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>
>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how can
>>>>> proceed
>>>>> operations with 4-byte if we disable during probe.
>>>>>
>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>
>>>>
>>>> OK, so your patch does something different than what I did.
>>>>
>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>> fixing this issue I was seeing:
>>>>
>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>> leave it any time we like by issuing a command.
>>>>
>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>> I do think this is a better alternative than writing BAR! But this
>>>> change probably needs discussion and testing.
>>>
>>>
>>> OK, will review your patch.
>>
>>
>> Any update here? The last input on this is about five months old! This is
>> the last patch I need to run my socfpga board from qspi.
>>
>> I added Marek to the discussion as at least the SoCrates board does not have
>> a reset connected to the qspi chip and needs this as well. Note that the
>> system boot rom does not have a problem with the chip being in 4-byte mode
>> but SPL fails to load U-Boot from qspi.
> 
> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and rebooted.

Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function 
'set_4byte'. I'm using 4.9 where they don't have support for 4 byte 
opcodes (which is why I'm seeing this bug after all). OK, this is not 
the latest kernel, but it's LTS, so I think U-Boot should handle this 
Kernel.

What happens in Linux (4.9) is that depending on the flash size, 4-byte 
mode is *always* enabled. And it stays enabled after soft reboot. So 
consequently, we have to *always* disable it in U-Boot.

In newer versions, they still use 4-byte mode if the flash chip does not 
support 4-byte opcodes. I suppose that would fix it for me, too, but I'm 
stuck with LTS for now.


Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 14.05.2018 09:47, Simon Goldschmidt wrote:
> 
> 
> On 14.05.2018 09:22, Jagan Teki wrote:
>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>> + Marek for the socfpga platform, see below
>>>
>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>
>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>
>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>
>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>
>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how 
>>>>>> can
>>>>>> proceed
>>>>>> operations with 4-byte if we disable during probe.
>>>>>>
>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>
>>>>>
>>>>> OK, so your patch does something different than what I did.
>>>>>
>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>> fixing this issue I was seeing:
>>>>>
>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>> leave it any time we like by issuing a command.
>>>>>
>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>> I do think this is a better alternative than writing BAR! But this
>>>>> change probably needs discussion and testing.
>>>>
>>>>
>>>> OK, will review your patch.
>>>
>>>
>>> Any update here? The last input on this is about five months old! 
>>> This is
>>> the last patch I need to run my socfpga board from qspi.
>>>
>>> I added Marek to the discussion as at least the SoCrates board does 
>>> not have
>>> a reset connected to the qspi chip and needs this as well. Note that the
>>> system boot rom does not have a problem with the chip being in 4-byte 
>>> mode
>>> but SPL fails to load U-Boot from qspi.
>>
>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and 
>> rebooted.
> 
> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function 
> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte 
> opcodes (which is why I'm seeing this bug after all). OK, this is not 
> the latest kernel, but it's LTS, so I think U-Boot should handle this 
> Kernel.
> 
> What happens in Linux (4.9) is that depending on the flash size, 4-byte 
> mode is *always* enabled. And it stays enabled after soft reboot. So 
> consequently, we have to *always* disable it in U-Boot.
> 
> In newer versions, they still use 4-byte mode if the flash chip does not 
> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm 
> stuck with LTS for now.

Do you need any more information here? I'd really love to get this into 
2018.07, finally. As I said before, this is the last patch missing for 
socfpga cyclone5 running from qspi.


Thanks,
Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

Hi Simon,

On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
>
> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>
>>
>>
>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>
>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>> + Marek for the socfpga platform, see below
>>>>
>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>
>>>>>
>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>>
>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>
>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>
>>>>>>>
>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>> can
>>>>>>> proceed
>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>
>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>
>>>>>>
>>>>>>
>>>>>> OK, so your patch does something different than what I did.
>>>>>>
>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>> fixing this issue I was seeing:
>>>>>>
>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>> leave it any time we like by issuing a command.
>>>>>>
>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>> change probably needs discussion and testing.
>>>>>
>>>>>
>>>>>
>>>>> OK, will review your patch.
>>>>
>>>>
>>>>
>>>> Any update here? The last input on this is about five months old! This
>>>> is
>>>> the last patch I need to run my socfpga board from qspi.
>>>>
>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>> have
>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>> mode
>>>> but SPL fails to load U-Boot from qspi.
>>>
>>>
>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>> rebooted.
>>
>>
>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>> (which is why I'm seeing this bug after all). OK, this is not the latest
>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>
>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>> mode is *always* enabled. And it stays enabled after soft reboot. So
>> consequently, we have to *always* disable it in U-Boot.
>>
>> In newer versions, they still use 4-byte mode if the flash chip does not
>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>> stuck with LTS for now.
>
>
> Do you need any more information here? I'd really love to get this into
> 2018.07, finally. As I said before, this is the last patch missing for
> socfpga cyclone5 running from qspi.

The point I'm not clear is we don't have 4-byte addressing (we are
using Bank addressing for > 16MiB) so how come we disable 4-byte
addressing for the sake of other software blocks enabled it? It's like
a hack to me.

Jagan.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

Hi Jagan,

On 21.05.2018 17:09, Jagan Teki wrote:
> Hi Simon,
> 
> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>
>>>
>>>
>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>
>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>
>>>>> + Marek for the socfpga platform, see below
>>>>>
>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>
>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>>> can
>>>>>>>> proceed
>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>
>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>
>>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>>> fixing this issue I was seeing:
>>>>>>>
>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>> leave it any time we like by issuing a command.
>>>>>>>
>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>> change probably needs discussion and testing.
>>>>>>
>>>>>>
>>>>>>
>>>>>> OK, will review your patch.
>>>>>
>>>>>
>>>>>
>>>>> Any update here? The last input on this is about five months old! This
>>>>> is
>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>
>>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>>> have
>>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>>> mode
>>>>> but SPL fails to load U-Boot from qspi.
>>>>
>>>>
>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>> rebooted.
>>>
>>>
>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>>> (which is why I'm seeing this bug after all). OK, this is not the latest
>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>
>>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>> consequently, we have to *always* disable it in U-Boot.
>>>
>>> In newer versions, they still use 4-byte mode if the flash chip does not
>>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>>> stuck with LTS for now.
>>
>>
>> Do you need any more information here? I'd really love to get this into
>> 2018.07, finally. As I said before, this is the last patch missing for
>> socfpga cyclone5 running from qspi.
> 
> The point I'm not clear is we don't have 4-byte addressing (we are
> using Bank addressing for > 16MiB) so how come we disable 4-byte
> addressing for the sake of other software blocks enabled it? It's like
> a hack to me.

I understand your point. However, there *are* SPI chips without a reset 
line. And if linux brings them into 4-byte address mode and then the 
system gets a warm reset e.g. by the watchdog, where do you suggest to 
set the chip back to 3-byte address mode?

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
>
> Hi Jagan,
>
>
> On 21.05.2018 17:09, Jagan Teki wrote:
>>
>> Hi Simon,
>>
>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>
>>>
>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>
>>>>
>>>>
>>>>
>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>
>>>>>
>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>>
>>>>>> + Marek for the socfpga platform, see below
>>>>>>
>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>
>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>>>> can
>>>>>>>>> proceed
>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>
>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>
>>>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>>>> fixing this issue I was seeing:
>>>>>>>>
>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>
>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>> change probably needs discussion and testing.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> OK, will review your patch.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Any update here? The last input on this is about five months old! This
>>>>>> is
>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>
>>>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>>>> have
>>>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>>>> mode
>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>
>>>>>
>>>>>
>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>> rebooted.
>>>>
>>>>
>>>>
>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>>>> (which is why I'm seeing this bug after all). OK, this is not the latest
>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>
>>>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>> consequently, we have to *always* disable it in U-Boot.
>>>>
>>>> In newer versions, they still use 4-byte mode if the flash chip does not
>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>>>> stuck with LTS for now.
>>>
>>>
>>>
>>> Do you need any more information here? I'd really love to get this into
>>> 2018.07, finally. As I said before, this is the last patch missing for
>>> socfpga cyclone5 running from qspi.
>>
>>
>> The point I'm not clear is we don't have 4-byte addressing (we are
>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>> addressing for the sake of other software blocks enabled it? It's like
>> a hack to me.
>
>
> I understand your point. However, there *are* SPI chips without a reset line. And if linux brings them into 4-byte address mode and then the system gets a warm reset e.g. by the watchdog, where do you suggest to set the chip back to 3-byte address mode?

What are those chips? what if we have 4-byte addressing mode in
U-Boot, we completely operate these into 3-byte mode by disabling
4-byte mode?

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 30.05.2018 07:10, Jagan Teki wrote:
> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>> Hi Jagan,
>>
>>
>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>
>>> Hi Simon,
>>>
>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>>
>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>
>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>
>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>>>>> can
>>>>>>>>>> proceed
>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>
>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>
>>>>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>
>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>
>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> OK, will review your patch.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Any update here? The last input on this is about five months old! This
>>>>>>> is
>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>
>>>>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>>>>> have
>>>>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>>>>> mode
>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>> rebooted.
>>>>>
>>>>>
>>>>>
>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>>>>> (which is why I'm seeing this bug after all). OK, this is not the latest
>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>
>>>>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>
>>>>> In newer versions, they still use 4-byte mode if the flash chip does not
>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>>>>> stuck with LTS for now.
>>>>
>>>>
>>>>
>>>> Do you need any more information here? I'd really love to get this into
>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>> socfpga cyclone5 running from qspi.
>>>
>>>
>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>> addressing for the sake of other software blocks enabled it? It's like
>>> a hack to me.
>>
>>
>> I understand your point. However, there *are* SPI chips without a reset line. And if linux brings them into 4-byte address mode and then the system gets a warm reset e.g. by the watchdog, where do you suggest to set the chip back to 3-byte address mode?
> 
> What are those chips?

For example the EPCQ256N mounted on the EBV Socrates board (Cyclone V 
SoC), see this doc, pinout is in chapter 1.11.2:
https://www.altera.com/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf


> what if we have 4-byte addressing mode in
> U-Boot, we completely operate these into 3-byte mode by disabling
> 4-byte mode?

Ehrm, we don't have 4-byte addressing mode in U-Boot. That's the 
problem. If we would, we would surely execute the opcode I have added 
and explicitly set the device into 4-byte mode. That would solve the 
problem.

The opcode I have added does *NOT* disable 4-byte mode but just leaves 
it. You can re-enter it at any time. You can see this opcode as setting 
a flag inside the chip that tells it how many addressing bytes it expects.

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Marek Vasut]

On 05/30/2018 07:10 AM, Jagan Teki wrote:
> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>> Hi Jagan,
>>
>>
>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>
>>> Hi Simon,
>>>
>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>>
>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>
>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>
>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>>>>> can
>>>>>>>>>> proceed
>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>
>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>
>>>>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>
>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>
>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> OK, will review your patch.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Any update here? The last input on this is about five months old! This
>>>>>>> is
>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>
>>>>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>>>>> have
>>>>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>>>>> mode
>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>> rebooted.
>>>>>
>>>>>
>>>>>
>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>>>>> (which is why I'm seeing this bug after all). OK, this is not the latest
>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>
>>>>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>
>>>>> In newer versions, they still use 4-byte mode if the flash chip does not
>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>>>>> stuck with LTS for now.
>>>>
>>>>
>>>>
>>>> Do you need any more information here? I'd really love to get this into
>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>> socfpga cyclone5 running from qspi.
>>>
>>>
>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>> addressing for the sake of other software blocks enabled it? It's like
>>> a hack to me.
>>
>>
>> I understand your point. However, there *are* SPI chips without a reset line. And if linux brings them into 4-byte address mode and then the system gets a warm reset e.g. by the watchdog, where do you suggest to set the chip back to 3-byte address mode?
> 
> What are those chips?

Very select few actually have a reset line, which lures HW designers to
believe reseting SPI NOR is optional, which in turn is BS.

> what if we have 4-byte addressing mode in
> U-Boot, we completely operate these into 3-byte mode by disabling
> 4-byte mode?

If you operate large chip in 3B addressing mode, you lose a lot of
performance. This should be fixed in U-Boot too.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 30.05.2018 11:56, Marek Vasut wrote:
> On 05/30/2018 07:10 AM, Jagan Teki wrote:
>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>
>>> Hi Jagan,
>>>
>>>
>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>
>>>>>
>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>
>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>
>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder how
>>>>>>>>>>> can
>>>>>>>>>>> proceed
>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>
>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>
>>>>>>>>>> I was trying to keep the change to U-Boot as small as possible, only
>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>
>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left in
>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>
>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips with
>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK, will review your patch.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Any update here? The last input on this is about five months old! This
>>>>>>>> is
>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>
>>>>>>>> I added Marek to the discussion as at least the SoCrates board does not
>>>>>>>> have
>>>>>>>> a reset connected to the qspi chip and needs this as well. Note that the
>>>>>>>> system boot rom does not have a problem with the chip being in 4-byte
>>>>>>>> mode
>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>>> rebooted.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte opcodes
>>>>>> (which is why I'm seeing this bug after all). OK, this is not the latest
>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>
>>>>>> What happens in Linux (4.9) is that depending on the flash size, 4-byte
>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>
>>>>>> In newer versions, they still use 4-byte mode if the flash chip does not
>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but I'm
>>>>>> stuck with LTS for now.
>>>>>
>>>>>
>>>>>
>>>>> Do you need any more information here? I'd really love to get this into
>>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>>> socfpga cyclone5 running from qspi.
>>>>
>>>>
>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>> addressing for the sake of other software blocks enabled it? It's like
>>>> a hack to me.
>>>
>>>
>>> I understand your point. However, there *are* SPI chips without a reset line. And if linux brings them into 4-byte address mode and then the system gets a warm reset e.g. by the watchdog, where do you suggest to set the chip back to 3-byte address mode?
>>
>> What are those chips?
> 
> Very select few actually have a reset line, which lures HW designers to
> believe reseting SPI NOR is optional, which in turn is BS.
> 
>> what if we have 4-byte addressing mode in
>> U-Boot, we completely operate these into 3-byte mode by disabling
>> 4-byte mode?
> 
> If you operate large chip in 3B addressing mode, you lose a lot of
> performance. This should be fixed in U-Boot too.

Can you elaborate on the performance loss?

Of course I'd also prefer using 4-byte addressing mode or even 4-byte 
opcodes in U-Boot. However, I'm not sure I'll get the time to implement 
this. Especially when knowing that there is a big patch to change all 
this is in the queue... (whatever the status of this is)

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Marek Vasut]

On 05/30/2018 01:18 PM, Simon Goldschmidt wrote:
> 
> 
> On 30.05.2018 11:56, Marek Vasut wrote:
>> On 05/30/2018 07:10 AM, Jagan Teki wrote:
>>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>> Hi Jagan,
>>>>
>>>>
>>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>>
>>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would
>>>>>>>>>>>> wonder how
>>>>>>>>>>>> can
>>>>>>>>>>>> proceed
>>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>>
>>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>>
>>>>>>>>>>> I was trying to keep the change to U-Boot as small as
>>>>>>>>>>> possible, only
>>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>>
>>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is
>>>>>>>>>>> left in
>>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously).
>>>>>>>>>>> Remember
>>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>>
>>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash
>>>>>>>>>>> chips with
>>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a
>>>>>>>>>>> discussion
>>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash
>>>>>>>>>>> chips?".
>>>>>>>>>>> I do think this is a better alternative than writing BAR! But
>>>>>>>>>>> this
>>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, will review your patch.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Any update here? The last input on this is about five months
>>>>>>>>> old! This
>>>>>>>>> is
>>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>>
>>>>>>>>> I added Marek to the discussion as at least the SoCrates board
>>>>>>>>> does not
>>>>>>>>> have
>>>>>>>>> a reset connected to the qspi chip and needs this as well. Note
>>>>>>>>> that the
>>>>>>>>> system boot rom does not have a problem with the chip being in
>>>>>>>>> 4-byte
>>>>>>>>> mode
>>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL
>>>>>>>> and
>>>>>>>> rebooted.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4
>>>>>>> byte opcodes
>>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
>>>>>>> latest
>>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>>
>>>>>>> What happens in Linux (4.9) is that depending on the flash size,
>>>>>>> 4-byte
>>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>>
>>>>>>> In newer versions, they still use 4-byte mode if the flash chip
>>>>>>> does not
>>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too,
>>>>>>> but I'm
>>>>>>> stuck with LTS for now.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Do you need any more information here? I'd really love to get this
>>>>>> into
>>>>>> 2018.07, finally. As I said before, this is the last patch missing
>>>>>> for
>>>>>> socfpga cyclone5 running from qspi.
>>>>>
>>>>>
>>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>>> addressing for the sake of other software blocks enabled it? It's like
>>>>> a hack to me.
>>>>
>>>>
>>>> I understand your point. However, there *are* SPI chips without a
>>>> reset line. And if linux brings them into 4-byte address mode and
>>>> then the system gets a warm reset e.g. by the watchdog, where do you
>>>> suggest to set the chip back to 3-byte address mode?
>>>
>>> What are those chips?
>>
>> Very select few actually have a reset line, which lures HW designers to
>> believe reseting SPI NOR is optional, which in turn is BS.
>>
>>> what if we have 4-byte addressing mode in
>>> U-Boot, we completely operate these into 3-byte mode by disabling
>>> 4-byte mode?
>>
>> If you operate large chip in 3B addressing mode, you lose a lot of
>> performance. This should be fixed in U-Boot too.
> 
> Can you elaborate on the performance loss?
> 
> Of course I'd also prefer using 4-byte addressing mode or even 4-byte
> opcodes in U-Boot. However, I'm not sure I'll get the time to implement
> this. Especially when knowing that there is a big patch to change all
> this is in the queue... (whatever the status of this is)

Depends on the usecase really. If you keep switching the banks, it'll be
a heavy hit. I'm not asking you to implement anything ;-)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Wed, May 30, 2018 at 1:42 PM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
>
>
> On 30.05.2018 07:10, Jagan Teki wrote:
>>
>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>
>>>
>>> Hi Jagan,
>>>
>>>
>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>
>>>>
>>>> Hi Simon,
>>>>
>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>
>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>
>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder
>>>>>>>>>>> how
>>>>>>>>>>> can
>>>>>>>>>>> proceed
>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>
>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>
>>>>>>>>>> I was trying to keep the change to U-Boot as small as possible,
>>>>>>>>>> only
>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>
>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left
>>>>>>>>>> in
>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>
>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips
>>>>>>>>>> with
>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK, will review your patch.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Any update here? The last input on this is about five months old!
>>>>>>>> This
>>>>>>>> is
>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>
>>>>>>>> I added Marek to the discussion as at least the SoCrates board does
>>>>>>>> not
>>>>>>>> have
>>>>>>>> a reset connected to the qspi chip and needs this as well. Note that
>>>>>>>> the
>>>>>>>> system boot rom does not have a problem with the chip being in
>>>>>>>> 4-byte
>>>>>>>> mode
>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>>> rebooted.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte
>>>>>> opcodes
>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
>>>>>> latest
>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>
>>>>>> What happens in Linux (4.9) is that depending on the flash size,
>>>>>> 4-byte
>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>
>>>>>> In newer versions, they still use 4-byte mode if the flash chip does
>>>>>> not
>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but
>>>>>> I'm
>>>>>> stuck with LTS for now.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Do you need any more information here? I'd really love to get this into
>>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>>> socfpga cyclone5 running from qspi.
>>>>
>>>>
>>>>
>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>> addressing for the sake of other software blocks enabled it? It's like
>>>> a hack to me.
>>>
>>>
>>>
>>> I understand your point. However, there *are* SPI chips without a reset
>>> line. And if linux brings them into 4-byte address mode and then the system
>>> gets a warm reset e.g. by the watchdog, where do you suggest to set the chip
>>> back to 3-byte address mode?
>>
>>
>> What are those chips?
>
>
> For example the EPCQ256N mounted on the EBV Socrates board (Cyclone V SoC),
> see this doc, pinout is in chapter 1.11.2:
> https://www.altera.com/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf
>
>
>> what if we have 4-byte addressing mode in
>> U-Boot, we completely operate these into 3-byte mode by disabling
>> 4-byte mode?
>
>
> Ehrm, we don't have 4-byte addressing mode in U-Boot. That's the problem. If
> we would, we would surely execute the opcode I have added and explicitly set
> the device into 4-byte mode. That would solve the problem.

I'm not sure I understand this, how should 4-byte addressing solve the
problem? you claimed in patch that bootrom would need 3-byte
addressing during warm reset ie what the problem is all about right?
what if u-boot operates in 4-bytes and trigger watchdog, should rom
boots from 4-byte addressing.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Marek Vasut]

On 05/30/2018 01:25 PM, Jagan Teki wrote:
> On Wed, May 30, 2018 at 1:42 PM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>>
>> On 30.05.2018 07:10, Jagan Teki wrote:
>>>
>>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>>
>>>> Hi Jagan,
>>>>
>>>>
>>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>>
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>>
>>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder
>>>>>>>>>>>> how
>>>>>>>>>>>> can
>>>>>>>>>>>> proceed
>>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>>
>>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>>
>>>>>>>>>>> I was trying to keep the change to U-Boot as small as possible,
>>>>>>>>>>> only
>>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>>
>>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left
>>>>>>>>>>> in
>>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>>
>>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips
>>>>>>>>>>> with
>>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, will review your patch.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Any update here? The last input on this is about five months old!
>>>>>>>>> This
>>>>>>>>> is
>>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>>
>>>>>>>>> I added Marek to the discussion as at least the SoCrates board does
>>>>>>>>> not
>>>>>>>>> have
>>>>>>>>> a reset connected to the qspi chip and needs this as well. Note that
>>>>>>>>> the
>>>>>>>>> system boot rom does not have a problem with the chip being in
>>>>>>>>> 4-byte
>>>>>>>>> mode
>>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>>>> rebooted.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte
>>>>>>> opcodes
>>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
>>>>>>> latest
>>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>>
>>>>>>> What happens in Linux (4.9) is that depending on the flash size,
>>>>>>> 4-byte
>>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>>
>>>>>>> In newer versions, they still use 4-byte mode if the flash chip does
>>>>>>> not
>>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but
>>>>>>> I'm
>>>>>>> stuck with LTS for now.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Do you need any more information here? I'd really love to get this into
>>>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>>>> socfpga cyclone5 running from qspi.
>>>>>
>>>>>
>>>>>
>>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>>> addressing for the sake of other software blocks enabled it? It's like
>>>>> a hack to me.
>>>>
>>>>
>>>>
>>>> I understand your point. However, there *are* SPI chips without a reset
>>>> line. And if linux brings them into 4-byte address mode and then the system
>>>> gets a warm reset e.g. by the watchdog, where do you suggest to set the chip
>>>> back to 3-byte address mode?
>>>
>>>
>>> What are those chips?
>>
>>
>> For example the EPCQ256N mounted on the EBV Socrates board (Cyclone V SoC),
>> see this doc, pinout is in chapter 1.11.2:
>> https://www.altera.com/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf
>>
>>
>>> what if we have 4-byte addressing mode in
>>> U-Boot, we completely operate these into 3-byte mode by disabling
>>> 4-byte mode?
>>
>>
>> Ehrm, we don't have 4-byte addressing mode in U-Boot. That's the problem. If
>> we would, we would surely execute the opcode I have added and explicitly set
>> the device into 4-byte mode. That would solve the problem.
> 
> I'm not sure I understand this, how should 4-byte addressing solve the
> problem? you claimed in patch that bootrom would need 3-byte
> addressing during warm reset ie what the problem is all about right?
> what if u-boot operates in 4-bytes and trigger watchdog, should rom
> boots from 4-byte addressing.

IMO his board needs a SPI NOR reset, otherwise he will always have
reliability problems with booting. There will always be cornercases
where the CPU reboots and SPI NOR is left in some crappy state and the
machine will just hang. If you screw up the reset routing, your hardware
is DOOMED and there's no fixing it in software.

That said, the 3byte addressing mode in U-Boot is crap too and should've
been fixed since forever by importing the Linux SPI NOR stack.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 30.05.2018 13:25, Jagan Teki wrote:
> On Wed, May 30, 2018 at 1:42 PM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>
>>
>> On 30.05.2018 07:10, Jagan Teki wrote:
>>>
>>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>
>>>>
>>>> Hi Jagan,
>>>>
>>>>
>>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>>
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>>
>>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder
>>>>>>>>>>>> how
>>>>>>>>>>>> can
>>>>>>>>>>>> proceed
>>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>>
>>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>>
>>>>>>>>>>> I was trying to keep the change to U-Boot as small as possible,
>>>>>>>>>>> only
>>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>>
>>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left
>>>>>>>>>>> in
>>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>>
>>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips
>>>>>>>>>>> with
>>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, will review your patch.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Any update here? The last input on this is about five months old!
>>>>>>>>> This
>>>>>>>>> is
>>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>>
>>>>>>>>> I added Marek to the discussion as at least the SoCrates board does
>>>>>>>>> not
>>>>>>>>> have
>>>>>>>>> a reset connected to the qspi chip and needs this as well. Note that
>>>>>>>>> the
>>>>>>>>> system boot rom does not have a problem with the chip being in
>>>>>>>>> 4-byte
>>>>>>>>> mode
>>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>>>> rebooted.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte
>>>>>>> opcodes
>>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
>>>>>>> latest
>>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>>
>>>>>>> What happens in Linux (4.9) is that depending on the flash size,
>>>>>>> 4-byte
>>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>>
>>>>>>> In newer versions, they still use 4-byte mode if the flash chip does
>>>>>>> not
>>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but
>>>>>>> I'm
>>>>>>> stuck with LTS for now.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Do you need any more information here? I'd really love to get this into
>>>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>>>> socfpga cyclone5 running from qspi.
>>>>>
>>>>>
>>>>>
>>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>>> addressing for the sake of other software blocks enabled it? It's like
>>>>> a hack to me.
>>>>
>>>>
>>>>
>>>> I understand your point. However, there *are* SPI chips without a reset
>>>> line. And if linux brings them into 4-byte address mode and then the system
>>>> gets a warm reset e.g. by the watchdog, where do you suggest to set the chip
>>>> back to 3-byte address mode?
>>>
>>>
>>> What are those chips?
>>
>>
>> For example the EPCQ256N mounted on the EBV Socrates board (Cyclone V SoC),
>> see this doc, pinout is in chapter 1.11.2:
>> https://www.altera.com/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf
>>
>>
>>> what if we have 4-byte addressing mode in
>>> U-Boot, we completely operate these into 3-byte mode by disabling
>>> 4-byte mode?
>>
>>
>> Ehrm, we don't have 4-byte addressing mode in U-Boot. That's the problem. If
>> we would, we would surely execute the opcode I have added and explicitly set
>> the device into 4-byte mode. That would solve the problem.
> 
> I'm not sure I understand this, how should 4-byte addressing solve the
> problem? you claimed in patch that bootrom would need 3-byte
> addressing during warm reset ie what the problem is all about right?

It's not the boot rom which needs 3-byte mode but it's SPL (and U-Boot).

Maybe I expressed myself unclear. Let me explain again:

1) Cold reset, qspi chip is in 3-byte addressing mode
2) Boot rom loads SPL from qspi (don't know in which mode that happens)
3) SPL & U-Boot use the chip in 3-byte mode
4) Linux 4.9 is booted, switches the chip into 4-byte mode
    (Note that newer versions seem to use 4-byte opcodes and don't
     touch the mode)
5) Soft reboot, qspi does not get a reset (I know this is not good but 
for now it's just like that.
6) SPL is not loaded from qspi again since the boot rom uses the version 
buffered in onchip SRAM (I think that's strange, too, but that's what 
happens)
7) SPL tries to use the chip in 3-byte mode without changing the mode, 
which, of course, fails.

Now I see three options here:
a) Wire qspi reset to be triggered on warm reset, too: can't do that on 
existing boards
b) Use 4-byte addressing mode or even better, 4-byte opcodes
c) Switch the chip into 3-byte mode

So would a patch to use 4-byte addressing be better accepted?

> what if u-boot operates in 4-bytes and trigger watchdog, should rom
> boots from 4-byte addressing.

As written above, I don't know what the boot rom needs since a reboot 
does not trigger loading from qspi again. Maybe Marek or someone at 
Intel knows more here...

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Simon Goldschmidt]

On 30.05.2018 13:27, Marek Vasut wrote:
> On 05/30/2018 01:25 PM, Jagan Teki wrote:
>> On Wed, May 30, 2018 at 1:42 PM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>
>>>
>>> On 30.05.2018 07:10, Jagan Teki wrote:
>>>>
>>>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>
>>>>>
>>>>> Hi Jagan,
>>>>>
>>>>>
>>>>> On 21.05.2018 17:09, Jagan Teki wrote:
>>>>>>
>>>>>>
>>>>>> Hi Simon,
>>>>>>
>>>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> + Marek for the socfpga platform, see below
>>>>>>>>>>
>>>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
>>>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would wonder
>>>>>>>>>>>>> how
>>>>>>>>>>>>> can
>>>>>>>>>>>>> proceed
>>>>>>>>>>>>> operations with 4-byte if we disable during probe.
>>>>>>>>>>>>>
>>>>>>>>>>>>> [1] http://git.denx.de/?p=u-boot-
>>>>>>>>>>>>> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> OK, so your patch does something different than what I did.
>>>>>>>>>>>>
>>>>>>>>>>>> I was trying to keep the change to U-Boot as small as possible,
>>>>>>>>>>>> only
>>>>>>>>>>>> fixing this issue I was seeing:
>>>>>>>>>>>>
>>>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is left
>>>>>>>>>>>> in
>>>>>>>>>>>> 4-byte addressing mode (linux uses this mode, obviously). Remember
>>>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter and
>>>>>>>>>>>> leave it any time we like by issuing a command.
>>>>>>>>>>>>
>>>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash chips
>>>>>>>>>>>> with
>>>>>>>>>>>> more than 16 MByte, so it impclitly assumes that the chip is in
>>>>>>>>>>>> 3-byte address mode. As I see it, your patch is worth a discussion
>>>>>>>>>>>> named "should we use 4-byte addressing mode on spi flash chips?".
>>>>>>>>>>>> I do think this is a better alternative than writing BAR! But this
>>>>>>>>>>>> change probably needs discussion and testing.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> OK, will review your patch.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Any update here? The last input on this is about five months old!
>>>>>>>>>> This
>>>>>>>>>> is
>>>>>>>>>> the last patch I need to run my socfpga board from qspi.
>>>>>>>>>>
>>>>>>>>>> I added Marek to the discussion as at least the SoCrates board does
>>>>>>>>>> not
>>>>>>>>>> have
>>>>>>>>>> a reset connected to the qspi chip and needs this as well. Note that
>>>>>>>>>> the
>>>>>>>>>> system boot rom does not have a problem with the chip being in
>>>>>>>>>> 4-byte
>>>>>>>>>> mode
>>>>>>>>>> but SPL fails to load U-Boot from qspi.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed SPL and
>>>>>>>>> rebooted.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
>>>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4 byte
>>>>>>>> opcodes
>>>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
>>>>>>>> latest
>>>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
>>>>>>>>
>>>>>>>> What happens in Linux (4.9) is that depending on the flash size,
>>>>>>>> 4-byte
>>>>>>>> mode is *always* enabled. And it stays enabled after soft reboot. So
>>>>>>>> consequently, we have to *always* disable it in U-Boot.
>>>>>>>>
>>>>>>>> In newer versions, they still use 4-byte mode if the flash chip does
>>>>>>>> not
>>>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too, but
>>>>>>>> I'm
>>>>>>>> stuck with LTS for now.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Do you need any more information here? I'd really love to get this into
>>>>>>> 2018.07, finally. As I said before, this is the last patch missing for
>>>>>>> socfpga cyclone5 running from qspi.
>>>>>>
>>>>>>
>>>>>>
>>>>>> The point I'm not clear is we don't have 4-byte addressing (we are
>>>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
>>>>>> addressing for the sake of other software blocks enabled it? It's like
>>>>>> a hack to me.
>>>>>
>>>>>
>>>>>
>>>>> I understand your point. However, there *are* SPI chips without a reset
>>>>> line. And if linux brings them into 4-byte address mode and then the system
>>>>> gets a warm reset e.g. by the watchdog, where do you suggest to set the chip
>>>>> back to 3-byte address mode?
>>>>
>>>>
>>>> What are those chips?
>>>
>>>
>>> For example the EPCQ256N mounted on the EBV Socrates board (Cyclone V SoC),
>>> see this doc, pinout is in chapter 1.11.2:
>>> https://www.altera.com/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf
>>>
>>>
>>>> what if we have 4-byte addressing mode in
>>>> U-Boot, we completely operate these into 3-byte mode by disabling
>>>> 4-byte mode?
>>>
>>>
>>> Ehrm, we don't have 4-byte addressing mode in U-Boot. That's the problem. If
>>> we would, we would surely execute the opcode I have added and explicitly set
>>> the device into 4-byte mode. That would solve the problem.
>>
>> I'm not sure I understand this, how should 4-byte addressing solve the
>> problem? you claimed in patch that bootrom would need 3-byte
>> addressing during warm reset ie what the problem is all about right?
>> what if u-boot operates in 4-bytes and trigger watchdog, should rom
>> boots from 4-byte addressing.
> 
> IMO his board needs a SPI NOR reset, otherwise he will always have
> reliability problems with booting. There will always be cornercases
> where the CPU reboots and SPI NOR is left in some crappy state and the
> machine will just hang. If you screw up the reset routing, your hardware
> is DOOMED and there's no fixing it in software.
> 
> That said, the 3byte addressing mode in U-Boot is crap too and should've
> been fixed since forever by importing the Linux SPI NOR stack.

So isn't that porting in progress? I haven't followed the ML for that 
long though, so I really can't tell what's going on here :-)

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Prabhakar Kushwaha]

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Marek
> Vasut
> Sent: Wednesday, May 30, 2018 4:53 PM
> To: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>; Jagan Teki
> <jagannadh.teki@gmail.com>
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address
> mode
> 
> On 05/30/2018 01:18 PM, Simon Goldschmidt wrote:
> >
> >
> > On 30.05.2018 11:56, Marek Vasut wrote:
> >> On 05/30/2018 07:10 AM, Jagan Teki wrote:
> >>> On Tue, May 22, 2018 at 9:59 AM, Simon Goldschmidt
> >>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> >>>>
> >>>> Hi Jagan,
> >>>>
> >>>>
> >>>> On 21.05.2018 17:09, Jagan Teki wrote:
> >>>>>
> >>>>> Hi Simon,
> >>>>>
> >>>>> On Fri, May 18, 2018 at 12:48 PM, Simon Goldschmidt
> >>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 14.05.2018 09:47, Simon Goldschmidt wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 14.05.2018 09:22, Jagan Teki wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Mon, May 14, 2018 at 12:34 PM, Simon Goldschmidt
> >>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> + Marek for the socfpga platform, see below
> >>>>>>>>>
> >>>>>>>>> On 07.12.2017 06:49, Jagan Teki wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
> >>>>>>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> + Lukasz (as a reviewer of my patch[1])
> >>>>>>>>>>>
> >>>>>>>>>>> On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> This is the patch[1] for 4-byte addressing, but I would
> >>>>>>>>>>>> wonder how can proceed operations with 4-byte if we disable
> >>>>>>>>>>>> during probe.
> >>>>>>>>>>>>
> >>>>>>>>>>>> [1]
> >>>>>>>>>>>> https://emea01.safelinks.protection.outlook.com/?url=http%3
> >>>>>>>>>>>> A%2F%2Fgit.denx.de%2F%3Fp%3Du-boot-
> &data=02%7C01%7Cprabhaka
> >>>>>>>>>>>>
> r.kushwaha%40nxp.com%7C339cc76ac69b4defecd508d5c6201e4d%7C6
> >>>>>>>>>>>>
> 86ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C63663276362809106
> >>>>>>>>>>>>
> 7&sdata=WnAoQnIq48iDPtWamYqKhlaMYSQXm0QdOHp8qBsPxpc%3D&rese
> >>>>>>>>>>>> rved=0
> >>>>>>>>>>>>
> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee1
> >>>>>>>>>>>> 7dca
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> OK, so your patch does something different than what I did.
> >>>>>>>>>>>
> >>>>>>>>>>> I was trying to keep the change to U-Boot as small as
> >>>>>>>>>>> possible, only fixing this issue I was seeing:
> >>>>>>>>>>>
> >>>>>>>>>>> After a soft-reboot where the SPI chip was not reset, it is
> >>>>>>>>>>> left in 4-byte addressing mode (linux uses this mode,
> >>>>>>>>>>> obviously).
> >>>>>>>>>>> Remember
> >>>>>>>>>>> that 4-byte mode is not a permanent setting, so we can enter
> >>>>>>>>>>> and leave it any time we like by issuing a command.
> >>>>>>>>>>>
> >>>>>>>>>>> U-Boot uses the Bank Address Register (BAR) for spi flash
> >>>>>>>>>>> chips with more than 16 MByte, so it impclitly assumes that
> >>>>>>>>>>> the chip is in 3-byte address mode. As I see it, your patch
> >>>>>>>>>>> is worth a discussion named "should we use 4-byte addressing
> >>>>>>>>>>> mode on spi flash chips?".
> >>>>>>>>>>> I do think this is a better alternative than writing BAR!
> >>>>>>>>>>> But this change probably needs discussion and testing.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> OK, will review your patch.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Any update here? The last input on this is about five months
> >>>>>>>>> old! This is the last patch I need to run my socfpga board
> >>>>>>>>> from qspi.
> >>>>>>>>>
> >>>>>>>>> I added Marek to the discussion as at least the SoCrates board
> >>>>>>>>> does not have a reset connected to the qspi chip and needs
> >>>>>>>>> this as well. Note that the system boot rom does not have a
> >>>>>>>>> problem with the chip being in 4-byte mode but SPL fails to
> >>>>>>>>> load U-Boot from qspi.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Does Linux do this stuff? say my flash in 4-byte and flashed
> >>>>>>>> SPL and rebooted.
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Yes. My code is inspired by 'drivers/mtd/spi-bor/spi-nor.c' function
> >>>>>>> 'set_4byte'. I'm using 4.9 where they don't have support for 4
> >>>>>>> byte opcodes
> >>>>>>> (which is why I'm seeing this bug after all). OK, this is not the
> >>>>>>> latest
> >>>>>>> kernel, but it's LTS, so I think U-Boot should handle this Kernel.
> >>>>>>>
> >>>>>>> What happens in Linux (4.9) is that depending on the flash size,
> >>>>>>> 4-byte
> >>>>>>> mode is *always* enabled. And it stays enabled after soft reboot.
> So
> >>>>>>> consequently, we have to *always* disable it in U-Boot.
> >>>>>>>
> >>>>>>> In newer versions, they still use 4-byte mode if the flash chip
> >>>>>>> does not
> >>>>>>> support 4-byte opcodes. I suppose that would fix it for me, too,
> >>>>>>> but I'm
> >>>>>>> stuck with LTS for now.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Do you need any more information here? I'd really love to get this
> >>>>>> into
> >>>>>> 2018.07, finally. As I said before, this is the last patch missing
> >>>>>> for
> >>>>>> socfpga cyclone5 running from qspi.
> >>>>>
> >>>>>
> >>>>> The point I'm not clear is we don't have 4-byte addressing (we are
> >>>>> using Bank addressing for > 16MiB) so how come we disable 4-byte
> >>>>> addressing for the sake of other software blocks enabled it? It's like
> >>>>> a hack to me.
> >>>>
> >>>>
> >>>> I understand your point. However, there *are* SPI chips without a
> >>>> reset line. And if linux brings them into 4-byte address mode and
> >>>> then the system gets a warm reset e.g. by the watchdog, where do you
> >>>> suggest to set the chip back to 3-byte address mode?
> >>>
> >>> What are those chips?
> >>
> >> Very select few actually have a reset line, which lures HW designers to
> >> believe reseting SPI NOR is optional, which in turn is BS.
> >>
> >>> what if we have 4-byte addressing mode in
> >>> U-Boot, we completely operate these into 3-byte mode by disabling
> >>> 4-byte mode?
> >>
> >> If you operate large chip in 3B addressing mode, you lose a lot of
> >> performance. This should be fixed in U-Boot too.
> >
> > Can you elaborate on the performance loss?
> >
> > Of course I'd also prefer using 4-byte addressing mode or even 4-byte
> > opcodes in U-Boot. However, I'm not sure I'll get the time to implement
> > this. Especially when knowing that there is a big patch to change all
> > this is in the queue... (whatever the status of this is)
> 
> Depends on the usecase really. If you keep switching the banks, it'll be
> a heavy hit. I'm not asking you to implement anything ;-)
> 

Why are we relying the bootROM/pre-loader will only be accessing 16MB of memory (using 3B) memory. 
As per system design, u-boot can be at any location of spi-flash.   

There can be such bootROM/pre-loader which transfer control beyond 16MB
Or
This scenario will never ever come and this is limitation from flash point of view.

--pk

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On 2017-10-07 09:23 AM, Prabhakar Kushwaha wrote:
> Dear Jagan, Simon,
> 
> > -----Original Message-----
> > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Jagan
> > Teki
> > Sent: Thursday, December 07, 2017 11:19 AM
> > To: Goldschmidt Simon <sgoldschmidt@de.pepperl-fuchs.com>
> > Cc: u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte
> > address mode
> >
> > On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
> > <sgoldschmidt@de.pepperl-fuchs.com> wrote:
> > > + Lukasz (as a reviewer of my patch[1])
> > >
> > > On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> > >> This is the patch[1] for 4-byte addressing, but I would wonder how
> > >> can
> > proceed
> > >> operations with 4-byte if we disable during probe.
> > >>
> > >> [1]
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.
> > denx
> > .de%2F%3Fp%3Du-boot-
> > &data=02%7C01%7Cprabhakar.kushwaha%40nxp.com%7Ca37e67c0f5fd431396
> > 5f08d53d3649b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364
> >
> 82225771650679&sdata=CBQkKDXTE1g1mvEbYuyiBApW2NTxQFCirGeJV9uzX8E
> > %3D&reserved=0
> > >> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
> > >
> > > OK, so your patch does something different than what I did.
> > >
> > > I was trying to keep the change to U-Boot as small as possible, only
> > > fixing this issue I was seeing:
> > >
> > > After a soft-reboot where the SPI chip was not reset, it is left in
> > > 4-byte addressing mode (linux uses this mode, obviously). Remember
> > > that 4-byte mode is not a permanent setting, so we can enter and
> > > leave it any time we like by issuing a command.
> > >
> > > U-Boot uses the Bank Address Register (BAR) for spi flash chips with
> > > more than 16 MByte, so it impclitly assumes that the chip is in
> > > 3-byte address mode. As I see it, your patch is worth a discussion
> > > named "should we use 4-byte addressing mode on spi flash chips?".
> > > I do think this is a better alternative than writing BAR! But this
> > > change probably needs discussion and testing.
> >
> > OK, will review your patch.
> >
> 
> Other solution to this problem could have been "adding support of 4byte
> addressing".
> 
> There will always be a requirement of supporting >16MB flash.

I would be very happy to have 4-byte addressing support. I just
thought it would be better to first fix the soft-reboot issue I am
having (and at least one other person on this list also had).

Plus, I haven't seen a workign patch for 4-byte addressing on this
list, yet. My patch has no side effects, works and could be merged
for 2018.01.

Thanks,
Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Thu, Nov 30, 2017 at 2:11 PM, Goldschmidt Simon
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> Hi Jagan,
>
> On Fri, Nov 10, 2017 08:04, Jagan Teki wrote:
>>>> I've similar change on my patchwork, since no-one tested Will CC you by re-
>> basing it please have test?
>>>
>>> Yes, of course I'd like to test this. Where do I find your patch?
>>
>> Will rebase and send to ML soon.

This is the patch[1] for 4-byte addressing, but I would wonder how can
proceed operations with 4-byte if we disable during probe.

[1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

Hi Jagan,

On Fri, Nov 10, 2017 08:04, Jagan Teki wrote:
>>> I've similar change on my patchwork, since no-one tested Will CC you by re-
> basing it please have test?
>>
>> Yes, of course I'd like to test this. Where do I find your patch?
> 
> Will rebase and send to ML soon.

Any progress here? Any chance that this one and the other fixes
needed for cadence_qspi to work correctly get included in 2018.01?

The following patches would be required for this in addition to the
3-byte mode switch you wanted to submit:

"spi: cadence_spi: Adopt Linux DT bindings":
https://patchwork.ozlabs.org/project/uboot/list/?series=13864

Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction
when possible" (this one goes on top of the above)
https://patchwork.ozlabs.org/patch/838871/

BTW: the series "spi: cadence_spi_apb: fix using bouncebuf with
writeback dcache" can be closed when the reverting patch above is
applied.

I already sent reviewed-by and tested-by for the first series
above but I can't see them in patchwork.

Please let me know if there's anything missing or anything I can
do to get this pushed.

Thanks,
Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Fri, Nov 10, 2017 at 12:11 PM, Goldschmidt Simon
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> On Mon, Oct 30, 2017 at 7:26 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> I've similar change on my patchwork, since no-one tested Will CC you by re-basing it please have test?
>
> Yes, of course I'd like to test this. Where do I find your patch?

Will rebase and send to ML soon.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On Mon, Oct 30, 2017 at 7:26 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> I've similar change on my patchwork, since no-one tested Will CC you by re-basing it please have test?

Yes, of course I'd like to test this. Where do I find your patch?

Simon

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Jagan Teki]

On Tue, Oct 17, 2017 at 5:17 PM, Goldschmidt Simon
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> On some boards where the spi flash is not reset during warm reboot,
> the chip has to be manually set into 3-byte address mode.
>
> Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
> ---
>  drivers/mtd/spi/sf_internal.h |  2 ++
>  drivers/mtd/spi/spi_flash.c   | 53 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 839cdbe1b0..06dee0a4ea 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -62,6 +62,8 @@ enum spi_nor_option_flags {
>  #define CMD_READ_STATUS1               0x35
>  #define CMD_READ_CONFIG                        0x35
>  #define CMD_FLAG_STATUS                        0x70
> +#define CMD_EN4B                               0xB7
> +#define CMD_EX4B                               0xE9
>
>  /* Bank addr access commands */
>  #ifdef CONFIG_SPI_FLASH_BAR
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 34f68881ed..8db2882075 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -165,6 +165,55 @@ bar_end:
>  }
>  #endif
>
> +static int set_4byte(struct spi_flash *flash, const struct spi_flash_info *info,
> +               u8 enable)
> +{
> +       int ret;
> +       bool need_wren = false;
> +       u8 cmd;
> +
> +       if (flash->size <= SPI_FLASH_16MB_BOUN)
> +               return 0;
> +
> +       switch (JEDEC_MFR(info)) {
> +       case SPI_FLASH_CFI_MFR_STMICRO:
> +               /* Some Micron need WREN command; all will accept it */
> +               need_wren = true;
> +       case SPI_FLASH_CFI_MFR_MACRONIX:
> +       case SPI_FLASH_CFI_MFR_WINBOND:
> +               ret = spi_claim_bus(flash->spi);
> +               if (ret) {
> +                       debug("SF: Unable to claim SPI bus\n");
> +                       return ret;
> +               }
> +
> +               if (need_wren) {
> +                       ret = spi_flash_cmd_write_enable(flash);
> +                       if (ret < 0) {
> +                               debug("SF: enabling write failed\n");
> +                               spi_release_bus(flash->spi);
> +                               return ret;
> +                       }
> +               }
> +
> +               cmd = enable ? CMD_EN4B : CMD_EX4B;
> +               ret = spi_flash_cmd_write(flash->spi, &cmd, 1, NULL, 0);
> +               if (ret) {
> +                       debug("SF: fail to %s 4-byte address mode\n",
> +                               enable ? "enter" : "exit");
> +               }
> +               if (need_wren)
> +                       if (spi_flash_cmd_write_disable(flash) < 0)
> +                               debug("SF: disabling write failed\n");
> +               spi_release_bus(flash->spi);
> +               return ret;
> +       default:
> +               /* Spansion style handled by bar_write  */
> +               break;
> +       }
> +       return 0;
> +}
> +
>  #ifdef CONFIG_SF_DUAL_FLASH
>  static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
>  {
> @@ -1086,6 +1135,10 @@ int spi_flash_scan(struct spi_flash *flash)
>                 flash->flags |= SNOR_F_USE_FSR;
>  #endif
>
> +       /* disable 4-byte addressing if the device exceeds 16MiB */
> +       if (flash->size > SPI_FLASH_16MB_BOUN)
> +               set_4byte(flash, info, 0);
> +

I've similar change on my patchwork, since no-one tested Will CC you
by re-basing it please have test?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Lukasz Majewski]

Hi Simon,

> On some boards where the spi flash is not reset during warm reboot,
> the chip has to be manually set into 3-byte address mode.
> 
> Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>

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

> ---
>  drivers/mtd/spi/sf_internal.h |  2 ++
>  drivers/mtd/spi/spi_flash.c   | 53
> +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55
> insertions(+)
> 
> diff --git a/drivers/mtd/spi/sf_internal.h
> b/drivers/mtd/spi/sf_internal.h index 839cdbe1b0..06dee0a4ea 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -62,6 +62,8 @@ enum spi_nor_option_flags {
>  #define CMD_READ_STATUS1		0x35
>  #define CMD_READ_CONFIG			0x35
>  #define CMD_FLAG_STATUS			0x70
> +#define CMD_EN4B				0xB7
> +#define CMD_EX4B				0xE9
>  
>  /* Bank addr access commands */
>  #ifdef CONFIG_SPI_FLASH_BAR
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 34f68881ed..8db2882075 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -165,6 +165,55 @@ bar_end:
>  }
>  #endif
>  
> +static int set_4byte(struct spi_flash *flash, const struct
> spi_flash_info *info,
> +		u8 enable)
> +{
> +	int ret;
> +	bool need_wren = false;
> +	u8 cmd;
> +
> +	if (flash->size <= SPI_FLASH_16MB_BOUN)
> +		return 0;
> +
> +	switch (JEDEC_MFR(info)) {
> +	case SPI_FLASH_CFI_MFR_STMICRO:
> +		/* Some Micron need WREN command; all will accept it
> */
> +		need_wren = true;
> +	case SPI_FLASH_CFI_MFR_MACRONIX:
> +	case SPI_FLASH_CFI_MFR_WINBOND:
> +		ret = spi_claim_bus(flash->spi);
> +		if (ret) {
> +			debug("SF: Unable to claim SPI bus\n");
> +			return ret;
> +		}
> +
> +		if (need_wren) {
> +			ret = spi_flash_cmd_write_enable(flash);
> +			if (ret < 0) {
> +				debug("SF: enabling write failed\n");
> +				spi_release_bus(flash->spi);
> +				return ret;
> +			}
> +		}
> +
> +		cmd = enable ? CMD_EN4B : CMD_EX4B;
> +		ret = spi_flash_cmd_write(flash->spi, &cmd, 1, NULL,
> 0);
> +		if (ret) {
> +			debug("SF: fail to %s 4-byte address mode\n",
> +				enable ? "enter" : "exit");
> +		}
> +		if (need_wren)
> +			if (spi_flash_cmd_write_disable(flash) < 0)
> +				debug("SF: disabling write
> failed\n");
> +		spi_release_bus(flash->spi);
> +		return ret;
> +	default:
> +		/* Spansion style handled by bar_write  */
> +		break;
> +	}
> +	return 0;
> +}
> +
>  #ifdef CONFIG_SF_DUAL_FLASH
>  static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
>  {
> @@ -1086,6 +1135,10 @@ int spi_flash_scan(struct spi_flash *flash)
>  		flash->flags |= SNOR_F_USE_FSR;
>  #endif
>  
> +	/* disable 4-byte addressing if the device exceeds 16MiB */
> +	if (flash->size > SPI_FLASH_16MB_BOUN)
> +		set_4byte(flash, info, 0);
> +
>  	/* Configure the BAR - discover bank cmds and read current
> bank */ #ifdef CONFIG_SPI_FLASH_BAR
>  	ret = read_bar(flash, info);




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On some boards where the spi flash is not reset during warm reboot,
the chip has to be manually set into 3-byte address mode.

Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
---
 drivers/mtd/spi/sf_internal.h |  2 ++
 drivers/mtd/spi/spi_flash.c   | 53 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 839cdbe1b0..06dee0a4ea 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -62,6 +62,8 @@ enum spi_nor_option_flags {
 #define CMD_READ_STATUS1		0x35
 #define CMD_READ_CONFIG			0x35
 #define CMD_FLAG_STATUS			0x70
+#define CMD_EN4B				0xB7
+#define CMD_EX4B				0xE9
 
 /* Bank addr access commands */
 #ifdef CONFIG_SPI_FLASH_BAR
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 34f68881ed..8db2882075 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -165,6 +165,55 @@ bar_end:
 }
 #endif
 
+static int set_4byte(struct spi_flash *flash, const struct spi_flash_info *info,
+		u8 enable)
+{
+	int ret;
+	bool need_wren = false;
+	u8 cmd;
+
+	if (flash->size <= SPI_FLASH_16MB_BOUN)
+		return 0;
+
+	switch (JEDEC_MFR(info)) {
+	case SPI_FLASH_CFI_MFR_STMICRO:
+		/* Some Micron need WREN command; all will accept it */
+		need_wren = true;
+	case SPI_FLASH_CFI_MFR_MACRONIX:
+	case SPI_FLASH_CFI_MFR_WINBOND:
+		ret = spi_claim_bus(flash->spi);
+		if (ret) {
+			debug("SF: Unable to claim SPI bus\n");
+			return ret;
+		}
+
+		if (need_wren) {
+			ret = spi_flash_cmd_write_enable(flash);
+			if (ret < 0) {
+				debug("SF: enabling write failed\n");
+				spi_release_bus(flash->spi);
+				return ret;
+			}
+		}
+
+		cmd = enable ? CMD_EN4B : CMD_EX4B;
+		ret = spi_flash_cmd_write(flash->spi, &cmd, 1, NULL, 0);
+		if (ret) {
+			debug("SF: fail to %s 4-byte address mode\n",
+				enable ? "enter" : "exit");
+		}
+		if (need_wren)
+			if (spi_flash_cmd_write_disable(flash) < 0)
+				debug("SF: disabling write failed\n");
+		spi_release_bus(flash->spi);
+		return ret;
+	default:
+		/* Spansion style handled by bar_write  */
+		break;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_SF_DUAL_FLASH
 static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
 {
@@ -1086,6 +1135,10 @@ int spi_flash_scan(struct spi_flash *flash)
 		flash->flags |= SNOR_F_USE_FSR;
 #endif
 
+	/* disable 4-byte addressing if the device exceeds 16MiB */
+	if (flash->size > SPI_FLASH_16MB_BOUN)
+		set_4byte(flash, info, 0);
+
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
 	ret = read_bar(flash, info);
-- 
2.12.2.windows.2