| Message ID | 20220304185137.3376011-1-michael@walle.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v1] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it | expand |
On 3/4/22 20:51, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > While the first version of JESD216 specify the opcode for 4 bit I/O > accesses, it lacks information on how to actually enable this mode. > > For now, the one set in spi_nor_init_default_params() will be used. > But this one is likely wrong for some flashes, in particular the > Macronix MX25L12835F. Thus we need to clear the enable method when > parsing the SFDP. Flashes with such an SFDP revision will have to use a > flash (and SFDP revision) specific fixup. > > This might break quad I/O for some flashes which relied on the > spi_nor_sr2_bit1_quad_enable() that was formerly set. If your bisect > turns up this commit, you'll probably have to set the proper > quad_enable method in a post_bfpt() fixup for your flash. > Right, I meant adding a paragraph such as the one from above. > Signed-off-by: Michael Walle <michael@walle.cc> > Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > changes since RFC: > - reworded commit message > - added comment about post_bfpt hook > > Tudor, I'm not sure what you meant with > Maybe you can update the commit message and explain why would some > flashes fail to enable quad mode, similar to what I did. > > It doesn't work because the wrong method is chosen? ;) > > drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index a5211543d30d..6bba9b601846 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > map->uniform_erase_type = map->uniform_region.offset & > SNOR_ERASE_TYPE_MASK; > > + /* > + * The first JESD216 revision doesn't specify a method to enable > + * quad mode. spi_nor_init_default_params() will set a legacy > + * default method to enable quad mode. We have to disable it > + * again. > + * Flashes with this JESD216 revision need to set the quad_enable > + * method in their post_bfpt() fixup if they want to use quad I/O. > + */ Great. Looks good to me. I'll change the subject to "mtd: spi-nor: sfdp:" when applying. Cheers, ta > + params->quad_enable = NULL; > + > /* Stop here if not JESD216 rev A or later. */ > if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) > return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); > @@ -562,7 +572,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > /* Quad Enable Requirements. */ > switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { > case BFPT_DWORD15_QER_NONE: > - params->quad_enable = NULL; > break; > > case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: > -- > 2.30.2 >
On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 3/4/22 20:51, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> While the first version of JESD216 specify the opcode for 4 bit I/O >> accesses, it lacks information on how to actually enable this mode. >> >> For now, the one set in spi_nor_init_default_params() will be used. >> But this one is likely wrong for some flashes, in particular the >> Macronix MX25L12835F. Thus we need to clear the enable method when >> parsing the SFDP. Flashes with such an SFDP revision will have to use a >> flash (and SFDP revision) specific fixup. >> >> This might break quad I/O for some flashes which relied on the >> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your bisect >> turns up this commit, you'll probably have to set the proper >> quad_enable method in a post_bfpt() fixup for your flash. >> > > Right, I meant adding a paragraph such as the one from above. > >> Signed-off-by: Michael Walle <michael@walle.cc> >> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >> --- >> changes since RFC: >> - reworded commit message >> - added comment about post_bfpt hook >> >> Tudor, I'm not sure what you meant with >> Maybe you can update the commit message and explain why would some >> flashes fail to enable quad mode, similar to what I did. >> >> It doesn't work because the wrong method is chosen? ;) >> >> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c >> index a5211543d30d..6bba9b601846 100644 >> --- a/drivers/mtd/spi-nor/sfdp.c >> +++ b/drivers/mtd/spi-nor/sfdp.c >> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> map->uniform_erase_type = map->uniform_region.offset & >> SNOR_ERASE_TYPE_MASK; >> >> + /* >> + * The first JESD216 revision doesn't specify a method to enable >> + * quad mode. spi_nor_init_default_params() will set a legacy >> + * default method to enable quad mode. We have to disable it >> + * again. >> + * Flashes with this JESD216 revision need to set the quad_enable >> + * method in their post_bfpt() fixup if they want to use quad I/O. >> + */ > > Great. Looks good to me. I'll change the subject to "mtd: spi-nor: sfdp:" > when applying. As we talked on the meeting, we can instead move the default quad mode init to the deprecated way of initializing the params, or/and to where SKIP_SFDP is used. This way you'll no longer need to clear it here. > > Cheers, > ta >> + params->quad_enable = NULL; >> + >> /* Stop here if not JESD216 rev A or later. */ >> if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) >> return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); >> @@ -562,7 +572,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> /* Quad Enable Requirements. */ >> switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { >> case BFPT_DWORD15_QER_NONE: >> - params->quad_enable = NULL; >> break; >> >> case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: >> -- >> 2.30.2 >> > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: > On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> On 3/4/22 20:51, Michael Walle wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>> know the content is safe >>> >>> While the first version of JESD216 specify the opcode for 4 bit I/O >>> accesses, it lacks information on how to actually enable this mode. >>> >>> For now, the one set in spi_nor_init_default_params() will be used. >>> But this one is likely wrong for some flashes, in particular the >>> Macronix MX25L12835F. Thus we need to clear the enable method when >>> parsing the SFDP. Flashes with such an SFDP revision will have to use >>> a >>> flash (and SFDP revision) specific fixup. >>> >>> This might break quad I/O for some flashes which relied on the >>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your bisect >>> turns up this commit, you'll probably have to set the proper >>> quad_enable method in a post_bfpt() fixup for your flash. >>> >> >> Right, I meant adding a paragraph such as the one from above. >> >>> Signed-off-by: Michael Walle <michael@walle.cc> >>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>> --- >>> changes since RFC: >>> - reworded commit message >>> - added comment about post_bfpt hook >>> >>> Tudor, I'm not sure what you meant with >>> Maybe you can update the commit message and explain why would some >>> flashes fail to enable quad mode, similar to what I did. >>> >>> It doesn't work because the wrong method is chosen? ;) >>> >>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c >>> index a5211543d30d..6bba9b601846 100644 >>> --- a/drivers/mtd/spi-nor/sfdp.c >>> +++ b/drivers/mtd/spi-nor/sfdp.c >>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>> *nor, >>> map->uniform_erase_type = map->uniform_region.offset & >>> SNOR_ERASE_TYPE_MASK; >>> >>> + /* >>> + * The first JESD216 revision doesn't specify a method to >>> enable >>> + * quad mode. spi_nor_init_default_params() will set a legacy >>> + * default method to enable quad mode. We have to disable it >>> + * again. >>> + * Flashes with this JESD216 revision need to set the >>> quad_enable >>> + * method in their post_bfpt() fixup if they want to use quad >>> I/O. >>> + */ >> >> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >> sfdp:" >> when applying. > > As we talked on the meeting, we can instead move the default quad mode > init > to the deprecated way of initializing the params, or/and to where > SKIP_SFDP > is used. This way you'll no longer need to clear it here. Mh, I just had a look and I'm not sure it will work there, because in the deprecated way, the SFDP is still parsed and thus we might still have the wrong enable method for flashes which don't have PARSE_SFDP set. -michael
On 3/7/22 20:56, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: >> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know >>> the content is safe >>> >>> On 3/4/22 20:51, Michael Walle wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know the content is safe >>>> >>>> While the first version of JESD216 specify the opcode for 4 bit I/O >>>> accesses, it lacks information on how to actually enable this mode. >>>> >>>> For now, the one set in spi_nor_init_default_params() will be used. >>>> But this one is likely wrong for some flashes, in particular the >>>> Macronix MX25L12835F. Thus we need to clear the enable method when >>>> parsing the SFDP. Flashes with such an SFDP revision will have to use >>>> a >>>> flash (and SFDP revision) specific fixup. >>>> >>>> This might break quad I/O for some flashes which relied on the >>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your bisect >>>> turns up this commit, you'll probably have to set the proper >>>> quad_enable method in a post_bfpt() fixup for your flash. >>>> >>> >>> Right, I meant adding a paragraph such as the one from above. >>> >>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>>> --- >>>> changes since RFC: >>>> - reworded commit message >>>> - added comment about post_bfpt hook >>>> >>>> Tudor, I'm not sure what you meant with >>>> Maybe you can update the commit message and explain why would some >>>> flashes fail to enable quad mode, similar to what I did. >>>> >>>> It doesn't work because the wrong method is chosen? ;) >>>> >>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c >>>> index a5211543d30d..6bba9b601846 100644 >>>> --- a/drivers/mtd/spi-nor/sfdp.c >>>> +++ b/drivers/mtd/spi-nor/sfdp.c >>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>>> *nor, >>>> map->uniform_erase_type = map->uniform_region.offset & >>>> SNOR_ERASE_TYPE_MASK; >>>> >>>> + /* >>>> + * The first JESD216 revision doesn't specify a method to >>>> enable >>>> + * quad mode. spi_nor_init_default_params() will set a legacy >>>> + * default method to enable quad mode. We have to disable it >>>> + * again. >>>> + * Flashes with this JESD216 revision need to set the >>>> quad_enable >>>> + * method in their post_bfpt() fixup if they want to use quad >>>> I/O. >>>> + */ >>> >>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >>> sfdp:" >>> when applying. >> >> As we talked on the meeting, we can instead move the default quad mode >> init >> to the deprecated way of initializing the params, or/and to where >> SKIP_SFDP >> is used. This way you'll no longer need to clear it here. > > Mh, I just had a look and I'm not sure it will work there, > because in the deprecated way, the SFDP is still parsed and > thus we might still have the wrong enable method for flashes > which don't have PARSE_SFDP set. Moving the default quad_enable method to spi_nor_no_sfdp_init_params(), thus also for spi_nor_init_params_deprecated() because it calls spi_nor_no_sfdp_init_params(), will not change the behavior for the deprecated way of initializing the params, isn't it? A more reason to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params init at some point. No new fixes for spi_nor_init_params_deprecated().
Am 2022-03-09 05:49, schrieb Tudor.Ambarus@microchip.com: > On 3/7/22 20:56, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: >>> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know >>>> the content is safe >>>> >>>> On 3/4/22 20:51, Michael Walle wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>> know the content is safe >>>>> >>>>> While the first version of JESD216 specify the opcode for 4 bit I/O >>>>> accesses, it lacks information on how to actually enable this mode. >>>>> >>>>> For now, the one set in spi_nor_init_default_params() will be used. >>>>> But this one is likely wrong for some flashes, in particular the >>>>> Macronix MX25L12835F. Thus we need to clear the enable method when >>>>> parsing the SFDP. Flashes with such an SFDP revision will have to >>>>> use >>>>> a >>>>> flash (and SFDP revision) specific fixup. >>>>> >>>>> This might break quad I/O for some flashes which relied on the >>>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your >>>>> bisect >>>>> turns up this commit, you'll probably have to set the proper >>>>> quad_enable method in a post_bfpt() fixup for your flash. >>>>> >>>> >>>> Right, I meant adding a paragraph such as the one from above. >>>> >>>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>>>> --- >>>>> changes since RFC: >>>>> - reworded commit message >>>>> - added comment about post_bfpt hook >>>>> >>>>> Tudor, I'm not sure what you meant with >>>>> Maybe you can update the commit message and explain why would >>>>> some >>>>> flashes fail to enable quad mode, similar to what I did. >>>>> >>>>> It doesn't work because the wrong method is chosen? ;) >>>>> >>>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c >>>>> b/drivers/mtd/spi-nor/sfdp.c >>>>> index a5211543d30d..6bba9b601846 100644 >>>>> --- a/drivers/mtd/spi-nor/sfdp.c >>>>> +++ b/drivers/mtd/spi-nor/sfdp.c >>>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>>>> *nor, >>>>> map->uniform_erase_type = map->uniform_region.offset & >>>>> SNOR_ERASE_TYPE_MASK; >>>>> >>>>> + /* >>>>> + * The first JESD216 revision doesn't specify a method to >>>>> enable >>>>> + * quad mode. spi_nor_init_default_params() will set a >>>>> legacy >>>>> + * default method to enable quad mode. We have to disable >>>>> it >>>>> + * again. >>>>> + * Flashes with this JESD216 revision need to set the >>>>> quad_enable >>>>> + * method in their post_bfpt() fixup if they want to use >>>>> quad >>>>> I/O. >>>>> + */ >>>> >>>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >>>> sfdp:" >>>> when applying. >>> >>> As we talked on the meeting, we can instead move the default quad >>> mode >>> init >>> to the deprecated way of initializing the params, or/and to where >>> SKIP_SFDP >>> is used. This way you'll no longer need to clear it here. >> >> Mh, I just had a look and I'm not sure it will work there, >> because in the deprecated way, the SFDP is still parsed and >> thus we might still have the wrong enable method for flashes >> which don't have PARSE_SFDP set. > > Moving the default quad_enable method to spi_nor_no_sfdp_init_params(), > thus also for spi_nor_init_params_deprecated() because it calls > spi_nor_no_sfdp_init_params(), will not change the behavior for the > deprecated way of initializing the params, isn't it? What do you mean? The behavior is not changed and the bug is not fixed for the flashes which use the deprecated way. It will get overwritten by the spi_nor_parse_sfdp call in spi_nor_sfdp_init_params_deprecated(). > A more reason > to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params > init at some point. > > No new fixes for spi_nor_init_params_deprecated(). Hm, so we deliberately won't fix known bugs there? I'm not sure I'd agree here. Esp. because it is hard to debug and might even depend on non-volatile state of the flash. -michael
On 3/14/22 22:42, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-09 05:49, schrieb Tudor.Ambarus@microchip.com: >> On 3/7/22 20:56, Michael Walle wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know >>> the content is safe >>> >>> Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: >>>> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>> know >>>>> the content is safe >>>>> >>>>> On 3/4/22 20:51, Michael Walle wrote: >>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>>> know the content is safe >>>>>> >>>>>> While the first version of JESD216 specify the opcode for 4 bit I/O >>>>>> accesses, it lacks information on how to actually enable this mode. >>>>>> >>>>>> For now, the one set in spi_nor_init_default_params() will be used. >>>>>> But this one is likely wrong for some flashes, in particular the >>>>>> Macronix MX25L12835F. Thus we need to clear the enable method when >>>>>> parsing the SFDP. Flashes with such an SFDP revision will have to >>>>>> use >>>>>> a >>>>>> flash (and SFDP revision) specific fixup. >>>>>> >>>>>> This might break quad I/O for some flashes which relied on the >>>>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your >>>>>> bisect >>>>>> turns up this commit, you'll probably have to set the proper >>>>>> quad_enable method in a post_bfpt() fixup for your flash. >>>>>> >>>>> >>>>> Right, I meant adding a paragraph such as the one from above. >>>>> >>>>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>>>>> --- >>>>>> changes since RFC: >>>>>> - reworded commit message >>>>>> - added comment about post_bfpt hook >>>>>> >>>>>> Tudor, I'm not sure what you meant with >>>>>> Maybe you can update the commit message and explain why would >>>>>> some >>>>>> flashes fail to enable quad mode, similar to what I did. >>>>>> >>>>>> It doesn't work because the wrong method is chosen? ;) >>>>>> >>>>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c >>>>>> b/drivers/mtd/spi-nor/sfdp.c >>>>>> index a5211543d30d..6bba9b601846 100644 >>>>>> --- a/drivers/mtd/spi-nor/sfdp.c >>>>>> +++ b/drivers/mtd/spi-nor/sfdp.c >>>>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>>>>> *nor, >>>>>> map->uniform_erase_type = map->uniform_region.offset & >>>>>> SNOR_ERASE_TYPE_MASK; >>>>>> >>>>>> + /* >>>>>> + * The first JESD216 revision doesn't specify a method to >>>>>> enable >>>>>> + * quad mode. spi_nor_init_default_params() will set a >>>>>> legacy >>>>>> + * default method to enable quad mode. We have to disable >>>>>> it >>>>>> + * again. >>>>>> + * Flashes with this JESD216 revision need to set the >>>>>> quad_enable >>>>>> + * method in their post_bfpt() fixup if they want to use >>>>>> quad >>>>>> I/O. >>>>>> + */ >>>>> >>>>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >>>>> sfdp:" >>>>> when applying. >>>> >>>> As we talked on the meeting, we can instead move the default quad >>>> mode >>>> init >>>> to the deprecated way of initializing the params, or/and to where >>>> SKIP_SFDP >>>> is used. This way you'll no longer need to clear it here. >>> >>> Mh, I just had a look and I'm not sure it will work there, >>> because in the deprecated way, the SFDP is still parsed and >>> thus we might still have the wrong enable method for flashes >>> which don't have PARSE_SFDP set. >> >> Moving the default quad_enable method to spi_nor_no_sfdp_init_params(), >> thus also for spi_nor_init_params_deprecated() because it calls >> spi_nor_no_sfdp_init_params(), will not change the behavior for the >> deprecated way of initializing the params, isn't it? > > What do you mean? The behavior is not changed and the bug is not > fixed for the flashes which use the deprecated way. It will get > overwritten by the spi_nor_parse_sfdp call in > spi_nor_sfdp_init_params_deprecated(). right, it will not change the logic for the deprecated way of initializing the params. > >> A more reason >> to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params >> init at some point. >> >> No new fixes for spi_nor_init_params_deprecated(). > > Hm, so we deliberately won't fix known bugs there? I'm not sure > I'd agree here. Esp. because it is hard to debug and might even > depend on non-volatile state of the flash. > even more a reason to switch to the recommended way of initializing the flash. We'll get rid of the deprecated code anyway, no?
Am 2022-03-15 06:55, schrieb Tudor.Ambarus@microchip.com: > On 3/14/22 22:42, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2022-03-09 05:49, schrieb Tudor.Ambarus@microchip.com: >>> On 3/7/22 20:56, Michael Walle wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know >>>> the content is safe >>>> >>>> Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: >>>>> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>>> know >>>>>> the content is safe >>>>>> >>>>>> On 3/4/22 20:51, Michael Walle wrote: >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>>>> know the content is safe >>>>>>> >>>>>>> While the first version of JESD216 specify the opcode for 4 bit >>>>>>> I/O >>>>>>> accesses, it lacks information on how to actually enable this >>>>>>> mode. >>>>>>> >>>>>>> For now, the one set in spi_nor_init_default_params() will be >>>>>>> used. >>>>>>> But this one is likely wrong for some flashes, in particular the >>>>>>> Macronix MX25L12835F. Thus we need to clear the enable method >>>>>>> when >>>>>>> parsing the SFDP. Flashes with such an SFDP revision will have to >>>>>>> use >>>>>>> a >>>>>>> flash (and SFDP revision) specific fixup. >>>>>>> >>>>>>> This might break quad I/O for some flashes which relied on the >>>>>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your >>>>>>> bisect >>>>>>> turns up this commit, you'll probably have to set the proper >>>>>>> quad_enable method in a post_bfpt() fixup for your flash. >>>>>>> >>>>>> >>>>>> Right, I meant adding a paragraph such as the one from above. >>>>>> >>>>>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>>>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>>>>>> --- >>>>>>> changes since RFC: >>>>>>> - reworded commit message >>>>>>> - added comment about post_bfpt hook >>>>>>> >>>>>>> Tudor, I'm not sure what you meant with >>>>>>> Maybe you can update the commit message and explain why would >>>>>>> some >>>>>>> flashes fail to enable quad mode, similar to what I did. >>>>>>> >>>>>>> It doesn't work because the wrong method is chosen? ;) >>>>>>> >>>>>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c >>>>>>> b/drivers/mtd/spi-nor/sfdp.c >>>>>>> index a5211543d30d..6bba9b601846 100644 >>>>>>> --- a/drivers/mtd/spi-nor/sfdp.c >>>>>>> +++ b/drivers/mtd/spi-nor/sfdp.c >>>>>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>>>>>> *nor, >>>>>>> map->uniform_erase_type = map->uniform_region.offset & >>>>>>> SNOR_ERASE_TYPE_MASK; >>>>>>> >>>>>>> + /* >>>>>>> + * The first JESD216 revision doesn't specify a method to >>>>>>> enable >>>>>>> + * quad mode. spi_nor_init_default_params() will set a >>>>>>> legacy >>>>>>> + * default method to enable quad mode. We have to disable >>>>>>> it >>>>>>> + * again. >>>>>>> + * Flashes with this JESD216 revision need to set the >>>>>>> quad_enable >>>>>>> + * method in their post_bfpt() fixup if they want to use >>>>>>> quad >>>>>>> I/O. >>>>>>> + */ >>>>>> >>>>>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >>>>>> sfdp:" >>>>>> when applying. >>>>> >>>>> As we talked on the meeting, we can instead move the default quad >>>>> mode >>>>> init >>>>> to the deprecated way of initializing the params, or/and to where >>>>> SKIP_SFDP >>>>> is used. This way you'll no longer need to clear it here. >>>> >>>> Mh, I just had a look and I'm not sure it will work there, >>>> because in the deprecated way, the SFDP is still parsed and >>>> thus we might still have the wrong enable method for flashes >>>> which don't have PARSE_SFDP set. >>> >>> Moving the default quad_enable method to >>> spi_nor_no_sfdp_init_params(), >>> thus also for spi_nor_init_params_deprecated() because it calls >>> spi_nor_no_sfdp_init_params(), will not change the behavior for the >>> deprecated way of initializing the params, isn't it? >> >> What do you mean? The behavior is not changed and the bug is not >> fixed for the flashes which use the deprecated way. It will get >> overwritten by the spi_nor_parse_sfdp call in >> spi_nor_sfdp_init_params_deprecated(). > > right, it will not change the logic for the deprecated way of > initializing > the params. > >> >>> A more reason >>> to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params >>> init at some point. >>> >>> No new fixes for spi_nor_init_params_deprecated(). >> >> Hm, so we deliberately won't fix known bugs there? I'm not sure >> I'd agree here. Esp. because it is hard to debug and might even >> depend on non-volatile state of the flash. >> > > even more a reason to switch to the recommended way of initializing > the flash. We'll get rid of the deprecated code anyway, no? I get your point. But I disagree with you on that point :) Features? sure we can say this shouldn't go to any deprectated code flow and might poke users to post a patch. But bug fixes? I don't think we should hold these back. Correct me if I'm wrong, but we can get rid of the deprecated way only if all the flashes are converted to PARSE_SFDP or SKIP_SFDP, right? And I don't see this happening anytime soon. -michael
On 3/15/22 09:24, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-15 06:55, schrieb Tudor.Ambarus@microchip.com: >> On 3/14/22 22:42, Michael Walle wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know >>> the content is safe >>> >>> Am 2022-03-09 05:49, schrieb Tudor.Ambarus@microchip.com: >>>> On 3/7/22 20:56, Michael Walle wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>> know >>>>> the content is safe >>>>> >>>>> Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: >>>>>> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>>>> know >>>>>>> the content is safe >>>>>>> >>>>>>> On 3/4/22 20:51, Michael Walle wrote: >>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>>>>>> know the content is safe >>>>>>>> >>>>>>>> While the first version of JESD216 specify the opcode for 4 bit >>>>>>>> I/O >>>>>>>> accesses, it lacks information on how to actually enable this >>>>>>>> mode. >>>>>>>> >>>>>>>> For now, the one set in spi_nor_init_default_params() will be >>>>>>>> used. >>>>>>>> But this one is likely wrong for some flashes, in particular the >>>>>>>> Macronix MX25L12835F. Thus we need to clear the enable method >>>>>>>> when >>>>>>>> parsing the SFDP. Flashes with such an SFDP revision will have to >>>>>>>> use >>>>>>>> a >>>>>>>> flash (and SFDP revision) specific fixup. >>>>>>>> >>>>>>>> This might break quad I/O for some flashes which relied on the >>>>>>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your >>>>>>>> bisect >>>>>>>> turns up this commit, you'll probably have to set the proper >>>>>>>> quad_enable method in a post_bfpt() fixup for your flash. >>>>>>>> >>>>>>> >>>>>>> Right, I meant adding a paragraph such as the one from above. >>>>>>> >>>>>>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>>>>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> >>>>>>>> --- >>>>>>>> changes since RFC: >>>>>>>> - reworded commit message >>>>>>>> - added comment about post_bfpt hook >>>>>>>> >>>>>>>> Tudor, I'm not sure what you meant with >>>>>>>> Maybe you can update the commit message and explain why would >>>>>>>> some >>>>>>>> flashes fail to enable quad mode, similar to what I did. >>>>>>>> >>>>>>>> It doesn't work because the wrong method is chosen? ;) >>>>>>>> >>>>>>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- >>>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c >>>>>>>> b/drivers/mtd/spi-nor/sfdp.c >>>>>>>> index a5211543d30d..6bba9b601846 100644 >>>>>>>> --- a/drivers/mtd/spi-nor/sfdp.c >>>>>>>> +++ b/drivers/mtd/spi-nor/sfdp.c >>>>>>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor >>>>>>>> *nor, >>>>>>>> map->uniform_erase_type = map->uniform_region.offset & >>>>>>>> SNOR_ERASE_TYPE_MASK; >>>>>>>> >>>>>>>> + /* >>>>>>>> + * The first JESD216 revision doesn't specify a method to >>>>>>>> enable >>>>>>>> + * quad mode. spi_nor_init_default_params() will set a >>>>>>>> legacy >>>>>>>> + * default method to enable quad mode. We have to disable >>>>>>>> it >>>>>>>> + * again. >>>>>>>> + * Flashes with this JESD216 revision need to set the >>>>>>>> quad_enable >>>>>>>> + * method in their post_bfpt() fixup if they want to use >>>>>>>> quad >>>>>>>> I/O. >>>>>>>> + */ >>>>>>> >>>>>>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: >>>>>>> sfdp:" >>>>>>> when applying. >>>>>> >>>>>> As we talked on the meeting, we can instead move the default quad >>>>>> mode >>>>>> init >>>>>> to the deprecated way of initializing the params, or/and to where >>>>>> SKIP_SFDP >>>>>> is used. This way you'll no longer need to clear it here. >>>>> >>>>> Mh, I just had a look and I'm not sure it will work there, >>>>> because in the deprecated way, the SFDP is still parsed and >>>>> thus we might still have the wrong enable method for flashes >>>>> which don't have PARSE_SFDP set. >>>> >>>> Moving the default quad_enable method to >>>> spi_nor_no_sfdp_init_params(), >>>> thus also for spi_nor_init_params_deprecated() because it calls >>>> spi_nor_no_sfdp_init_params(), will not change the behavior for the >>>> deprecated way of initializing the params, isn't it? >>> >>> What do you mean? The behavior is not changed and the bug is not >>> fixed for the flashes which use the deprecated way. It will get >>> overwritten by the spi_nor_parse_sfdp call in >>> spi_nor_sfdp_init_params_deprecated(). >> >> right, it will not change the logic for the deprecated way of >> initializing >> the params. >> >>> >>>> A more reason >>>> to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params >>>> init at some point. >>>> >>>> No new fixes for spi_nor_init_params_deprecated(). >>> >>> Hm, so we deliberately won't fix known bugs there? I'm not sure >>> I'd agree here. Esp. because it is hard to debug and might even >>> depend on non-volatile state of the flash. >>> >> >> even more a reason to switch to the recommended way of initializing >> the flash. We'll get rid of the deprecated code anyway, no? > > I get your point. But I disagree with you on that point :) Features? > sure we can say this shouldn't go to any deprectated code flow and > might poke users to post a patch. But bug fixes? I don't think > we should hold these back. Why to fix something that never worked in a deprecated code path? It's equivalent to adding new support, no? > Correct me if I'm wrong, but we can get rid of the deprecated way > only if all the flashes are converted to PARSE_SFDP or SKIP_SFDP, > right? And I don't see this happening anytime soon. Right. I vote to don't queue any new patches for deprecated code paths, new support or fixes. But I'm not completely against it, I don't see the point, that's all. Let's sync with Pratyush and Vignesh too. Cheers, ta
On 15/03/22 07:47AM, Tudor.Ambarus@microchip.com wrote: > On 3/15/22 09:24, Michael Walle wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Am 2022-03-15 06:55, schrieb Tudor.Ambarus@microchip.com: > >> On 3/14/22 22:42, Michael Walle wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know > >>> the content is safe > >>> > >>> Am 2022-03-09 05:49, schrieb Tudor.Ambarus@microchip.com: > >>>> On 3/7/22 20:56, Michael Walle wrote: > >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you > >>>>> know > >>>>> the content is safe > >>>>> > >>>>> Am 2022-03-07 10:23, schrieb Tudor.Ambarus@microchip.com: > >>>>>> On 3/7/22 09:12, Tudor.Ambarus@microchip.com wrote: > >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you > >>>>>>> know > >>>>>>> the content is safe > >>>>>>> > >>>>>>> On 3/4/22 20:51, Michael Walle wrote: > >>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you > >>>>>>>> know the content is safe > >>>>>>>> > >>>>>>>> While the first version of JESD216 specify the opcode for 4 bit > >>>>>>>> I/O > >>>>>>>> accesses, it lacks information on how to actually enable this > >>>>>>>> mode. > >>>>>>>> > >>>>>>>> For now, the one set in spi_nor_init_default_params() will be > >>>>>>>> used. > >>>>>>>> But this one is likely wrong for some flashes, in particular the > >>>>>>>> Macronix MX25L12835F. Thus we need to clear the enable method > >>>>>>>> when > >>>>>>>> parsing the SFDP. Flashes with such an SFDP revision will have to > >>>>>>>> use > >>>>>>>> a > >>>>>>>> flash (and SFDP revision) specific fixup. > >>>>>>>> > >>>>>>>> This might break quad I/O for some flashes which relied on the > >>>>>>>> spi_nor_sr2_bit1_quad_enable() that was formerly set. If your > >>>>>>>> bisect > >>>>>>>> turns up this commit, you'll probably have to set the proper > >>>>>>>> quad_enable method in a post_bfpt() fixup for your flash. > >>>>>>>> > >>>>>>> > >>>>>>> Right, I meant adding a paragraph such as the one from above. > >>>>>>> > >>>>>>>> Signed-off-by: Michael Walle <michael@walle.cc> > >>>>>>>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > >>>>>>>> --- > >>>>>>>> changes since RFC: > >>>>>>>> - reworded commit message > >>>>>>>> - added comment about post_bfpt hook > >>>>>>>> > >>>>>>>> Tudor, I'm not sure what you meant with > >>>>>>>> Maybe you can update the commit message and explain why would > >>>>>>>> some > >>>>>>>> flashes fail to enable quad mode, similar to what I did. > >>>>>>>> > >>>>>>>> It doesn't work because the wrong method is chosen? ;) > >>>>>>>> > >>>>>>>> drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- > >>>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) > >>>>>>>> > >>>>>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c > >>>>>>>> b/drivers/mtd/spi-nor/sfdp.c > >>>>>>>> index a5211543d30d..6bba9b601846 100644 > >>>>>>>> --- a/drivers/mtd/spi-nor/sfdp.c > >>>>>>>> +++ b/drivers/mtd/spi-nor/sfdp.c > >>>>>>>> @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor > >>>>>>>> *nor, > >>>>>>>> map->uniform_erase_type = map->uniform_region.offset & > >>>>>>>> SNOR_ERASE_TYPE_MASK; > >>>>>>>> > >>>>>>>> + /* > >>>>>>>> + * The first JESD216 revision doesn't specify a method to > >>>>>>>> enable > >>>>>>>> + * quad mode. spi_nor_init_default_params() will set a > >>>>>>>> legacy > >>>>>>>> + * default method to enable quad mode. We have to disable > >>>>>>>> it > >>>>>>>> + * again. > >>>>>>>> + * Flashes with this JESD216 revision need to set the > >>>>>>>> quad_enable > >>>>>>>> + * method in their post_bfpt() fixup if they want to use > >>>>>>>> quad > >>>>>>>> I/O. > >>>>>>>> + */ > >>>>>>> > >>>>>>> Great. Looks good to me. I'll change the subject to "mtd: spi-nor: > >>>>>>> sfdp:" > >>>>>>> when applying. > >>>>>> > >>>>>> As we talked on the meeting, we can instead move the default quad > >>>>>> mode > >>>>>> init > >>>>>> to the deprecated way of initializing the params, or/and to where > >>>>>> SKIP_SFDP > >>>>>> is used. This way you'll no longer need to clear it here. > >>>>> > >>>>> Mh, I just had a look and I'm not sure it will work there, > >>>>> because in the deprecated way, the SFDP is still parsed and > >>>>> thus we might still have the wrong enable method for flashes > >>>>> which don't have PARSE_SFDP set. > >>>> > >>>> Moving the default quad_enable method to > >>>> spi_nor_no_sfdp_init_params(), > >>>> thus also for spi_nor_init_params_deprecated() because it calls > >>>> spi_nor_no_sfdp_init_params(), will not change the behavior for the > >>>> deprecated way of initializing the params, isn't it? > >>> > >>> What do you mean? The behavior is not changed and the bug is not > >>> fixed for the flashes which use the deprecated way. It will get > >>> overwritten by the spi_nor_parse_sfdp call in > >>> spi_nor_sfdp_init_params_deprecated(). > >> > >> right, it will not change the logic for the deprecated way of > >> initializing > >> the params. > >> > >>> > >>>> A more reason > >>>> to use PARSE_SFDP/SKIP_SFDP, we'll get rid of the deprecated params > >>>> init at some point. > >>>> > >>>> No new fixes for spi_nor_init_params_deprecated(). > >>> > >>> Hm, so we deliberately won't fix known bugs there? I'm not sure > >>> I'd agree here. Esp. because it is hard to debug and might even > >>> depend on non-volatile state of the flash. > >>> > >> > >> even more a reason to switch to the recommended way of initializing > >> the flash. We'll get rid of the deprecated code anyway, no? > > > > I get your point. But I disagree with you on that point :) Features? > > sure we can say this shouldn't go to any deprectated code flow and > > might poke users to post a patch. But bug fixes? I don't think > > we should hold these back. > > Why to fix something that never worked in a deprecated code path? It's > equivalent to adding new support, no? I have not followed this discussion very closely but this argument makes sense to me. If something never worked in the deprecated path then we have don't have to fix it. > > > Correct me if I'm wrong, but we can get rid of the deprecated way > > only if all the flashes are converted to PARSE_SFDP or SKIP_SFDP, > > right? And I don't see this happening anytime soon. > > Right. I vote to don't queue any new patches for deprecated code paths, > new support or fixes. But I'm not completely against it, I don't see > the point, that's all. Let's sync with Pratyush and Vignesh too. I agree with no new features for deprecated path. But I think we should still take in bug fixes.
On 3/4/22 20:51, Michael Walle wrote: Hi! > While the first version of JESD216 specify the opcode for 4 bit I/O > accesses, it lacks information on how to actually enable this mode. > > For now, the one set in spi_nor_init_default_params() will be used. > But this one is likely wrong for some flashes, in particular the > Macronix MX25L12835F. Thus we need to clear the enable method when > parsing the SFDP. Flashes with such an SFDP revision will have to use a > flash (and SFDP revision) specific fixup. This is equivalent to clearing the default QE method for all those flashes that support SFDP, with implications for those that support SFDP Rev A. If I continue the logic, I could remove the default QE method from spi_nor_init_default_params(), but I don't think I would like that. You could use a post_bfpt hook without explicitly clearing it here. Would you please explain more why is clearing the default method better than using a wrong default one, and why you chose to do this just for the Rev A SFDP flashes and you didn't include the no-SFDP flashes as well? thanks, ta > > This might break quad I/O for some flashes which relied on the > spi_nor_sr2_bit1_quad_enable() that was formerly set. If your bisect > turns up this commit, you'll probably have to set the proper > quad_enable method in a post_bfpt() fixup for your flash. > > Signed-off-by: Michael Walle <michael@walle.cc> > Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > changes since RFC: > - reworded commit message > - added comment about post_bfpt hook > > Tudor, I'm not sure what you meant with > Maybe you can update the commit message and explain why would some > flashes fail to enable quad mode, similar to what I did. > > It doesn't work because the wrong method is chosen? ;) > > drivers/mtd/spi-nor/sfdp.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index a5211543d30d..6bba9b601846 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > map->uniform_erase_type = map->uniform_region.offset & > SNOR_ERASE_TYPE_MASK; > > + /* > + * The first JESD216 revision doesn't specify a method to enable > + * quad mode. spi_nor_init_default_params() will set a legacy > + * default method to enable quad mode. We have to disable it > + * again. > + * Flashes with this JESD216 revision need to set the quad_enable > + * method in their post_bfpt() fixup if they want to use quad I/O. > + */ > + params->quad_enable = NULL; > + > /* Stop here if not JESD216 rev A or later. */ > if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) > return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); > @@ -562,7 +572,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > /* Quad Enable Requirements. */ > switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { > case BFPT_DWORD15_QER_NONE: > - params->quad_enable = NULL; > break; > > case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
Am 2022-07-19 06:57, schrieb Tudor.Ambarus@microchip.com: > On 3/4/22 20:51, Michael Walle wrote: > > Hi! > >> While the first version of JESD216 specify the opcode for 4 bit I/O >> accesses, it lacks information on how to actually enable this mode. >> >> For now, the one set in spi_nor_init_default_params() will be used. >> But this one is likely wrong for some flashes, in particular the >> Macronix MX25L12835F. Thus we need to clear the enable method when >> parsing the SFDP. Flashes with such an SFDP revision will have to use >> a >> flash (and SFDP revision) specific fixup. > > This is equivalent to clearing the default QE method for all those > flashes > that support SFDP, with implications for those that support SFDP Rev A. > If I continue the logic, I could remove the default QE method from > spi_nor_init_default_params(), but I don't think I would like that. > You could use a post_bfpt hook without explicitly clearing it here. > > Would you please explain more why is clearing the default method better > than using a wrong default one, and why you chose to do this just for > the Rev A SFDP flashes and you didn't include the no-SFDP flashes as > well? Honestly, I don't care too much about this flash. I can't remember any details from this 4 months old thread. Sorry. I guess it is fine to drop this patch. If someone cares, she or he can resurrect this one. -michael
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c index a5211543d30d..6bba9b601846 100644 --- a/drivers/mtd/spi-nor/sfdp.c +++ b/drivers/mtd/spi-nor/sfdp.c @@ -549,6 +549,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, map->uniform_erase_type = map->uniform_region.offset & SNOR_ERASE_TYPE_MASK; + /* + * The first JESD216 revision doesn't specify a method to enable + * quad mode. spi_nor_init_default_params() will set a legacy + * default method to enable quad mode. We have to disable it + * again. + * Flashes with this JESD216 revision need to set the quad_enable + * method in their post_bfpt() fixup if they want to use quad I/O. + */ + params->quad_enable = NULL; + /* Stop here if not JESD216 rev A or later. */ if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); @@ -562,7 +572,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, /* Quad Enable Requirements. */ switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { case BFPT_DWORD15_QER_NONE: - params->quad_enable = NULL; break; case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: