Attention is currently required from: Michał Kopeć.
Maciej Pijanowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80609?usp=email )
Change subject: mb/lenovo/m920q: add board
......................................................................
Patch Set 3:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80609?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iea1dc5745c0ecf687fa18b793f0aab4b0855d6d4
Gerrit-Change-Number: 80609
Gerrit-PatchSet: 3
Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Comment-Date: Sat, 17 Feb 2024 12:56:10 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80608?usp=email
to look at the new patch set (#2).
Change subject: soc/intel/common/block/fast_spi: probe for 2nd flash component
......................................................................
soc/intel/common/block/fast_spi: probe for 2nd flash component
Fast SPI code assumes only one SPI flash is present. The SPI flash
driver for older southbridges used to be able to detect multichip.
Some boards still come with two chips populated instead of one.
With this change, both chips are probed, and the correct total
size is calculated. Otherwise, only the first one was probed,
which resulted in an error such as:
SF size 0x1000000 does not correspond to CONFIG_ROM_SIZE 0x1800000!!
Change-Id: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Signed-off-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
---
M src/soc/intel/common/block/fast_spi/fast_spi_def.h
M src/soc/intel/common/block/fast_spi/fast_spi_flash.c
2 files changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/80608/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80608?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c
Gerrit-Change-Number: 80608
Gerrit-PatchSet: 2
Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Michał Żygowski.
Hello Michał Żygowski,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/80608?usp=email
to review the following change.
Change subject: soc/intel/common/block/fast_spi: probe for 2nd flash component
......................................................................
soc/intel/common/block/fast_spi: probe for 2nd flash component
Fast SPI code assumes only one SPI flash is present. The SPI flash
driver for older southbridges used to be able to detect multichip.
Some boards still come with two chips populated instead of one.
With this change, both chips are probed, and the correct total
size is calculated. Otherwise, only the first one was probed,
which resulted in an error such as:
SF size 0x1000000 does not correspond to CONFIG_ROM_SIZE 0x1800000!!
Change-Id: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Signed-off-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
---
M src/soc/intel/common/block/fast_spi/fast_spi_def.h
M src/soc/intel/common/block/fast_spi/fast_spi_flash.c
2 files changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/80608/1
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_def.h b/src/soc/intel/common/block/fast_spi/fast_spi_def.h
index cc1cdaa..72c359e 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi_def.h
+++ b/src/soc/intel/common/block/fast_spi/fast_spi_def.h
@@ -3,6 +3,11 @@
#ifndef SOC_INTEL_COMMON_BLOCK_FAST_SPI_DEF_H
#define SOC_INTEL_COMMON_BLOCK_FAST_SPI_DEF_H
+/* From JEDEC SFDP JESD216F.02 */
+#define SFDP_HDR_SIG 0x00 /* 1st DWORD of SFDP header */
+#define SFDP_PARAM_DENSITY 0x04 /* 2nd DWORD of SFDP params */
+#define SFDP_SIGNATURE 0x50444653 /* Valid sig in 1st DWORD of SFDP header */
+
/* PCI configuration registers */
#define SPI_BIOS_DECODE_EN 0xd8
#define SPI_BIOS_DECODE_LOCK BIT(31)
@@ -153,7 +158,6 @@
#define SPIBAR_PTINX_HORD_SFDP (0 << 12)
#define SPIBAR_PTINX_HORD_PARAM (1 << 12)
#define SPIBAR_PTINX_HORD_JEDEC (2 << 12)
-#define SPIBAR_PTINX_IDX_MASK 0xffc
/* Register Offsets of BIOS Flash Program Registers */
#define SPIBAR_RESET_LOCK 0xf0
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
index f896cd9..0115ea4 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
+++ b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
@@ -54,22 +54,21 @@
}
/*
- * The hardware datasheet is not clear on what HORD values actually do. It
- * seems that HORD_SFDP provides access to the first 8 bytes of the SFDP, which
- * is the signature and revision fields. HORD_JEDEC provides access to the
- * actual flash parameters, and is most likely what you want to use when
- * probing the flash from software.
+ * Via component we can select either 1st or 2nd flash (in dual flash setups).
+ * Via hord we can select either:
+ * - SFDP Header
+ * - Param Table Header
+ * - Data (JEDEC params, including density)
+ *
* It's okay to rely on SFDP, since the SPI flash controller requires an SFDP
* 1.5 or newer compliant FAST_SPI flash chip.
* NOTE: Due to the register layout of the hardware, all accesses will be
* aligned to a 4 byte boundary.
*/
-static uint32_t fast_spi_flash_read_sfdp_param(struct fast_spi_flash_ctx *ctx,
- uint16_t sfdp_reg)
+static uint32_t fast_spi_flash_read_sfdp(struct fast_spi_flash_ctx *ctx,
+ uint32_t ptinx_reg)
{
- uint32_t ptinx_index = sfdp_reg & SPIBAR_PTINX_IDX_MASK;
- fast_spi_flash_ctrlr_reg_write(ctx, SPIBAR_PTINX,
- ptinx_index | SPIBAR_PTINX_HORD_JEDEC);
+ fast_spi_flash_ctrlr_reg_write(ctx, SPIBAR_PTINX, ptinx_reg);
return fast_spi_flash_ctrlr_reg_read(ctx, SPIBAR_PTDATA);
}
@@ -312,15 +311,30 @@
{
BOILERPLATE_CREATE_CTX(ctx);
uint32_t flash_bits;
+ uint32_t ptinx_reg;
/*
* bytes = (bits + 1) / 8;
* But we need to do the addition in a way which doesn't overflow for
* 4 Gbit devices (flash_bits == 0xffffffff).
*/
- flash_bits = fast_spi_flash_read_sfdp_param(ctx, 0x04);
+ ptinx_reg = SPIBAR_PTINX_COMP_0 | SPIBAR_PTINX_HORD_JEDEC | SFDP_PARAM_DENSITY;
+ flash_bits = fast_spi_flash_read_sfdp(ctx, ptinx_reg);
flash->size = (flash_bits >> 3) + 1;
+ /*
+ * Now check if we have a second flash component.
+ * Check SFDP header for the SFDP signature. If valid, then 2nd component is present.
+ * Increase the flash size if 2nd component is found, analogically like the 1st
+ * component.
+ */
+ ptinx_reg = SPIBAR_PTINX_COMP_1 | SPIBAR_PTINX_HORD_SFDP | SFDP_HDR_SIG;
+ if (fast_spi_flash_read_sfdp(ctx, ptinx_reg == SFDP_SIGNATURE)) {
+ ptinx_reg = SPIBAR_PTINX_COMP_1 | SPIBAR_PTINX_HORD_JEDEC | SFDP_PARAM_DENSITY;
+ flash_bits = fast_spi_flash_read_sfdp(ctx, ptinx_reg);
+ flash->size += ((flash_bits >> 3) + 1);
+ }
+
memcpy(&flash->spi, dev, sizeof(*dev));
/* Can erase both 4 KiB and 64 KiB chunks. Declare the smaller size. */
--
To view, visit https://review.coreboot.org/c/coreboot/+/80608?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c
Gerrit-Change-Number: 80608
Gerrit-PatchSet: 1
Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jonathon Hall, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80602?usp=email )
Change subject: mb/purism/librem_cnl: Drop selection of USE_LEGACY_8254_TIMER
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80602?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I974a82bedc4e06f48ce801f2bc0c29afbd80ffcf
Gerrit-Change-Number: 80602
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sat, 17 Feb 2024 12:10:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jonathon Hall, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80601?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/purism/librem_cnl/var/librem_mini: Set RTC register defaults
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/purism/librem_cnl/variants/librem_mini/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/80601/comment/91a92e5f_4c0a5c47 :
PS2, Line 154: io 0x60 = 0x070
Is this forwarded to LPC? If not, would be worth a comment, so nobody
gets confused. Or maybe even set it to 0.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80601?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I310a834a1fa7b8428879fbc160e4aae0a519e063
Gerrit-Change-Number: 80601
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sat, 17 Feb 2024 12:09:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jonathon Hall, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80600?usp=email )
Change subject: mb/purism_librem_cnl/var/*: Drop redundant entries in overridetrees
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80600?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I12498e7261dafd7ee59fe79926532399392d1b09
Gerrit-Change-Number: 80600
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sat, 17 Feb 2024 12:04:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jonathon Hall, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80599?usp=email )
Change subject: mb/purism/librem_cnl: Drop devicetree entries identical to chipset.cb
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80599?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6c656d227962548cebde61f1d82333837adbbf56
Gerrit-Change-Number: 80599
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sat, 17 Feb 2024 12:01:10 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas, Martin L Roth, Nico Huber.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80607?usp=email )
Change subject: [test only] upgrade to gcc-14-20240211
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Nico, could you look at the issues from the toolchain builder? There are some Ada related issues.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80607?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib96709309e7c71b76a51d3b8600be793312d6a8f
Gerrit-Change-Number: 80607
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sat, 17 Feb 2024 11:57:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80598?usp=email )
Change subject: device/pnp_device: Skip init on disabled functions
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80598/comment/ae3dc7f6_0090738c :
PS1, Line 11: any resources to that function.
The code disabled here merely attaches the structures for the
resources. Resource assignment would happen later and should
happen only if an LDN is enabled.
https://review.coreboot.org/c/coreboot/+/80598/comment/c07073d9_c41be90a :
PS1, Line 15: in the log / don't cause any errors.
Can you share a log? errors are often because of mistakes in the
devicetree.
File src/device/pnp_device.c:
https://review.coreboot.org/c/coreboot/+/80598/comment/4a3e3e3e_0b2832a3 :
PS1, Line 403: dev->ops = ops;
Not setting the ops has certain side effects. For instance one couldn't generate
SSDT entries for disabled devices. So skipping this seems wrong. Do I miss something?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80598?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5c9d5cf34947535bf249b91f78d6fabfb28729ea
Gerrit-Change-Number: 80598
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Sat, 17 Feb 2024 11:44:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment