* [U-Boot] [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
@ 2019-07-17 6:16 Ashish Kumar
2019-07-18 10:28 ` Jagan Teki
0 siblings, 1 reply; 9+ messages in thread
From: Ashish Kumar @ 2019-07-17 6:16 UTC (permalink / raw)
To: u-boot
s25fs512s and s25fl512s which has same JEDEC ID but only varies
in operating volatge so s25fs512s shares same command set as
mentioned below:
– Serial Command subset and footprint compatible with
S25FL-A, S25FL-K, S25FL-P, and S25FL-S SPI families
– Multi I/O Command subset and footprint compatible with
S25FL-P, and S25FL-S SPI families
Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
---
v3:
1. Add version info, rebase to top.
2. Re-word commit message.
v2:
1. Adding more description in commit msg.
2. consolidating "" and "" in single patch.
drivers/mtd/spi/spi-nor-ids.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 3217fc6..a992966 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
{ INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
- { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR | SPI_NOR_4B_OPCODES) },
{ INFO("s25fl512s_256k", 0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_64k", 0x010220, 0x4d01, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_512k", 0x010220, 0x4f00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-17 6:16 [U-Boot] [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s Ashish Kumar
@ 2019-07-18 10:28 ` Jagan Teki
2019-07-18 10:45 ` [U-Boot] [EXT] " Ashish Kumar
0 siblings, 1 reply; 9+ messages in thread
From: Jagan Teki @ 2019-07-18 10:28 UTC (permalink / raw)
To: u-boot
+ Vignesh
On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar <Ashish.Kumar@nxp.com> wrote:
>
> s25fs512s and s25fl512s which has same JEDEC ID but only varies
> in operating volatge so s25fs512s shares same command set as
> mentioned below:
> – Serial Command subset and footprint compatible with
> S25FL-A, S25FL-K, S25FL-P, and S25FL-S SPI families
> – Multi I/O Command subset and footprint compatible with
> S25FL-P, and S25FL-S SPI families
>
> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> ---
> v3:
> 1. Add version info, rebase to top.
> 2. Re-word commit message.
> v2:
> 1. Adding more description in commit msg.
> 2. consolidating "" and "" in single patch.
>
> drivers/mtd/spi/spi-nor-ids.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 3217fc6..a992966 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR | SPI_NOR_4B_OPCODES) },
I didn't find any diff b/w this with respect to v1 patch, seems like
Vignesh commented some issue? any update on that.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-18 10:28 ` Jagan Teki
@ 2019-07-18 10:45 ` Ashish Kumar
2019-07-18 10:59 ` Jagan Teki
0 siblings, 1 reply; 9+ messages in thread
From: Ashish Kumar @ 2019-07-18 10:45 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Thursday, July 18, 2019 3:59 PM
> To: Ashish Kumar <ashish.kumar@nxp.com>; Vignesh R <vigneshr@ti.com>
> Cc: U-Boot-Denx <u-boot@lists.denx.de>
> Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> SPANSION s25fl512s
>
> Caution: EXT Email
>
> + Vignesh
>
> On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar <Ashish.Kumar@nxp.com>
> wrote:
> >
> > s25fs512s and s25fl512s which has same JEDEC ID but only varies in
> > operating volatge so s25fs512s shares same command set as mentioned
> > below:
> > – Serial Command subset and footprint compatible with S25FL-A,
> > S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command subset
> > and footprint compatible with S25FL-P, and S25FL-S SPI families
> >
> > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> > ---
> > v3:
> > 1. Add version info, rebase to top.
> > 2. Re-word commit message.
> > v2:
> > 1. Adding more description in commit msg.
> > 2. consolidating "" and "" in single patch.
> >
> > drivers/mtd/spi/spi-nor-ids.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi/spi-nor-ids.c
> > b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> > --- a/drivers/mtd/spi/spi-nor-ids.c
> > +++ b/drivers/mtd/spi/spi-nor-ids.c
> > @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> > { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
> > { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> > + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> > + SPI_NOR_4B_OPCODES) },
>
> I didn't find any diff b/w this with respect to v1 patch, seems like Vignesh
> commented some issue? any update on that.
Hi Jagan, Vignesh,
I had updated commit message.
Wrt Vignesh's comment:
To me it seems not valid. This Flash can be used in extended 3-byte addressing mode as well as 4-byte addressing mode.
Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode with CONFIG_SPI_FLASH_BAR being __not__ set.
By adding SPI_NOR_4B_OPCODES code flow will via spi_nor_set_4byte_opcodes() in place of set_4byte().
There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash
http://patchwork.ozlabs.org/patch/1108287/
Regards
Ashish
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-18 10:45 ` [U-Boot] [EXT] " Ashish Kumar
@ 2019-07-18 10:59 ` Jagan Teki
2019-07-18 12:07 ` Vignesh Raghavendra
0 siblings, 1 reply; 9+ messages in thread
From: Jagan Teki @ 2019-07-18 10:59 UTC (permalink / raw)
To: u-boot
On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar <ashish.kumar@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Thursday, July 18, 2019 3:59 PM
> > To: Ashish Kumar <ashish.kumar@nxp.com>; Vignesh R <vigneshr@ti.com>
> > Cc: U-Boot-Denx <u-boot@lists.denx.de>
> > Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> > SPANSION s25fl512s
> >
> > Caution: EXT Email
> >
> > + Vignesh
> >
> > On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar <Ashish.Kumar@nxp.com>
> > wrote:
> > >
> > > s25fs512s and s25fl512s which has same JEDEC ID but only varies in
> > > operating volatge so s25fs512s shares same command set as mentioned
> > > below:
> > > – Serial Command subset and footprint compatible with S25FL-A,
> > > S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command subset
> > > and footprint compatible with S25FL-P, and S25FL-S SPI families
> > >
> > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> > > ---
> > > v3:
> > > 1. Add version info, rebase to top.
> > > 2. Re-word commit message.
> > > v2:
> > > 1. Adding more description in commit msg.
> > > 2. consolidating "" and "" in single patch.
> > >
> > > drivers/mtd/spi/spi-nor-ids.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/spi/spi-nor-ids.c
> > > b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> > > --- a/drivers/mtd/spi/spi-nor-ids.c
> > > +++ b/drivers/mtd/spi/spi-nor-ids.c
> > > @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> > > { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
> > > { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > > - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > > + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> > > + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> > > + SPI_NOR_4B_OPCODES) },
> >
> > I didn't find any diff b/w this with respect to v1 patch, seems like Vignesh
> > commented some issue? any update on that.
>
> Hi Jagan, Vignesh,
>
> I had updated commit message.
>
> Wrt Vignesh's comment:
> To me it seems not valid. This Flash can be used in extended 3-byte addressing mode as well as 4-byte addressing mode.
> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode with CONFIG_SPI_FLASH_BAR being __not__ set.
> By adding SPI_NOR_4B_OPCODES code flow will via spi_nor_set_4byte_opcodes() in place of set_4byte().
>
> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash
> http://patchwork.ozlabs.org/patch/1108287/
Thanks for the information.
Applied to u-boot-spi/master
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-18 10:59 ` Jagan Teki
@ 2019-07-18 12:07 ` Vignesh Raghavendra
2019-07-18 14:27 ` Ashish Kumar
0 siblings, 1 reply; 9+ messages in thread
From: Vignesh Raghavendra @ 2019-07-18 12:07 UTC (permalink / raw)
To: u-boot
On 18/07/19 4:29 PM, Jagan Teki wrote:
> On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar <ashish.kumar@nxp.com> wrote:
>>
>>
>>
>>> -----Original Message-----
>>> From: Jagan Teki <jagan@amarulasolutions.com>
>>> Sent: Thursday, July 18, 2019 3:59 PM
>>> To: Ashish Kumar <ashish.kumar@nxp.com>; Vignesh R <vigneshr@ti.com>
>>> Cc: U-Boot-Denx <u-boot@lists.denx.de>
>>> Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
>>> SPANSION s25fl512s
>>>
>>> Caution: EXT Email
>>>
>>> + Vignesh
>>>
>>> On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar <Ashish.Kumar@nxp.com>
>>> wrote:
>>>>
>>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies in
>>>> operating volatge so s25fs512s shares same command set as mentioned
>>>> below:
>>>> – Serial Command subset and footprint compatible with S25FL-A,
>>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command subset
>>>> and footprint compatible with S25FL-P, and S25FL-S SPI families
>>>>
>>>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
>>>> ---
>>>> v3:
>>>> 1. Add version info, rebase to top.
>>>> 2. Re-word commit message.
>>>> v2:
>>>> 1. Adding more description in commit msg.
>>>> 2. consolidating "" and "" in single patch.
>>>>
>>>> drivers/mtd/spi/spi-nor-ids.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
>>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
>>>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>>> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>>>> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
>>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
>>>> + SPI_NOR_4B_OPCODES) },
>>>
>>> I didn't find any diff b/w this with respect to v1 patch, seems like Vignesh
>>> commented some issue? any update on that.
>>
>> Hi Jagan, Vignesh,
>>
>> I had updated commit message.
>>
>> Wrt Vignesh's comment:
>> To me it seems not valid. This Flash can be used in extended 3-byte addressing mode as well as 4-byte addressing mode.
>> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode with CONFIG_SPI_FLASH_BAR being __not__ set.
>> By adding SPI_NOR_4B_OPCODES code flow will via spi_nor_set_4byte_opcodes() in place of set_4byte().
>>
This does not seem right. Here is the code snippet from spi-nor-core.c::spi_nor_scan():
#ifndef CONFIG_SPI_FLASH_BAR
/* enable 4-byte addressing if the device exceeds 16MiB */
nor->addr_width = 4;
if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
info->flags & SPI_NOR_4B_OPCODES)
spi_nor_set_4byte_opcodes(nor, info);
#else
/* Configure the BAR - discover bank cmds and read current bank */
nor->addr_width = 3;
ret = read_bar(nor, info);
if (ret < 0)
return ret;
#endif
So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is Spansion flash
spi_nor_set_4byte_opcodes() is always called irrespective of SPI_NOR_4B_OPCODES
Also from spi_nor_init():,
if (nor->addr_width == 4 &&
(JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
!(nor->info->flags & SPI_NOR_4B_OPCODES)) {
/*
* If the RESET# pin isn't hooked up properly, or the system
* otherwise doesn't perform a reset command in the boot
* sequence, it's impossible to 100% protect against unexpected
* reboots (e.g., crashes). Warn the user (or hopefully, system
* designer) that this is bad.
*/
if (nor->flags & SNOR_F_BROKEN_RESET)
printf("enabling reset hack; may not recover from unexpected reboots\n");
set_4byte(nor, nor->info, 1);
}
So set_4byte() is not called for Spansion flashes. So I don't see need for this patch.
>> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash
>> http://patchwork.ozlabs.org/patch/1108287/
>
> Thanks for the information.
>
> Applied to u-boot-spi/master
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-18 12:07 ` Vignesh Raghavendra
@ 2019-07-18 14:27 ` Ashish Kumar
2019-07-19 18:11 ` Vignesh Raghavendra
0 siblings, 1 reply; 9+ messages in thread
From: Ashish Kumar @ 2019-07-18 14:27 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr@ti.com>
> Sent: Thursday, July 18, 2019 5:37 PM
> To: Jagan Teki <jagan@amarulasolutions.com>; Ashish Kumar
> <ashish.kumar@nxp.com>
> Cc: U-Boot-Denx <u-boot@lists.denx.de>
> Subject: Re: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> SPANSION s25fl512s
>
> Caution: EXT Email
>
> On 18/07/19 4:29 PM, Jagan Teki wrote:
> > On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar <ashish.kumar@nxp.com>
> wrote:
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Jagan Teki <jagan@amarulasolutions.com>
> >>> Sent: Thursday, July 18, 2019 3:59 PM
> >>> To: Ashish Kumar <ashish.kumar@nxp.com>; Vignesh R
> <vigneshr@ti.com>
> >>> Cc: U-Boot-Denx <u-boot@lists.denx.de>
> >>> Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes
> >>> for SPANSION s25fl512s
> >>>
> >>> Caution: EXT Email
> >>>
> >>> + Vignesh
> >>>
> >>> On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar
> <Ashish.Kumar@nxp.com>
> >>> wrote:
> >>>>
> >>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies in
> >>>> operating volatge so s25fs512s shares same command set as
> mentioned
> >>>> below:
> >>>> – Serial Command subset and footprint compatible with S25FL-A,
> >>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command
> >>>> subset and footprint compatible with S25FL-P, and S25FL-S SPI
> >>>> families
> >>>>
> >>>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> >>>> ---
> >>>> v3:
> >>>> 1. Add version info, rebase to top.
> >>>> 2. Re-word commit message.
> >>>> v2:
> >>>> 1. Adding more description in commit msg.
> >>>> 2. consolidating "" and "" in single patch.
> >>>>
> >>>> drivers/mtd/spi/spi-nor-ids.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
> >>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> >>>> --- a/drivers/mtd/spi/spi-nor-ids.c
> >>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
> >>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> >>>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >>>> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR)
> },
> >>>> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> >>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> >>>> + SPI_NOR_4B_OPCODES) },
> >>>
> >>> I didn't find any diff b/w this with respect to v1 patch, seems like
> >>> Vignesh commented some issue? any update on that.
> >>
> >> Hi Jagan, Vignesh,
> >>
> >> I had updated commit message.
> >>
> >> Wrt Vignesh's comment:
> >> To me it seems not valid. This Flash can be used in extended 3-byte
> addressing mode as well as 4-byte addressing mode.
> >> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode
> with CONFIG_SPI_FLASH_BAR being __not__ set.
> >> By adding SPI_NOR_4B_OPCODES code flow will via
> spi_nor_set_4byte_opcodes() in place of set_4byte().
> >>
>
> This does not seem right. Here is the code snippet from spi-nor-
> core.c::spi_nor_scan():
>
> #ifndef CONFIG_SPI_FLASH_BAR
> /* enable 4-byte addressing if the device exceeds 16MiB */
> nor->addr_width = 4;
> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> info->flags & SPI_NOR_4B_OPCODES)
Hi Vignesh,
1. It seems old, but at least one of the flash(spansion) on my NXP/Freescale board reported manufacturing ID as 02h in place of 01h as result I had to add this flags, I will try to trace that particular board. Reading the flow again it seems this patch is not required due to presence of || operator as long manufacturer ID is reported correctly.
if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
info->flags & SPI_NOR_4B_OPCODES)
2. The very first link on google [1] leads to an APP note from spansion suggesting manufacturing id as 02h, is there a possibility of such flashes in circulation, or am I referring something incorrect ? I will double check this with spansion FAE.
Snippet from APP note named: "AN98488 - Quick Guide to Common Flash Interface"
>>>>>
The Cypress Manufacturer ID for flash devices is 0002h (historically the AMD ID is
0002h and the Fujitsu ID is 0004h; the former Spansion ID is 0002h)."
<<<<<
3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use of this flag here then ?
[1]: https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&cad=rja&uact=8&ved=2ahUKEwjHpY3U0b7jAhVEWX0KHeqSAQkQFjAAegQIBBAB&url=http%3A%2F%2Fwww.cypress.com%2Ffile%2F195291%2Fdownload&usg=AOvVaw0zPEXOjP80J8XVrwmB7TBS
Regards
Ashish
> spi_nor_set_4byte_opcodes(nor, info); #else
> /* Configure the BAR - discover bank cmds and read current bank */
> nor->addr_width = 3;
> ret = read_bar(nor, info);
> if (ret < 0)
> return ret;
> #endif
>
>
> So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is Spansion
> flash
> spi_nor_set_4byte_opcodes() is always called irrespective of
> SPI_NOR_4B_OPCODES
>
> Also from spi_nor_init():,
>
>
> if (nor->addr_width == 4 &&
> (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
> /*
> * If the RESET# pin isn't hooked up properly, or the system
> * otherwise doesn't perform a reset command in the boot
> * sequence, it's impossible to 100% protect against unexpected
> * reboots (e.g., crashes). Warn the user (or hopefully, system
> * designer) that this is bad.
> */
> if (nor->flags & SNOR_F_BROKEN_RESET)
> printf("enabling reset hack; may not recover from unexpected
> reboots\n");
> set_4byte(nor, nor->info, 1);
> }
>
> So set_4byte() is not called for Spansion flashes. So I don't see need for this
> patch.
>
>
> >> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are
> >> using 4-byte addressing mode with this flash
> >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >>
> hwork.ozlabs.org%2Fpatch%2F1108287%2F&data=02%7C01%7Cashish.k
> umar
> >>
> %40nxp.com%7C035eff480dc046fe37c108d70b78630e%7C686ea1d3bc2b4c6f
> a92cd
> >>
> 99c5c301635%7C0%7C0%7C636990484001673615&sdata=dsD80nUUtmJ
> yOoxGvi
> >> iDFaOx1SKPXzI6bzXXpH630i8%3D&reserved=0
> >
> > Thanks for the information.
> >
> > Applied to u-boot-spi/master
> >
>
> --
> Regards
> Vignesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-18 14:27 ` Ashish Kumar
@ 2019-07-19 18:11 ` Vignesh Raghavendra
2019-07-22 5:41 ` Ashish Kumar
0 siblings, 1 reply; 9+ messages in thread
From: Vignesh Raghavendra @ 2019-07-19 18:11 UTC (permalink / raw)
To: u-boot
>>>>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies in
>>>>>> operating volatge so s25fs512s shares same command set as
>> mentioned
>>>>>> below:
>>>>>> – Serial Command subset and footprint compatible with S25FL-A,
>>>>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command
>>>>>> subset and footprint compatible with S25FL-P, and S25FL-S SPI
>>>>>> families
>>>>>>
>>>>>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
>>>>>> ---
>>>>>> v3:
>>>>>> 1. Add version info, rebase to top.
>>>>>> 2. Re-word commit message.
>>>>>> v2:
>>>>>> 1. Adding more description in commit msg.
>>>>>> 2. consolidating "" and "" in single patch.
>>>>>>
>>>>>> drivers/mtd/spi/spi-nor-ids.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
>>>>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
>>>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>>>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
>>>>>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>>>>> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR)
>> },
>>>>>> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>>>> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>>>> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
>>>>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
>>>>>> + SPI_NOR_4B_OPCODES) },
>>>>>
>>>>> I didn't find any diff b/w this with respect to v1 patch, seems like
>>>>> Vignesh commented some issue? any update on that.
>>>>
>>>> Hi Jagan, Vignesh,
>>>>
>>>> I had updated commit message.
>>>>
>>>> Wrt Vignesh's comment:
>>>> To me it seems not valid. This Flash can be used in extended 3-byte
>> addressing mode as well as 4-byte addressing mode.
>>>> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode
>> with CONFIG_SPI_FLASH_BAR being __not__ set.
>>>> By adding SPI_NOR_4B_OPCODES code flow will via
>> spi_nor_set_4byte_opcodes() in place of set_4byte().
>>>>
>>
>> This does not seem right. Here is the code snippet from spi-nor-
>> core.c::spi_nor_scan():
>>
>> #ifndef CONFIG_SPI_FLASH_BAR
>> /* enable 4-byte addressing if the device exceeds 16MiB */
>> nor->addr_width = 4;
>> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>> info->flags & SPI_NOR_4B_OPCODES)
> Hi Vignesh,
>
> 1. It seems old, but at least one of the flash(spansion) on my NXP/Freescale board reported manufacturing ID as 02h in place of 01h as result I had to add this flags, I will try to trace that particular board. Reading the flow again it seems this patch is not required due to presence of || operator as long manufacturer ID is reported correctly.
> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> info->flags & SPI_NOR_4B_OPCODES)
So its just a quirky flash with different manf ID? If manuf ID reads
different then how does it match s25fl512s entry in the spi-nor-ids?
> 2. The very first link on google [1] leads to an APP note from spansion suggesting manufacturing id as 02h, is there a possibility of such flashes in circulation, or am I referring something incorrect ? I will double check this with spansion FAE.
> Snippet from APP note named: "AN98488 - Quick Guide to Common Flash Interface"
>>>>>>
> The Cypress Manufacturer ID for flash devices is 0002h (historically the AMD ID is
> 0002h and the Fujitsu ID is 0004h; the former Spansion ID is 0002h)."
> <<<<<
Cypress seems to use different ID for CFI NOR flash (on Parallel NOR
interface or HyperBus interface). Thats a different standard (JEDEC CFI
ID codes https://www.jedec.org/standards-documents/docs/jep-137b)
SPI NOR flashes from Cypress/Spansion reads 0x1 for Manufacturer ID.
This is based on JEDEC standard
https://www.jedec.org/standards-documents/docs/jep-106ab
> 3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use of this flag here then ?
>
That was an overlook when those were added to Linux spi-nor code and
became part of U-Boot during syncing of framework.
I am not saying that adding SPI_NOR_4B_OPCODES is wrong, because
Spansion flashes does support stateless 4 byte addressing opcodes. But I
am saying that it should not be needed in the first place (as you can
see from code snippets). If you ever need to add SPI_NOR_4B_OPCODES to
spansion flashes (with manf ID 0x1) then I think there is a bug
somewhere in the code which needs to be debugged and understood.
Regards
Vignesh
> [1]: https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&cad=rja&uact=8&ved=2ahUKEwjHpY3U0b7jAhVEWX0KHeqSAQkQFjAAegQIBBAB&url=http%3A%2F%2Fwww.cypress.com%2Ffile%2F195291%2Fdownload&usg=AOvVaw0zPEXOjP80J8XVrwmB7TBS
>
> Regards
> Ashish
>> spi_nor_set_4byte_opcodes(nor, info); #else
>> /* Configure the BAR - discover bank cmds and read current bank */
>> nor->addr_width = 3;
>> ret = read_bar(nor, info);
>> if (ret < 0)
>> return ret;
>> #endif
>>
>>
>> So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is Spansion
>> flash
>> spi_nor_set_4byte_opcodes() is always called irrespective of
>> SPI_NOR_4B_OPCODES
>>
>> Also from spi_nor_init():,
>>
>>
>> if (nor->addr_width == 4 &&
>> (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
>> !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
>> /*
>> * If the RESET# pin isn't hooked up properly, or the system
>> * otherwise doesn't perform a reset command in the boot
>> * sequence, it's impossible to 100% protect against unexpected
>> * reboots (e.g., crashes). Warn the user (or hopefully, system
>> * designer) that this is bad.
>> */
>> if (nor->flags & SNOR_F_BROKEN_RESET)
>> printf("enabling reset hack; may not recover from unexpected
>> reboots\n");
>> set_4byte(nor, nor->info, 1);
>> }
>>
>> So set_4byte() is not called for Spansion flashes. So I don't see need for this
>> patch.
>>
>>
>>>> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are
>>>> using 4-byte addressing mode with this flash
>>>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
>>>>
>> hwork.ozlabs.org%2Fpatch%2F1108287%2F&data=02%7C01%7Cashish.k
>> umar
>>>>
>> %40nxp.com%7C035eff480dc046fe37c108d70b78630e%7C686ea1d3bc2b4c6f
>> a92cd
>>>>
>> 99c5c301635%7C0%7C0%7C636990484001673615&sdata=dsD80nUUtmJ
>> yOoxGvi
>>>> iDFaOx1SKPXzI6bzXXpH630i8%3D&reserved=0
>>>
>>> Thanks for the information.
>>>
>>> Applied to u-boot-spi/master
>>>
>>
>> --
>> Regards
>> Vignesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-19 18:11 ` Vignesh Raghavendra
@ 2019-07-22 5:41 ` Ashish Kumar
2019-07-22 6:06 ` Jagan Teki
0 siblings, 1 reply; 9+ messages in thread
From: Ashish Kumar @ 2019-07-22 5:41 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr@ti.com>
> Sent: Friday, July 19, 2019 11:41 PM
> To: Ashish Kumar <ashish.kumar@nxp.com>; Jagan Teki
> <jagan@amarulasolutions.com>; hs at denx.de
> Cc: U-Boot-Denx <u-boot@lists.denx.de>
> Subject: Re: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> SPANSION s25fl512s
>
> Caution: EXT Email
>
> >>>>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies
> >>>>>> in operating volatge so s25fs512s shares same command set as
> >> mentioned
> >>>>>> below:
> >>>>>> – Serial Command subset and footprint compatible with S25FL-A,
> >>>>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command
> >>>>>> subset and footprint compatible with S25FL-P, and S25FL-S SPI
> >>>>>> families
> >>>>>>
> >>>>>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> >>>>>> ---
> >>>>>> v3:
> >>>>>> 1. Add version info, rebase to top.
> >>>>>> 2. Re-word commit message.
> >>>>>> v2:
> >>>>>> 1. Adding more description in commit msg.
> >>>>>> 2. consolidating "" and "" in single patch.
> >>>>>>
> >>>>>> drivers/mtd/spi/spi-nor-ids.c | 2 +-
> >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
> >>>>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> >>>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
> >>>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
> >>>>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> >>>>>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >>>>>> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
> >>>>>> USE_CLSR)
> >> },
> >>>>>> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>>>> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>>>> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024,
> >>>>>> + 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> >>>>>> + SPI_NOR_4B_OPCODES) },
> >>>>>
> >>>>> I didn't find any diff b/w this with respect to v1 patch, seems
> >>>>> like Vignesh commented some issue? any update on that.
> >>>>
> >>>> Hi Jagan, Vignesh,
> >>>>
> >>>> I had updated commit message.
> >>>>
> >>>> Wrt Vignesh's comment:
> >>>> To me it seems not valid. This Flash can be used in extended 3-byte
> >> addressing mode as well as 4-byte addressing mode.
> >>>> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode
> >> with CONFIG_SPI_FLASH_BAR being __not__ set.
> >>>> By adding SPI_NOR_4B_OPCODES code flow will via
> >> spi_nor_set_4byte_opcodes() in place of set_4byte().
> >>>>
> >>
> >> This does not seem right. Here is the code snippet from spi-nor-
> >> core.c::spi_nor_scan():
> >>
> >> #ifndef CONFIG_SPI_FLASH_BAR
> >> /* enable 4-byte addressing if the device exceeds 16MiB */
> >> nor->addr_width = 4;
> >> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> >> info->flags & SPI_NOR_4B_OPCODES)
> > Hi Vignesh,
> >
> > 1. It seems old, but at least one of the flash(spansion) on my NXP/Freescale
> board reported manufacturing ID as 02h in place of 01h as result I had to
> add this flags, I will try to trace that particular board. Reading the flow again
> it seems this patch is not required due to presence of || operator as long
> manufacturer ID is reported correctly.
> > if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> > info->flags & SPI_NOR_4B_OPCODES)
>
> So its just a quirky flash with different manf ID? If manuf ID reads different
> then how does it match s25fl512s entry in the spi-nor-ids?
>
> > 2. The very first link on google [1] leads to an APP note from spansion
> suggesting manufacturing id as 02h, is there a possibility of such flashes in
> circulation, or am I referring something incorrect ? I will double check this
> with spansion FAE.
> > Snippet from APP note named: "AN98488 - Quick Guide to Common Flash
> Interface"
> >>>>>>
> > The Cypress Manufacturer ID for flash devices is 0002h (historically
> > the AMD ID is 0002h and the Fujitsu ID is 0004h; the former Spansion ID is
> 0002h)."
> > <<<<<
>
> Cypress seems to use different ID for CFI NOR flash (on Parallel NOR interface
> or HyperBus interface). Thats a different standard (JEDEC CFI ID codes
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> 137b&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe4541
> 8eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636991566928021371&sdata=1i7GEVXSIk2i5%2F%2FXuhXdan1%2BByRt
> yNZrVltINAka1sI%3D&reserved=0)
>
> SPI NOR flashes from Cypress/Spansion reads 0x1 for Manufacturer ID.
> This is based on JEDEC standard
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> 106ab&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe454
> 18eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636991566928021371&sdata=OA1ivCTeCO%2BIQsYFEZc5dMxEcETSIIt9
> yZu%2FU%2FWQBfI%3D&reserved=0
>
> > 3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use
> of this flag here then ?
> >
>
> That was an overlook when those were added to Linux spi-nor code and
> became part of U-Boot during syncing of framework.
>
> I am not saying that adding SPI_NOR_4B_OPCODES is wrong, because
> Spansion flashes does support stateless 4 byte addressing opcodes. But I am
> saying that it should not be needed in the first place (as you can see from
> code snippets). If you ever need to add SPI_NOR_4B_OPCODES to spansion
> flashes (with manf ID 0x1) then I think there is a bug somewhere in the code
> which needs to be debugged and understood.
Hi Vignesh ,
Agreed.
Should I send a negative patch to remove my patch for spansion flash.
Or it will be simply drop from spi-master by Jagan.
Regards
Ashish
>
> Regards
> Vignesh
>
> > [1]:
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> >
> google.com%2Furl%3Fsa%3Dt%26rct%3Dj%26q%3D%26esrc%3Ds%26source%
> 3Dweb%2
> >
> 6cd%3D1%26cad%3Drja%26uact%3D8%26ved%3D2ahUKEwjHpY3U0b7jAhVE
> WX0KHeqSAQ
> >
> kQFjAAegQIBBAB%26url%3Dhttp%253A%252F%252Fwww.cypress.com%252Ffi
> le%252
> >
> F195291%252Fdownload%26usg%3DAOvVaw0zPEXOjP80J8XVrwmB7TBS&
> ;data=02%
> >
> 7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe45418eae2608d70c7486b4
> %7C686
> >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636991566928021371&
> sdata=M
> >
> GPbuuu%2FmSWvgbJqs8H23r6hzjs0nm8kelQBby7%2F1Kk%3D&reserved
> =0
> >
> > Regards
> > Ashish
> >> spi_nor_set_4byte_opcodes(nor, info); #else
> >> /* Configure the BAR - discover bank cmds and read current bank */
> >> nor->addr_width = 3;
> >> ret = read_bar(nor, info);
> >> if (ret < 0)
> >> return ret;
> >> #endif
> >>
> >>
> >> So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is
> >> Spansion flash
> >> spi_nor_set_4byte_opcodes() is always called irrespective of
> >> SPI_NOR_4B_OPCODES
> >>
> >> Also from spi_nor_init():,
> >>
> >>
> >> if (nor->addr_width == 4 &&
> >> (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> >> !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
> >> /*
> >> * If the RESET# pin isn't hooked up properly, or the system
> >> * otherwise doesn't perform a reset command in the boot
> >> * sequence, it's impossible to 100% protect against unexpected
> >> * reboots (e.g., crashes). Warn the user (or hopefully, system
> >> * designer) that this is bad.
> >> */
> >> if (nor->flags & SNOR_F_BROKEN_RESET)
> >> printf("enabling reset hack; may not recover
> >> from unexpected reboots\n");
> >> set_4byte(nor, nor->info, 1);
> >> }
> >>
> >> So set_4byte() is not called for Spansion flashes. So I don't see
> >> need for this patch.
> >>
> >>
> >>>> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are
> >>>> using 4-byte addressing mode with this flash http://patc
> >>>>
> >>
> hwork.ozlabs.org%2Fpatch%2F1108287%2F&data=02%7C01%7Cashish.k
> >> umar
> >>>>
> >>
> %40nxp.com%7C035eff480dc046fe37c108d70b78630e%7C686ea1d3bc2b4c6f
> >> a92cd
> >>>>
> >>
> 99c5c301635%7C0%7C0%7C636990484001673615&sdata=dsD80nUUtmJ
> >> yOoxGvi
> >>>> iDFaOx1SKPXzI6bzXXpH630i8%3D&reserved=0
> >>>
> >>> Thanks for the information.
> >>>
> >>> Applied to u-boot-spi/master
> >>>
> >>
> >> --
> >> Regards
> >> Vignesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s
2019-07-22 5:41 ` Ashish Kumar
@ 2019-07-22 6:06 ` Jagan Teki
0 siblings, 0 replies; 9+ messages in thread
From: Jagan Teki @ 2019-07-22 6:06 UTC (permalink / raw)
To: u-boot
On Mon, Jul 22, 2019 at 11:11 AM Ashish Kumar <ashish.kumar@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Vignesh Raghavendra <vigneshr@ti.com>
> > Sent: Friday, July 19, 2019 11:41 PM
> > To: Ashish Kumar <ashish.kumar@nxp.com>; Jagan Teki
> > <jagan@amarulasolutions.com>; hs at denx.de
> > Cc: U-Boot-Denx <u-boot@lists.denx.de>
> > Subject: Re: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> > SPANSION s25fl512s
> >
> > Caution: EXT Email
> >
> > >>>>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies
> > >>>>>> in operating volatge so s25fs512s shares same command set as
> > >> mentioned
> > >>>>>> below:
> > >>>>>> – Serial Command subset and footprint compatible with S25FL-A,
> > >>>>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command
> > >>>>>> subset and footprint compatible with S25FL-P, and S25FL-S SPI
> > >>>>>> families
> > >>>>>>
> > >>>>>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> > >>>>>> ---
> > >>>>>> v3:
> > >>>>>> 1. Add version info, rebase to top.
> > >>>>>> 2. Re-word commit message.
> > >>>>>> v2:
> > >>>>>> 1. Adding more description in commit msg.
> > >>>>>> 2. consolidating "" and "" in single patch.
> > >>>>>>
> > >>>>>> drivers/mtd/spi/spi-nor-ids.c | 2 +-
> > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>
> > >>>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
> > >>>>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> > >>>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
> > >>>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
> > >>>>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> > >>>>>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> > >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > >>>>>> { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
> > >>>>>> USE_CLSR)
> > >> },
> > >>>>>> { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> > >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > >>>>>> - { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> > >>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> > >>>>>> + { INFO6("s25fl512s", 0x010220, 0x4d0081, 256 * 1024,
> > >>>>>> + 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> > >>>>>> + SPI_NOR_4B_OPCODES) },
> > >>>>>
> > >>>>> I didn't find any diff b/w this with respect to v1 patch, seems
> > >>>>> like Vignesh commented some issue? any update on that.
> > >>>>
> > >>>> Hi Jagan, Vignesh,
> > >>>>
> > >>>> I had updated commit message.
> > >>>>
> > >>>> Wrt Vignesh's comment:
> > >>>> To me it seems not valid. This Flash can be used in extended 3-byte
> > >> addressing mode as well as 4-byte addressing mode.
> > >>>> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode
> > >> with CONFIG_SPI_FLASH_BAR being __not__ set.
> > >>>> By adding SPI_NOR_4B_OPCODES code flow will via
> > >> spi_nor_set_4byte_opcodes() in place of set_4byte().
> > >>>>
> > >>
> > >> This does not seem right. Here is the code snippet from spi-nor-
> > >> core.c::spi_nor_scan():
> > >>
> > >> #ifndef CONFIG_SPI_FLASH_BAR
> > >> /* enable 4-byte addressing if the device exceeds 16MiB */
> > >> nor->addr_width = 4;
> > >> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> > >> info->flags & SPI_NOR_4B_OPCODES)
> > > Hi Vignesh,
> > >
> > > 1. It seems old, but at least one of the flash(spansion) on my NXP/Freescale
> > board reported manufacturing ID as 02h in place of 01h as result I had to
> > add this flags, I will try to trace that particular board. Reading the flow again
> > it seems this patch is not required due to presence of || operator as long
> > manufacturer ID is reported correctly.
> > > if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> > > info->flags & SPI_NOR_4B_OPCODES)
> >
> > So its just a quirky flash with different manf ID? If manuf ID reads different
> > then how does it match s25fl512s entry in the spi-nor-ids?
> >
> > > 2. The very first link on google [1] leads to an APP note from spansion
> > suggesting manufacturing id as 02h, is there a possibility of such flashes in
> > circulation, or am I referring something incorrect ? I will double check this
> > with spansion FAE.
> > > Snippet from APP note named: "AN98488 - Quick Guide to Common Flash
> > Interface"
> > >>>>>>
> > > The Cypress Manufacturer ID for flash devices is 0002h (historically
> > > the AMD ID is 0002h and the Fujitsu ID is 0004h; the former Spansion ID is
> > 0002h)."
> > > <<<<<
> >
> > Cypress seems to use different ID for CFI NOR flash (on Parallel NOR interface
> > or HyperBus interface). Thats a different standard (JEDEC CFI ID codes
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> > edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> > 137b&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe4541
> > 8eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C636991566928021371&sdata=1i7GEVXSIk2i5%2F%2FXuhXdan1%2BByRt
> > yNZrVltINAka1sI%3D&reserved=0)
> >
> > SPI NOR flashes from Cypress/Spansion reads 0x1 for Manufacturer ID.
> > This is based on JEDEC standard
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> > edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> > 106ab&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe454
> > 18eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> > 7C636991566928021371&sdata=OA1ivCTeCO%2BIQsYFEZc5dMxEcETSIIt9
> > yZu%2FU%2FWQBfI%3D&reserved=0
> >
> > > 3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use
> > of this flag here then ?
> > >
> >
> > That was an overlook when those were added to Linux spi-nor code and
> > became part of U-Boot during syncing of framework.
> >
> > I am not saying that adding SPI_NOR_4B_OPCODES is wrong, because
> > Spansion flashes does support stateless 4 byte addressing opcodes. But I am
> > saying that it should not be needed in the first place (as you can see from
> > code snippets). If you ever need to add SPI_NOR_4B_OPCODES to spansion
> > flashes (with manf ID 0x1) then I think there is a bug somewhere in the code
> > which needs to be debugged and understood.
> Hi Vignesh ,
>
> Agreed.
> Should I send a negative patch to remove my patch for spansion flash.
> Or it will be simply drop from spi-master by Jagan.
Will drop anyway.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-22 6:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 6:16 [U-Boot] [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s Ashish Kumar
2019-07-18 10:28 ` Jagan Teki
2019-07-18 10:45 ` [U-Boot] [EXT] " Ashish Kumar
2019-07-18 10:59 ` Jagan Teki
2019-07-18 12:07 ` Vignesh Raghavendra
2019-07-18 14:27 ` Ashish Kumar
2019-07-19 18:11 ` Vignesh Raghavendra
2019-07-22 5:41 ` Ashish Kumar
2019-07-22 6:06 ` Jagan Teki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.