Hello Mike Banon,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/30877
to review the following change.
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips
Required for ACPI S3 suspend support at some motherboards. Synchronizing with flashchips.c/h flashrom source code.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4508a65a5bdcbf58aadf452de5e896fc3c5b1bc3 --- M src/drivers/spi/macronix.c 1 file changed, 83 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/30877/1
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 7ed70c0..1fac076 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -80,22 +80,6 @@ .name = "MX25L3205D", }, { - .idcode = 0x5e16, - .page_size = 256, - .pages_per_sector = 16, - .sectors_per_block = 16, - .nr_blocks = 64, - .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */ - }, - { - .idcode = 0x2536, - .page_size = 256, - .pages_per_sector = 16, - .sectors_per_block = 16, - .nr_blocks = 64, - .name = "MX25L3239E", - }, - { .idcode = 0x2017, .page_size = 256, .pages_per_sector = 16, @@ -112,12 +96,60 @@ .name = "MX25L12805D", }, { - .idcode = 0x2618, + .idcode = 0x2019, .page_size = 256, .pages_per_sector = 16, .sectors_per_block = 16, - .nr_blocks = 256, - .name = "MX25L12855E", + .nr_blocks = 512, + .name = "MX25L25635F", + }, + { + .idcode = 0x201a, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 1024, + .name = "MX66L51235F", + }, + { + .idcode = 0x2415, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25L1635D", + }, + { + .idcode = 0x2515, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25L1635E", + }, + { + .idcode = 0x2534, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 16, + .name = "MX25U8032E", + }, + { + .idcode = 0x2535, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25U1635E", + }, + { + .idcode = 0x2536, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 64, + .name = "MX25U3235E", }, { .idcode = 0x2537, @@ -136,6 +168,38 @@ .name = "MX25U12835F", }, { + .idcode = 0x2539, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 512, + .name = "MX25U25635F", + }, + { + .idcode = 0x253a, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 1024, + .name = "MX25U51245G", + }, + { + .idcode = 0x2618, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 256, + .name = "MX25L12855E", + }, + { + .idcode = 0x5e16, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 64, + .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */ + }, + { .idcode = 0x9517, .page_size = 256, .pages_per_sector = 16,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c File src/drivers/spi/macronix.c:
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c@200 PS1, Line 200: .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */ line over 80 characters
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
Patch Set 1:
(1 comment)
More SPI chips :)
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c File src/drivers/spi/macronix.c:
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c@200 PS1, Line 200: .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */
line over 80 characters
This line was already over 80 characters in the past, I didn't change anything for 0x5e16 chip
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c File src/drivers/spi/macronix.c:
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c@200 PS1, Line 200: .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */
This line was already over 80 characters in the past, I didn't change anything for 0x5e16 chip
Yeah, well make jenkins happy ...
Hello Kyösti Mälkki, Paul Menzel, Mike Banon, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30877
to look at the new patch set (#2).
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips
Required for ACPI S3 suspend support at some motherboards. Synchronizing with flashchips.c/h flashrom source code.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4508a65a5bdcbf58aadf452de5e896fc3c5b1bc3 --- M src/drivers/spi/macronix.c 1 file changed, 83 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/30877/2
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c File src/drivers/spi/macronix.c:
https://review.coreboot.org/#/c/30877/1/src/drivers/spi/macronix.c@200 PS1, Line 200: .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */
Yeah, well make jenkins happy ...
Okay, removed MX25L3235D from this comment because it's already there as .name
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30877 )
Change subject: drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips ......................................................................
drivers/spi/macronix.c: Add the rest of >=1MB Macronix MX25 chips
Required for ACPI S3 suspend support at some motherboards. Synchronizing with flashchips.c/h flashrom source code.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4508a65a5bdcbf58aadf452de5e896fc3c5b1bc3 Reviewed-on: https://review.coreboot.org/c/30877 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/spi/macronix.c 1 file changed, 83 insertions(+), 19 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 7ed70c0..d9f0044 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -80,22 +80,6 @@ .name = "MX25L3205D", }, { - .idcode = 0x5e16, - .page_size = 256, - .pages_per_sector = 16, - .sectors_per_block = 16, - .nr_blocks = 64, - .name = "MX25L3235D", /* MX25L3225D/MX25L3235D/MX25L3236D/MX25L3237D */ - }, - { - .idcode = 0x2536, - .page_size = 256, - .pages_per_sector = 16, - .sectors_per_block = 16, - .nr_blocks = 64, - .name = "MX25L3239E", - }, - { .idcode = 0x2017, .page_size = 256, .pages_per_sector = 16, @@ -112,12 +96,60 @@ .name = "MX25L12805D", }, { - .idcode = 0x2618, + .idcode = 0x2019, .page_size = 256, .pages_per_sector = 16, .sectors_per_block = 16, - .nr_blocks = 256, - .name = "MX25L12855E", + .nr_blocks = 512, + .name = "MX25L25635F", + }, + { + .idcode = 0x201a, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 1024, + .name = "MX66L51235F", + }, + { + .idcode = 0x2415, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25L1635D", + }, + { + .idcode = 0x2515, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25L1635E", + }, + { + .idcode = 0x2534, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 16, + .name = "MX25U8032E", + }, + { + .idcode = 0x2535, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 32, + .name = "MX25U1635E", + }, + { + .idcode = 0x2536, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 64, + .name = "MX25U3235E", }, { .idcode = 0x2537, @@ -136,6 +168,38 @@ .name = "MX25U12835F", }, { + .idcode = 0x2539, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 512, + .name = "MX25U25635F", + }, + { + .idcode = 0x253a, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 1024, + .name = "MX25U51245G", + }, + { + .idcode = 0x2618, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 256, + .name = "MX25L12855E", + }, + { + .idcode = 0x5e16, + .page_size = 256, + .pages_per_sector = 16, + .sectors_per_block = 16, + .nr_blocks = 64, + .name = "MX25L3235D", /* MX25L3225D/MX25L3236D/MX25L3237D */ + }, + { .idcode = 0x9517, .page_size = 256, .pages_per_sector = 16,