| Message ID | 20220912051317.2369-1-Takahiro.Kuwano@infineon.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: sfdp: Fix index value for SCCR dwords | expand |
Am 2022-09-12 07:13, schrieb tkuw584924@gmail.com: > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > Array index for SCCR 22th DOWRD should be 21. > > Fixes: 981a8d60e01f ("mtd: spi-nor: Parse SFDP SCCR Map") > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> Reviewed-by: Michael Walle <michael@walle.cc>
Hi, Takahiro, On 9/12/22 08:13, tkuw584924@gmail.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > Array index for SCCR 22th DOWRD should be 21. > > Fixes: 981a8d60e01f ("mtd: spi-nor: Parse SFDP SCCR Map") > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > --- > v2: Add Fixes tag. > > drivers/mtd/spi-nor/sfdp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index 2257f1b4c2e2..681e5e78724c 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -1222,7 +1222,7 @@ static int spi_nor_parse_sccr(struct spi_nor *nor, > > le32_to_cpu_array(dwords, sccr_header->length); > > - if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[22])) > + if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[21])) Would such a change work? diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h index bbf80d2990ab..e83d9bd89066 100644 --- a/drivers/mtd/spi-nor/sfdp.h +++ b/drivers/mtd/spi-nor/sfdp.h @@ -19,7 +19,7 @@ * JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs. * They are indexed from 1 but C arrays are indexed from 0. */ -#define BFPT_DWORD(i) ((i) - 1) +#define SFDP_DWORD(i) ((i) - 1) and use: + if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[SFDP_DWORD(22)])) and dwords may be renamed to sccrm, from "Status, Control and Configuration Register Map", so: + if (FIELD_GET(SCCRM_DWORD22_OCTAL_DTR_EN_VOLATILE, sccrm[SFDP_DWORD(22)])) But you'll have to check that all the tables from the latest jesd216 (e version I guess) use the same kind of indexing. Let me know if you'd like to pursue my suggestion instead.
On 10/3/2022 3:07 PM, Tudor.Ambarus@microchip.com wrote: > Hi, Takahiro, > > On 9/12/22 08:13, tkuw584924@gmail.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> >> Array index for SCCR 22th DOWRD should be 21. >> >> Fixes: 981a8d60e01f ("mtd: spi-nor: Parse SFDP SCCR Map") >> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> --- >> v2: Add Fixes tag. >> >> drivers/mtd/spi-nor/sfdp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c >> index 2257f1b4c2e2..681e5e78724c 100644 >> --- a/drivers/mtd/spi-nor/sfdp.c >> +++ b/drivers/mtd/spi-nor/sfdp.c >> @@ -1222,7 +1222,7 @@ static int spi_nor_parse_sccr(struct spi_nor *nor, >> >> le32_to_cpu_array(dwords, sccr_header->length); >> >> - if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[22])) >> + if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[21])) > > Would such a change work? > diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h > index bbf80d2990ab..e83d9bd89066 100644 > --- a/drivers/mtd/spi-nor/sfdp.h > +++ b/drivers/mtd/spi-nor/sfdp.h > @@ -19,7 +19,7 @@ > * JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs. > * They are indexed from 1 but C arrays are indexed from 0. > */ > -#define BFPT_DWORD(i) ((i) - 1) > +#define SFDP_DWORD(i) ((i) - 1) > > > and use: > + if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[SFDP_DWORD(22)])) > > and dwords may be renamed to sccrm, from "Status, Control and Configuration Register Map", > so: > + if (FIELD_GET(SCCRM_DWORD22_OCTAL_DTR_EN_VOLATILE, sccrm[SFDP_DWORD(22)])) > > But you'll have to check that all the tables from the latest jesd216 (e version I guess) > use the same kind of indexing. Let me know if you'd like to pursue my suggestion instead. > Yes, this should work and I like to follow your suggestion. I will check the JESD216 (revision F is the latest). Thanks, Takahiro
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c index 2257f1b4c2e2..681e5e78724c 100644 --- a/drivers/mtd/spi-nor/sfdp.c +++ b/drivers/mtd/spi-nor/sfdp.c @@ -1222,7 +1222,7 @@ static int spi_nor_parse_sccr(struct spi_nor *nor, le32_to_cpu_array(dwords, sccr_header->length); - if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[22])) + if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[21])) nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; out: