In the latest JEDEC standard (JESD216F), there are now bitfields in the
4 byte address instruction table for 1S-1S-8S and 1S-8S-8S modes.
This patchset adds support for checking the 4BAIT for these modes.
v2: Move page program commands into sfdp.c instead of core.c,
as this appears to conform more closely with spi-nor paradigm.
Page program buswidth appears to be automatically determined, so let's
follow suit and do the same.
v3:
- Added missing SPI_NOR_OCTAL_READ_1_8_8 to spi_nor_sfdp check in
spi_nor_init_params_deprecated()
- Convert IS25LX256 to 1S-8S-8S instead of 1S-1S-8S
- Tested and confirmed both 1S-1S-8S and 1S-8S-8S work on IS25LX256
v4:
- Remove SPI_NOR_OCTAL_READ_1_8_8, this was following a defunct path
- Correct the IS25LX256's missing BFPT info via a fixup instead
Nathan Barrett-Morrison (3):
mtd: spi-nor: Extend SFDP 4byte address instruction lookup table with
new octal modes as per JEDEC JESD216F
mtd: spi-nor: Add additional octal-mode page program flags to be
checked during SFDP 4BAIT parsing
mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal
read mode
drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/sfdp.c | 13 +++++++++++++
2 files changed, 45 insertions(+)
--
2.30.2
This adds the new bit fields for
reading: 1S-1S-8S, 1S-8S-8S, 1D-8D-8D
programming: 1S-1S-8S, 1S-8S-8S
Signed-off-by: Nathan Barrett-Morrison <[email protected]>
---
drivers/mtd/spi-nor/sfdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 2257f1b4c2e2..e4e87815ba94 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -953,11 +953,16 @@ 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) },
+ { SNOR_HWCAPS_READ_1_8_8, BIT(21) },
+ { SNOR_HWCAPS_READ_1_8_8_DTR, BIT(22) },
};
static const struct sfdp_4bait programs[] = {
{ SNOR_HWCAPS_PP, BIT(6) },
{ SNOR_HWCAPS_PP_1_1_4, BIT(7) },
{ SNOR_HWCAPS_PP_1_4_4, BIT(8) },
+ { SNOR_HWCAPS_PP_1_1_8, BIT(23) },
+ { SNOR_HWCAPS_PP_1_8_8, BIT(24) },
};
static const struct sfdp_4bait erases[SNOR_ERASE_TYPE_MAX] = {
{ 0u /* not used */, BIT(9) },
--
2.30.2
This adds the IS25LX256 chip into the ISSI flash_info parts table
Signed-off-by: Nathan Barrett-Morrison <[email protected]>
---
drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..362bc3603d8f 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups = {
.post_bfpt = is25lp256_post_bfpt_fixups,
};
+static int
+is25lx256_post_bfpt_fixups(struct spi_nor *nor,
+ const struct sfdp_parameter_header *bfpt_header,
+ const struct sfdp_bfpt *bfpt)
+{
+ /*
+ * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
+ * However, the BFPT does not contain any information denoting this
+ * functionality, so the proper fast read opcodes are never setup.
+ * We're correcting this issue via the fixup below. Page program
+ * commands are detected and setup properly via the 4BAIT lookup.
+ */
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
+ 0, 8, SPINOR_OP_READ_1_1_8,
+ SNOR_PROTO_1_1_8);
+
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
+ 0, 16, SPINOR_OP_READ_1_8_8,
+ SNOR_PROTO_1_8_8);
+
+ return 0;
+}
+
+static const struct spi_nor_fixups is25lx256_fixups = {
+ .post_bfpt = is25lx256_post_bfpt_fixups,
+};
+
static void pm25lv_nor_late_init(struct spi_nor *nor)
{
struct spi_nor_erase_map *map = &nor->params->erase_map;
@@ -74,6 +103,9 @@ static const struct flash_info issi_nor_parts[] = {
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
.fixups = &is25lp256_fixups },
+ { "is25lx256", INFO(0x9d5a19, 0, 0, 0)
+ PARSE_SFDP
+ .fixups = &is25lx256_fixups },
/* PMC */
{ "pm25lv512", INFO(0, 0, 32 * 1024, 2)
--
2.30.2
This adds some support for automatically searching a chip's SFDP table for:
program commands: 1S-1S-8S, 1S-8S-8S
Signed-off-by: Nathan Barrett-Morrison <[email protected]>
---
drivers/mtd/spi-nor/sfdp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index e4e87815ba94..e1b7547bf81e 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1089,6 +1089,14 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_4_4],
SPINOR_OP_PP_1_4_4_4B,
SNOR_PROTO_1_4_4);
+ if (pp_hwcaps & SNOR_HWCAPS_PP_1_1_8)
+ spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_1_8],
+ SPINOR_OP_PP_1_1_8_4B,
+ SNOR_PROTO_1_1_8);
+ if (pp_hwcaps & SNOR_HWCAPS_PP_1_8_8)
+ spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_8_8],
+ SPINOR_OP_PP_1_8_8_4B,
+ SNOR_PROTO_1_8_8);
for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++) {
if (erase_mask & BIT(i))
--
2.30.2
Am 2022-12-02 14:55, schrieb Nathan Barrett-Morrison:
> This adds the IS25LX256 chip into the ISSI flash_info parts table
>
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
> ---
> drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..362bc3603d8f 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups
> = {
> .post_bfpt = is25lp256_post_bfpt_fixups,
> };
>
> +static int
> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
> + const struct sfdp_parameter_header *bfpt_header,
> + const struct sfdp_bfpt *bfpt)
> +{
> + /*
> + * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
> + * However, the BFPT does not contain any information denoting this
> + * functionality, so the proper fast read opcodes are never setup.
> + * We're correcting this issue via the fixup below. Page program
> + * commands are detected and setup properly via the 4BAIT lookup.
> + */
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
> + 0, 8, SPINOR_OP_READ_1_1_8,
> + SNOR_PROTO_1_1_8);
> +
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
> + 0, 16, SPINOR_OP_READ_1_8_8,
> + SNOR_PROTO_1_8_8);
> +
> + return 0;
> +}
> +
> +static const struct spi_nor_fixups is25lx256_fixups = {
> + .post_bfpt = is25lx256_post_bfpt_fixups,
> +};
> +
> static void pm25lv_nor_late_init(struct spi_nor *nor)
> {
> struct spi_nor_erase_map *map = &nor->params->erase_map;
> @@ -74,6 +103,9 @@ static const struct flash_info issi_nor_parts[] = {
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> .fixups = &is25lp256_fixups },
> + { "is25lx256", INFO(0x9d5a19, 0, 0, 0)
> + PARSE_SFDP
> + .fixups = &is25lx256_fixups },
Very nice!
Subject is slightly wrong because you fix up the BFPT to get
correct 1-1-8 and 1-8-8 read modes.
With that fixed:
Reviewed-by: Michael Walle <[email protected]>
Am 2022-12-02 14:55, schrieb Nathan Barrett-Morrison:
> This adds the new bit fields for
> reading: 1S-1S-8S, 1S-8S-8S, 1D-8D-8D
> programming: 1S-1S-8S, 1S-8S-8S
>
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
Am 2022-12-02 14:55, schrieb Nathan Barrett-Morrison:
> This adds some support for automatically searching a chip's SFDP table
> for:
>
> program commands: 1S-1S-8S, 1S-8S-8S
>
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
Maybe a bit shorter subject and more text in the commit
message. But I don't care too much.
Reviewed-by: Michael Walle <[email protected]>
Hi Nathan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v6.1-rc7]
[also build test ERROR on linus/master next-20221202]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
patch link: https://lore.kernel.org/r/20221202135539.271936-4-nathan.morrison%40timesys.com
patch subject: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode
config: i386-randconfig-a001
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/26787a4933a6a8591b6a66fc036e3b3c674cb57c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
git checkout 26787a4933a6a8591b6a66fc036e3b3c674cb57c
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/mtd/spi-nor/issi.c: In function 'is25lx256_post_bfpt_fixups':
>> drivers/mtd/spi-nor/issi.c:44:9: error: 'params' undeclared (first use in this function); did you mean 'parameq'?
44 | params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
| ^~~~~~
| parameq
drivers/mtd/spi-nor/issi.c:44:9: note: each undeclared identifier is reported only once for each function it appears in
vim +44 drivers/mtd/spi-nor/issi.c
31
32 static int
33 is25lx256_post_bfpt_fixups(struct spi_nor *nor,
34 const struct sfdp_parameter_header *bfpt_header,
35 const struct sfdp_bfpt *bfpt)
36 {
37 /*
38 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
39 * However, the BFPT does not contain any information denoting this
40 * functionality, so the proper fast read opcodes are never setup.
41 * We're correcting this issue via the fixup below. Page program
42 * commands are detected and setup properly via the 4BAIT lookup.
43 */
> 44 params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
45 spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
46 0, 8, SPINOR_OP_READ_1_1_8,
47 SNOR_PROTO_1_1_8);
48
49 params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
50 spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
51 0, 16, SPINOR_OP_READ_1_8_8,
52 SNOR_PROTO_1_8_8);
53
54 return 0;
55 }
56
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Nathan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v6.1-rc7]
[also build test ERROR on linus/master next-20221202]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
patch link: https://lore.kernel.org/r/20221202135539.271936-4-nathan.morrison%40timesys.com
patch subject: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode
config: x86_64-randconfig-a001
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/26787a4933a6a8591b6a66fc036e3b3c674cb57c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
git checkout 26787a4933a6a8591b6a66fc036e3b3c674cb57c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/mtd/spi-nor/issi.c:44:2: error: use of undeclared identifier 'params'; did you mean 'parameq'?
params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
^~~~~~
parameq
include/linux/moduleparam.h:372:13: note: 'parameq' declared here
extern bool parameq(const char *name1, const char *name2);
^
>> drivers/mtd/spi-nor/issi.c:44:8: error: member reference base type 'bool (const char *, const char *)' (aka '_Bool (const char *, const char *)') is not a structure or union
params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
~~~~~~^ ~~~~~~
drivers/mtd/spi-nor/issi.c:45:29: error: use of undeclared identifier 'params'
spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
^
drivers/mtd/spi-nor/issi.c:49:2: error: use of undeclared identifier 'params'; did you mean 'parameq'?
params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
^~~~~~
parameq
include/linux/moduleparam.h:372:13: note: 'parameq' declared here
extern bool parameq(const char *name1, const char *name2);
^
drivers/mtd/spi-nor/issi.c:49:8: error: member reference base type 'bool (const char *, const char *)' (aka '_Bool (const char *, const char *)') is not a structure or union
params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
~~~~~~^ ~~~~~~
drivers/mtd/spi-nor/issi.c:50:29: error: use of undeclared identifier 'params'
spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
^
6 errors generated.
vim +44 drivers/mtd/spi-nor/issi.c
31
32 static int
33 is25lx256_post_bfpt_fixups(struct spi_nor *nor,
34 const struct sfdp_parameter_header *bfpt_header,
35 const struct sfdp_bfpt *bfpt)
36 {
37 /*
38 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
39 * However, the BFPT does not contain any information denoting this
40 * functionality, so the proper fast read opcodes are never setup.
41 * We're correcting this issue via the fixup below. Page program
42 * commands are detected and setup properly via the 4BAIT lookup.
43 */
> 44 params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
45 spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
46 0, 8, SPINOR_OP_READ_1_1_8,
47 SNOR_PROTO_1_1_8);
48
49 params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
50 spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
51 0, 16, SPINOR_OP_READ_1_8_8,
52 SNOR_PROTO_1_8_8);
53
54 return 0;
55 }
56
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi, Nathan,
The series is starting to look good, but I'll need another version,
please.
On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
> This adds the IS25LX256 chip into the ISSI flash_info parts table
Describe your changes in imperative mood, e.g. "Add support for
S25LX256" instead of "This adds ..."
It may worth to re-read
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
once in a while.
>
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
> ---
> drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..362bc3603d8f 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups = {
> .post_bfpt = is25lp256_post_bfpt_fixups,
> };
>
> +static int
> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
> + const struct sfdp_parameter_header *bfpt_header,
> + const struct sfdp_bfpt *bfpt)
> +{
> + /*
> + * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
> + * However, the BFPT does not contain any information denoting this
> + * functionality, so the proper fast read opcodes are never setup.
> + * We're correcting this issue via the fixup below. Page program
> + * commands are detected and setup properly via the 4BAIT lookup.
> + */
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
add a reference to params, the build fails here.
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_8],
> + 0, 8, SPINOR_OP_READ_1_1_8,
> + SNOR_PROTO_1_1_8);
> +
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_8_8],
> + 0, 16, SPINOR_OP_READ_1_8_8,
> + SNOR_PROTO_1_8_8);
> +
> + return 0;
> +}
> +
> +static const struct spi_nor_fixups is25lx256_fixups = {
> + .post_bfpt = is25lx256_post_bfpt_fixups,
> +};
> +
> static void pm25lv_nor_late_init(struct spi_nor *nor)
> {
> struct spi_nor_erase_map *map = &nor->params->erase_map;
> @@ -74,6 +103,9 @@ static const struct flash_info issi_nor_parts[] = {
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> .fixups = &is25lp256_fixups },
> + { "is25lx256", INFO(0x9d5a19, 0, 0, 0)
> + PARSE_SFDP
> + .fixups = &is25lx256_fixups },
For every new flash addition or update, we ask contributors to do some
little tests. Would you please run the commands from below and send us
the output?
# dd if=/dev/urandom of=./qspi_test bs=1M count=6
6+0 records in
6+0 records out
# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
# mtd_debug erase /dev/mtd4 0 6291456
Erased 6291456 bytes from address 0x00000000 in flash
# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
# hexdump qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000
# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
# sha1sum qspi_test qspi_read
57f8d4fee65622104e24276e865f662844f12242 qspi_test
57f8d4fee65622104e24276e865f662844f12242 qspi_read
# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
is25wp256
# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
9d7019
# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
issi
# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff9d05010380000002ffffffffffff
ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b
083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff
ffffffffffffffff501950169ff9c0648fefffff
# md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
ba14818b9ec42713f24d94d66bb90ba0 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
On 26.12.2022 10:04, Tudor Ambarus wrote:
> Hi, Nathan,
>
> The series is starting to look good, but I'll need another version,
> please.
>
> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>
> Describe your changes in imperative mood, e.g. "Add support for
> S25LX256" instead of "This adds ..."
>
> It may worth to re-read
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> once in a while.
>
>>
>> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
>> ---
>> drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>> index 89a66a19d754..362bc3603d8f 100644
>> --- a/drivers/mtd/spi-nor/issi.c
>> +++ b/drivers/mtd/spi-nor/issi.c
>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups
>> = {
>> .post_bfpt = is25lp256_post_bfpt_fixups,
>> };
>> +static int
>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>> + const struct sfdp_parameter_header *bfpt_header,
>> + const struct sfdp_bfpt *bfpt)
>> +{
>> + /*
>> + * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>> + * However, the BFPT does not contain any information denoting this
>> + * functionality, so the proper fast read opcodes are never setup.
>> + * We're correcting this issue via the fixup below. Page program
>> + * commands are detected and setup properly via the 4BAIT lookup.
>> + */
Why don't you set the READ support when parsing the 4bait table? We need
to see the SFDP dump to determine how we treat this. I'm not sure a
post_bfpt hook is the right thing to do for this flash.
Thanks,
ta
Looks good.
On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
> This adds the new bit fields for
> reading: 1S-1S-8S, 1S-8S-8S, 1D-8D-8D
> programming: 1S-1S-8S, 1S-8S-8S
>
We usually aim to have ~60 chars for the subject line and 75 chars per
line for the commit message. If you want to reword both, fine, otherwise
I'll do it when applying.
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
> ---
> drivers/mtd/spi-nor/sfdp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 2257f1b4c2e2..e4e87815ba94 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -953,11 +953,16 @@ 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) },
> + { SNOR_HWCAPS_READ_1_8_8, BIT(21) },
> + { SNOR_HWCAPS_READ_1_8_8_DTR, BIT(22) },
> };
> static const struct sfdp_4bait programs[] = {
> { SNOR_HWCAPS_PP, BIT(6) },
> { SNOR_HWCAPS_PP_1_1_4, BIT(7) },
> { SNOR_HWCAPS_PP_1_4_4, BIT(8) },
> + { SNOR_HWCAPS_PP_1_1_8, BIT(23) },
> + { SNOR_HWCAPS_PP_1_8_8, BIT(24) },
> };
> static const struct sfdp_4bait erases[SNOR_ERASE_TYPE_MAX] = {
> { 0u /* not used */, BIT(9) },
On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
> This adds some support for automatically searching a chip's SFDP table for:
you can drop ":"
>
and this new line
> program commands: 1S-1S-8S, 1S-8S-8S
>
> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
> ---
> drivers/mtd/spi-nor/sfdp.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index e4e87815ba94..e1b7547bf81e 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -1089,6 +1089,14 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
> spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_4_4],
> SPINOR_OP_PP_1_4_4_4B,
> SNOR_PROTO_1_4_4);
> + if (pp_hwcaps & SNOR_HWCAPS_PP_1_1_8)
> + spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_1_8],
> + SPINOR_OP_PP_1_1_8_4B,
> + SNOR_PROTO_1_1_8);
> + if (pp_hwcaps & SNOR_HWCAPS_PP_1_8_8)
> + spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_8_8],
> + SPINOR_OP_PP_1_8_8_4B,
> + SNOR_PROTO_1_8_8);
Why did you choose to not add support for reads as well?
>
> for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++) {
> if (erase_mask & BIT(i))
Am 2022-12-26 09:17, schrieb Tudor Ambarus:
> On 26.12.2022 10:04, Tudor Ambarus wrote:
>> Hi, Nathan,
>>
>> The series is starting to look good, but I'll need another version,
>> please.
>>
>> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>>
>> Describe your changes in imperative mood, e.g. "Add support for
>> S25LX256" instead of "This adds ..."
>>
>> It may worth to re-read
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>> once in a while.
>>
>>>
>>> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
>>> ---
>>> drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>> 1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>>> index 89a66a19d754..362bc3603d8f 100644
>>> --- a/drivers/mtd/spi-nor/issi.c
>>> +++ b/drivers/mtd/spi-nor/issi.c
>>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups
>>> is25lp256_fixups = {
>>> .post_bfpt = is25lp256_post_bfpt_fixups,
>>> };
>>> +static int
>>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>>> + const struct sfdp_parameter_header *bfpt_header,
>>> + const struct sfdp_bfpt *bfpt)
>>> +{
>>> + /*
>>> + * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>>> + * However, the BFPT does not contain any information denoting
>>> this
>>> + * functionality, so the proper fast read opcodes are never
>>> setup.
>>> + * We're correcting this issue via the fixup below. Page
>>> program
>>> + * commands are detected and setup properly via the 4BAIT
>>> lookup.
>>> + */
>
> Why don't you set the READ support when parsing the 4bait table?
That would deviate from the read handling of all the other modes.
> We need to see the SFDP dump to determine how we treat this. I'm not
> sure a post_bfpt hook is the right thing to do for this flash.
See:
https://lore.kernel.org/linux-mtd/[email protected]/
But yes, I missed that you should include the sfdp dump and some other
commands in the comment section of the patch which adds support for that
flash (see Tudors reply).
Tudor, what is the status of your documentation patch? I'd really like
to refer to the kernel docs instead of having write the same over and
over again ;)
-michael
On 27.12.2022 14:37, Michael Walle wrote:
> Am 2022-12-26 09:17, schrieb Tudor Ambarus:
>> On 26.12.2022 10:04, Tudor Ambarus wrote:
>>> Hi, Nathan,
>>>
>>> The series is starting to look good, but I'll need another version,
>>> please.
>>>
>>> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>>>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>>>
>>> Describe your changes in imperative mood, e.g. "Add support for
>>> S25LX256" instead of "This adds ..."
>>>
>>> It may worth to re-read
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>> once in a while.
>>>
>>>>
>>>> Signed-off-by: Nathan Barrett-Morrison <[email protected]>
>>>> ---
>>>> drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>>> 1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>>>> index 89a66a19d754..362bc3603d8f 100644
>>>> --- a/drivers/mtd/spi-nor/issi.c
>>>> +++ b/drivers/mtd/spi-nor/issi.c
>>>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups
>>>> is25lp256_fixups = {
>>>> .post_bfpt = is25lp256_post_bfpt_fixups,
>>>> };
>>>> +static int
>>>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>>>> + const struct sfdp_parameter_header *bfpt_header,
>>>> + const struct sfdp_bfpt *bfpt)
>>>> +{
>>>> + /*
>>>> + * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>>>> + * However, the BFPT does not contain any information denoting
>>>> this
>>>> + * functionality, so the proper fast read opcodes are never setup.
>>>> + * We're correcting this issue via the fixup below. Page program
>>>> + * commands are detected and setup properly via the 4BAIT lookup.
>>>> + */
>>
>> Why don't you set the READ support when parsing the 4bait table?
>
> That would deviate from the read handling of all the other modes.
Oh, I see now, so you need to amend BFPT with the 3B opcodes so that the
4bait table convert the 3B read opcodes to 4B. That's fine, yes.
>
>> We need to see the SFDP dump to determine how we treat this. I'm not
>> sure a post_bfpt hook is the right thing to do for this flash.
>
> See:
> https://lore.kernel.org/linux-mtd/[email protected]/
Okay, it looks fine. I still need the write, erase, verify erase, write,
read and compare test.
>
> But yes, I missed that you should include the sfdp dump and some other
> commands in the comment section of the patch which adds support for that
> flash (see Tudors reply).
>
Nathan, please resubmit addressing those small comments if you can.
> Tudor, what is the status of your documentation patch? I'd really like
> to refer to the kernel docs instead of having write the same over and
> over again ;)
I hope to have it ready for 6.3.
Cheers,
ta