Messages in this thread | | | From | <> | Subject | Re: [PATCH v1] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it | Date | Mon, 7 Mar 2022 09:23:08 +0000 |
| |
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/
| |