[PATCH v2 1/1] mtd: spi-nor: sfdp: Get the 1-1-8 protocol from BFPT

[liao jaime]

Hi Tudor

>
>
>
> On 15.12.2023 08:59, liao jaime wrote:
> > Hi Tudor
> >
>
> Hi!
> >
> >>
> >> Hi, Jaime,
> >>
> >> Please update the commit subject to indicate that you parse 4BAIT as well.
> > Got it.
> >
> >>
> >> On 14.12.2023 09:33, Jaime Liao wrote:
> >>> From: JaimeLiao <jaimeliao at mxic.com.tw>
> >>>
> >>> BFPT 17th DWORD contains the informations about 1-1-8,
> >>
> >> Please run checkpatch on further submissions:
> >> ./scripts/checkpatch.pl --strict 000*
> >> WARNING: 'informations' may be misspelled - perhaps 'information'?
> >> #6:
> >> BFPT 17th DWORD contains the informations about 1-1-8,
> >>                              ^^^^^^^^^^^^
> >>
> >> total: 0 errors, 1 warnings, 0 checks, 46 lines checked
> >>
> >>
> >>> Parse BFPT 17 DWORD instruction to determine
> >>> whether flash support 1-1-8, and set its dummy cycle
> >>> accordingly.
> >>>
> >>> Signed-off-by: JaimeLiao <jaimeliao at mxic.com.tw>
> >>> ---
> >>>  drivers/mtd/spi-nor/sfdp.c | 13 ++++++++++++-
> >>>  drivers/mtd/spi-nor/sfdp.h |  3 +++
> >>>  2 files changed, 15 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> >>> index b3b11dfed789..c55f39978762 100644
> >>> --- a/drivers/mtd/spi-nor/sfdp.c
> >>> +++ b/drivers/mtd/spi-nor/sfdp.c
> >>> @@ -446,6 +446,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >>>       u32 dword;
> >>>       u16 half;
> >>>       u8 erase_mask;
> >>> +     u8 dummy, opcode;
> >>>
> >>>       /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
> >>>       if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> >>> @@ -525,7 +526,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >>>       for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_erases); i++) {
> >>>               const struct sfdp_bfpt_erase *er = &sfdp_bfpt_erases[i];
> >>>               u32 erasesize;
> >>> -             u8 opcode;
> >>>
> >>>               half = bfpt.dwords[er->dword] >> er->shift;
> >>>               erasesize = half & 0xff;
> >>> @@ -631,6 +631,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >>>       if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
> >>>               return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
> >>>
> >>> +     /* Parse 1-1-8 read instruction */
> >>> +     opcode = FIELD_GET(BFPT_DWORD17_RD_1_1_8_CMD, bfpt.dwords[SFDP_DWORD(17)]);
> >>> +     if (opcode) {
> >>> +             dummy = FIELD_GET(BFPT_DWORD17_RD_1_1_8_DUMMY, bfpt.dwords[SFDP_DWORD(17)]);
> >>> +             nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> >>> +             spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_1_1_8],
> >>> +                                       0, dummy, opcode,
> >>> +                                       SNOR_PROTO_1_1_8);
> >>> +     }
> >>
> >> You missed the mode clocks. You missed to parse 1-8-8. Please check how
> >> the read settings are parsed and set in parse_bfpt() and do the same
> >> here. See:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/drivers/mtd/spi-nor/sfdp.c?h=spi-nor/next#n502
> > Macronix Flash didn't support 1-8-8 feature.
> > So that I have no sample for validate it.
>
> don't worry, add it anyway. Specify in the commit message that you
> validated just 1-1-8.
Got it.

>
> > About read setting in parse_bfpt(), that is a good way for scan and
> > set for each protocol.
> > But 1-1-8 support judgement is not a single bit.
>
> oh the horror. And then it becomes a single bit judgement in bfpt dword
> 21. Nice consistency :).
As I know JESD216F BFPT 21th dowrd includes information about 1s-4d-4d and etc.
I didn't get it.

>
> > 1-1-8 read instruction could determine wether 1-1-8 support or not.
>
> Right. Then introduce a dedicated logic for these 2.
OK.

>
> >
> >>
> >>> +
> >>>       /* 8D-8D-8D command extension. */
> >>>       switch (bfpt.dwords[SFDP_DWORD(18)] & BFPT_DWORD18_CMD_EXT_MASK) {
> >>>       case BFPT_DWORD18_CMD_EXT_REP:
> >>> @@ -968,6 +978,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
> >>>               { SNOR_HWCAPS_READ_1_1_1_DTR,   BIT(13) },
> >>>               { SNOR_HWCAPS_READ_1_2_2_DTR,   BIT(14) },
> >>>               { SNOR_HWCAPS_READ_1_4_4_DTR,   BIT(15) },
> >>> +             { SNOR_HWCAPS_READ_1_1_8,       BIT(20) },
> >>
> >> let's set 1-8-8 as well please
> >>
> >>>       };
> >>>       static const struct sfdp_4bait programs[] = {
> >>>               { SNOR_HWCAPS_PP,               BIT(6) },
> >>> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> >>> index 6eb99e1cdd61..584a05f9931d 100644
> >>> --- a/drivers/mtd/spi-nor/sfdp.h
> >>> +++ b/drivers/mtd/spi-nor/sfdp.h
> >>> @@ -118,6 +118,9 @@ struct sfdp_bfpt {
> >>>       (BFPT_DWORD16_EN4B_EN4B | BFPT_DWORD16_EX4B_EX4B)
> >>>  #define BFPT_DWORD16_SWRST_EN_RST            BIT(12)
> >>>
> >>> +#define BFPT_DWORD17_RD_1_1_8_CMD            GENMASK(31, 24)
> >>> +#define BFPT_DWORD17_RD_1_1_8_DUMMY          GENMASK(20, 16)
> >>> +
> >>>  #define BFPT_DWORD18_CMD_EXT_MASK            GENMASK(30, 29)
> >>>  #define BFPT_DWORD18_CMD_EXT_REP             (0x0UL << 29) /* Repeat */
> >>>  #define BFPT_DWORD18_CMD_EXT_INV             (0x1UL << 29) /* Invert */
> >
> >
> > Thanks
> > Jaime

Thanks
Jaime