diff mbox series

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

Message ID 20231214073344.66644-2-jaimeliao.tw@gmail.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series Get the 1-1-8 protocol from BFPT | expand

Commit Message

liao jaime Dec. 14, 2023, 7:33 a.m. UTC 
From: JaimeLiao <jaimeliao@mxic.com.tw>

BFPT 17th DWORD contains the informations about 1-1-8,
Parse BFPT 17 DWORD instruction to determine
whether flash support 1-1-8, and set its dummy cycle
accordingly.

Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
 drivers/mtd/spi-nor/sfdp.c | 13 ++++++++++++-
 drivers/mtd/spi-nor/sfdp.h |  3 +++
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Michael Walle Dec. 14, 2023, 9:48 a.m. UTC | #1 
Hi,

> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> BFPT 17th DWORD contains the informations about 1-1-8,
> Parse BFPT 17 DWORD instruction to determine
> whether flash support 1-1-8, and set its dummy cycle
> accordingly.
> 
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>

Reviewed-by: Michael Walle <michael@walle.cc>

> ---

no need for a 0/N cover letter just for the changelog. That can
be included here below the "---".

And you can also keep my Rb tag.

-michael
liao jaime Dec. 14, 2023, 10:06 a.m. UTC | #2 
Hi Michael

>
> Hi,
>
> > From: JaimeLiao <jaimeliao@mxic.com.tw>
> >
> > BFPT 17th DWORD contains the informations about 1-1-8,
> > Parse BFPT 17 DWORD instruction to determine
> > whether flash support 1-1-8, and set its dummy cycle
> > accordingly.
> >
> > Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
>
> Reviewed-by: Michael Walle <michael@walle.cc>
>
> > ---
>
> no need for a 0/N cover letter just for the changelog. That can
> be included here below the "---".
>
> And you can also keep my Rb tag.
Got it.
Would this version be accepted or should I send a new one?

>
> -michael

Thanks
Jaime
Michael Walle Dec. 14, 2023, 10:09 a.m. UTC | #3 
>> > From: JaimeLiao <jaimeliao@mxic.com.tw>
>> >
>> > BFPT 17th DWORD contains the informations about 1-1-8,
>> > Parse BFPT 17 DWORD instruction to determine
>> > whether flash support 1-1-8, and set its dummy cycle
>> > accordingly.
>> >
>> > Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
>> 
>> Reviewed-by: Michael Walle <michael@walle.cc>
>> 
>> > ---
>> 
>> no need for a 0/N cover letter just for the changelog. That can
>> be included here below the "---".
>> 
>> And you can also keep my Rb tag.
> Got it.
> Would this version be accepted or should I send a new one?

No need for a new one. Just for your future patches.

-michael
liao jaime Dec. 14, 2023, 10:11 a.m. UTC | #4 
Hi Michael

>
> >> > From: JaimeLiao <jaimeliao@mxic.com.tw>
> >> >
> >> > BFPT 17th DWORD contains the informations about 1-1-8,
> >> > Parse BFPT 17 DWORD instruction to determine
> >> > whether flash support 1-1-8, and set its dummy cycle
> >> > accordingly.
> >> >
> >> > Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> >>
> >> Reviewed-by: Michael Walle <michael@walle.cc>
> >>
> >> > ---
> >>
> >> no need for a 0/N cover letter just for the changelog. That can
> >> be included here below the "---".
> >>
> >> And you can also keep my Rb tag.
> > Got it.
> > Would this version be accepted or should I send a new one?
>
> No need for a new one. Just for your future patches.
Thank for your reply.

>
> -michael

Thank
Jaime
Tudor Ambarus Dec. 15, 2023, 6:45 a.m. UTC | #5 
Hi, Jaime,

Please update the commit subject to indicate that you parse 4BAIT as well.

On 14.12.2023 09:33, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@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@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

> +
>  	/* 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 */
liao jaime Dec. 15, 2023, 6:59 a.m. UTC | #6 
Hi Tudor


>
> 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@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@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.
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.
1-1-8 read instruction could determine wether 1-1-8 support or not.

>
> > +
> >       /* 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
Tudor Ambarus Dec. 15, 2023, 7:12 a.m. UTC | #7 
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@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@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.

> 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 :).

> 1-1-8 read instruction could determine wether 1-1-8 support or not.

Right. Then introduce a dedicated logic for these 2.

> 
>>
>>> +
>>>       /* 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
liao jaime Dec. 15, 2023, 7:27 a.m. UTC | #8 
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@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@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
diff mbox series

Patch

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);
+	}
+
 	/* 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) },
 	};
 	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 */