Attention is currently required from: Sean Rhodes.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 10:
(1 comment)
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-120635):
https://review.coreboot.org/c/coreboot/+/55128/comment/ef0e6844_4bb729f4
PS10, Line 115: const uint8_t ht = get_uint_option("hyper_threading", memupd->FspmConfig.HyperThreading);
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 10
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 15:06:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 9: Code-Review+1
(7 comments)
Patchset:
PS9:
Few more nits, mostly LGTM
File Documentation/distributions.md:
https://review.coreboot.org/c/coreboot/+/55128/comment/75c7b640_a8a12305
PS6, Line 11: ### Purism
:
: [Purism](https://www.puri.sm) sells laptops with a focus on user privacy and
: security; part of that effort is to minimize the amount of proprietary and/or
: binary code. Their laptops ship with a blob-free OS and coreboot firmware
: with a neutralized Intel Management Engine (ME) and SeaBIOS as the payload.
> I was bundling all the documentation into one, seemed to miss the relevant 'Documentation/mainboard/ […]
Ack
File Documentation/mainboard/starlabs/labtop.md:
https://review.coreboot.org/c/coreboot/+/55128/comment/5036d716_739f5921
PS9, Line 120: higher than **1.5.6** will work.
: ![fwupd version](https://cdn.shopify.com/s/files/1/2059/5897/files/fwupdV.png?v=161…
: On Ubuntu 20.04, Ubuntu 20.10, Linux Mint 20.1 and elementaryOS 6, fwupd 1.5.6 can be installed from our PPA with the below terminal commands:
:
sounds like you mean 1.5.6 and greater will work?
File src/mainboard/starlabs/labtop/Kconfig:
https://review.coreboot.org/c/coreboot/+/55128/comment/834a527b_5709426f
PS6, Line 26: # select HAVE_IFD_BIN
: # select HAVE_ME_BIN
> ME and Descriptor aren't in the blobs repository, which causes the build to fail. […]
Ack
File src/mainboard/starlabs/labtop/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/55128/comment/31540f01_8ace18fe
PS9, Line 55:
Sorry I mean there's a tab between #define and EC_GPE_SWI, should be a space
File src/mainboard/starlabs/labtop/spd/micron-MT40A1G16KD-062E-E.spd.hex:
PS6:
> No, just as the hex files already existed in the tree. […]
This is fine; the spd_tools are really neat, though, if you have access to the datasheet for the memory part; it can generate an SPD file for some Intel & AMD chipsets automatically. I think it would need some porting work for CML or KBL, though.
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
https://review.coreboot.org/c/coreboot/+/55128/comment/96dc6be9_e799eb78
PS6, Line 41: (gpio_get(GPP_H6) < 2) | (gpio_get(GPP_E23) < 1) | gpio_get(GPP_E22);
> suggestion: use `gpio_base2_value` function, e.g.: […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 9
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 15:03:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Karthik Ramasubramanian.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55149 )
Change subject: mb/google/guybrush: Override I2C3 pad configuration
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/guybrush/i2c.c:
https://review.coreboot.org/c/coreboot/+/55149/comment/36d6fc17_1cc42a1e
PS1, Line 8: mainboard_i2c_override
> What do you think about moving this to a devicetree config? That will eliminate the need for the cal […]
+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/55149
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I595a64736fdac0274abffb68c5e521302275b845
Gerrit-Change-Number: 55149
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 03 Jun 2021 14:47:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55169 )
Change subject: mb/lenovo/t410: Enable WWAN and WUSB PCIe ports
......................................................................
mb/lenovo/t410: Enable WWAN and WUSB PCIe ports
These PCH PCIe ports are used and should be enabled.
Resolves: https://ticket.coreboot.org/issues/311
Change-Id: I26ace6e043c7c66f8944f0986923014703423b8c
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/mainboard/lenovo/t410/devicetree.cb
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/55169/1
diff --git a/src/mainboard/lenovo/t410/devicetree.cb b/src/mainboard/lenovo/t410/devicetree.cb
index 5f167df..a0c9a64 100644
--- a/src/mainboard/lenovo/t410/devicetree.cb
+++ b/src/mainboard/lenovo/t410/devicetree.cb
@@ -72,8 +72,8 @@
end
device pci 1c.0 on end # PCIe Port #1 (wlan)
- device pci 1c.1 off end # PCIe Port #2 (wwan)
- device pci 1c.2 off end # PCIe Port #3 (wusb)
+ device pci 1c.1 on end # PCIe Port #2 (wwan)
+ device pci 1c.2 on end # PCIe Port #3 (wusb)
device pci 1c.3 on end # PCIe Port #4 (ExpressCard)
device pci 1c.4 on
subsystemid 0x17aa 0x2133
--
To view, visit https://review.coreboot.org/c/coreboot/+/55169
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26ace6e043c7c66f8944f0986923014703423b8c
Gerrit-Change-Number: 55169
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Zheng Bao.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55160 )
Change subject: [WIP]amdfwtool: Add a script to replace AMD firmware
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55160/comment/294beb99_22bf76dd
PS2, Line 7: [WIP]amdfwtool: Add a script to replace AMD firmware
b:183447172
--
To view, visit https://review.coreboot.org/c/coreboot/+/55160
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib450277d55ad254234a451e06747121fff731ee0
Gerrit-Change-Number: 55160
Gerrit-PatchSet: 2
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Zheng Bao
Gerrit-Comment-Date: Thu, 03 Jun 2021 14:26:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber.
Patrick Georgi has removed Martin Roth from this change. ( https://review.coreboot.org/c/coreboot/+/55168 )
Change subject: Ada: Enable the newest Ada standard + extensions supported by gnat
......................................................................
Removed reviewer Martin Roth.
--
To view, visit https://review.coreboot.org/c/coreboot/+/55168
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If2f627bdf7f29ffdd4433b4ca608b2291ebe1551
Gerrit-Change-Number: 55168
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: deleteReviewer
Attention is currently required from: Nico Huber.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/55166 )
Change subject: Adapt to gcc11 compatible libhwbase API
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Quite a messy situation although I kinda prefer the new generic signature of MMIO_Range, given that the array type was fully defined by the other types already.
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/55166
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: I0054920b27d4c6ee75f2caa9a6c9b5005a694045
Gerrit-Change-Number: 55166
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 03 Jun 2021 13:52:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55168 )
Change subject: Ada: Enable the newest Ada standard + extensions supported by gnat
......................................................................
Ada: Enable the newest Ada standard + extensions supported by gnat
We're a bit in a bind here: The only way to declare an array containing
atomic or volatile members is to mark the member type atomic or
volatile as well.
The only way to mark a member type atomic or volatile seems to be to
use Ada202x.
Change-Id: If2f627bdf7f29ffdd4433b4ca608b2291ebe1551
Signed-off-by: Patrick Georgi <pgeorgi(a)google.com>
---
M Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/55168/1
diff --git a/Makefile.inc b/Makefile.inc
index eb505e5..fce53fa 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -435,7 +435,7 @@
endif
endif
-ADAFLAGS_common += -gnatp
+ADAFLAGS_common += -gnatp -gnatX
ADAFLAGS_common += -Wuninitialized -Wall -Werror
ADAFLAGS_common += -pipe -g -nostdinc
ADAFLAGS_common += -Wstrict-aliasing -Wshadow
--
To view, visit https://review.coreboot.org/c/coreboot/+/55168
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If2f627bdf7f29ffdd4433b4ca608b2291ebe1551
Gerrit-Change-Number: 55168
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange
Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/libhwbase/+/55167 )
Change subject: Make gcc11 compatible
......................................................................
Make gcc11 compatible
It's much more picky when dealing with arrays containing volatile or
atomic members.
Meanwhile System.Address_To_Access_Conversions doesn't seem to be
able to return an element with atomic/volatile aspect, so everything
fell apart.
Entire untested except that it builds (for me)
Change-Id: Ieb50c4dd8ba96248c3051a7282f9e5cdbb270344
Signed-off-by: Patrick Georgi <pgeorgi(a)google.com>
---
M ada/dynamic_mmio/hw-mmio_range.adb
M common/hw-mmio_range.ads
M common/hw-pci-dev.ads
M common/hw-pci-mmconf.adb
M common/hw-pci-mmconf.ads
5 files changed, 30 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/libhwbase refs/changes/67/55167/1
diff --git a/ada/dynamic_mmio/hw-mmio_range.adb b/ada/dynamic_mmio/hw-mmio_range.adb
index 86c8d1b..60cbbd5 100644
--- a/ada/dynamic_mmio/hw-mmio_range.adb
+++ b/ada/dynamic_mmio/hw-mmio_range.adb
@@ -14,14 +14,12 @@
with HW.Debug;
with GNAT.Source_Info;
-with System.Storage_Elements;
-with System.Address_To_Access_Conversions;
package body HW.MMIO_Range
with
Refined_State =>
- (State => null, -- the contents accessed, Range_A points to it
- Base_Address => Range_A) -- the address, stored in Range_A
+ (State => null, -- the contents accessed, Base points to it
+ Base_Address => (Base, Range_A)) -- the address, stored in Base
is
pragma Warnings (Off, "implicit dereference",
Reason => "This is what this package is about.");
@@ -29,12 +27,6 @@
Debug_Reads : constant Boolean := False;
Debug_Writes : constant Boolean := False;
- type Range_Access is access all Array_T;
- package Conv_Range is new System.Address_To_Access_Conversions (Array_T);
-
- Range_A : Range_Access :=
- Range_Access (Conv_Range.To_Pointer (System'To_Address (Base_Addr)));
-
procedure Read (Value : out Element_T; Index : in Index_T)
is
use type Word32;
@@ -45,9 +37,8 @@
pragma Debug (Debug_Reads, Debug.Put_Word32 (Word32 (Value)));
pragma Debug (Debug_Reads, Debug.Put (" <- "));
pragma Debug (Debug_Reads, Debug.Put_Word32
- (Word32 (System.Storage_Elements.To_Integer
- (Conv_Range.To_Address (Conv_Range.Object_Pointer (Range_A)))) +
- Word32 (Index) * (Element_T'Size / 8)));
+ (Word32 (System.Storage_Elements.To_Integer (Base))
+ + Word32 (Index) * (Element_T'Size / 8)));
pragma Debug (Debug_Reads, Debug.New_Line);
end Read;
@@ -60,17 +51,17 @@
pragma Debug (Debug_Writes, Debug.Put_Word32 (Word32 (Value)));
pragma Debug (Debug_Writes, Debug.Put (" -> "));
pragma Debug (Debug_Writes, Debug.Put_Word32
- (Word32 (System.Storage_Elements.To_Integer
- (Conv_Range.To_Address (Conv_Range.Object_Pointer (Range_A)))) +
- Word32 (Index) * (Element_T'Size / 8)));
+ (Word32 (System.Storage_Elements.To_Integer (Base))
+ + Word32 (Index) * (Element_T'Size / 8)));
pragma Debug (Debug_Writes, Debug.New_Line);
Range_A (Index) := Value;
end Write;
- procedure Set_Base_Address (Base : Word64) is
+ procedure Set_Base_Address (New_Base : Word64) is
+ Address : System.Storage_Elements.Integer_Address;
begin
- Range_A := Range_Access
- (Conv_Range.To_Pointer (System'To_Address (Base)));
+ Address := System.Storage_Elements.Integer_Address (New_Base);
+ Base := System.Storage_Elements.To_Address(Address);
end Set_Base_Address;
end HW.MMIO_Range;
diff --git a/common/hw-mmio_range.ads b/common/hw-mmio_range.ads
index 8495f73..86ddcd6 100644
--- a/common/hw-mmio_range.ads
+++ b/common/hw-mmio_range.ads
@@ -13,17 +13,17 @@
--
with System;
+with System.Storage_Elements;
generic
Base_Addr : Word64;
type Element_T is mod <>;
type Index_T is range <>;
- type Array_T is array (Index_T) of Element_T;
package HW.MMIO_Range
with
Abstract_State =>
((State with External),
- Base_Address),
+ (Base_Address with External)),
Initializes => Base_Address
is
@@ -31,6 +31,18 @@
procedure Write (Index : in Index_T; Value : in Element_T);
- procedure Set_Base_Address (Base : Word64);
+ procedure Set_Base_Address (New_Base : Word64);
+
+private
+
+ Base : System.Address := System.Storage_Elements.To_Address
+ (System.Storage_Elements.Integer_Address (Base_Addr))
+ with Part_Of => Base_Address;
+
+ type Volatile_Element_T is new Element_T with Volatile;
+ type Array_T is array (Index_T) of Element_T with Volatile_Components;
+
+ Range_A : Array_T with Part_Of => Base_Address;
+ for Range_A'Address use Base;
end HW.MMIO_Range;
diff --git a/common/hw-pci-dev.ads b/common/hw-pci-dev.ads
index 5b20aa1..6bfef74 100644
--- a/common/hw-pci-dev.ads
+++ b/common/hw-pci-dev.ads
@@ -16,7 +16,7 @@
Dev : PCI.Address := (0, 0, 0);
package HW.PCI.Dev
with
- Abstract_State => (Address_State, (PCI_State with External)),
+ Abstract_State => ((Address_State with External), (PCI_State with External)),
Initializes => Address_State
is
diff --git a/common/hw-pci-mmconf.adb b/common/hw-pci-mmconf.adb
index 180f2d7..493780d 100644
--- a/common/hw-pci-mmconf.adb
+++ b/common/hw-pci-mmconf.adb
@@ -29,16 +29,12 @@
type Index16 is new Index range 0 .. Index'Last / 2;
type Index32 is new Index range 0 .. Index'Last / 4;
- type Array8 is array (Index) of Byte with Atomic_Components;
- type Array16 is array (Index16) of Word16 with Atomic_Components;
- type Array32 is array (Index32) of Word32 with Atomic_Components;
-
package MM8 is new HW.MMIO_Range
- (Default_Base_Address, Word8, Index, Array8);
+ (Default_Base_Address, Word8, Index);
package MM16 is new HW.MMIO_Range
- (Default_Base_Address, Word16, Index16, Array16);
+ (Default_Base_Address, Word16, Index16);
package MM32 is new HW.MMIO_Range
- (Default_Base_Address, Word32, Index32, Array32);
+ (Default_Base_Address, Word32, Index32);
procedure Read8 (Value : out Word8; Offset : Index) renames MM8.Read;
diff --git a/common/hw-pci-mmconf.ads b/common/hw-pci-mmconf.ads
index 597ebfa..22e2e2e 100644
--- a/common/hw-pci-mmconf.ads
+++ b/common/hw-pci-mmconf.ads
@@ -21,7 +21,7 @@
Dev : Address := (0, 0, 0);
package HW.PCI.MMConf
with
- Abstract_State => (Address_State, (PCI_State with External)),
+ Abstract_State => ((Address_State with External), (PCI_State with External)),
Initializes => Address_State
is
--
To view, visit https://review.coreboot.org/c/libhwbase/+/55167
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libhwbase
Gerrit-Branch: master
Gerrit-Change-Id: Ieb50c4dd8ba96248c3051a7282f9e5cdbb270344
Gerrit-Change-Number: 55167
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Rex-BC Chen, Yu-Ping Wu.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55163 )
Change subject: soc/mediatek/mt8195: fix GPIO register offset
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55163/comment/b5a4c2fc_bedc046f
PS1, Line 9: Fix GPIO pu/pd offset.
Do you have a reference for these offsets so we can know it's now correct?
Maybe which chapter in the data sheet / application doc ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/55163
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9b0f8a24756092a97933cc9d4ba13a9e79c73e91
Gerrit-Change-Number: 55163
Gerrit-PatchSet: 1
Gerrit-Owner: Rex-BC Chen <rex-bc.chen(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Rex-BC Chen <rex-bc.chen(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 03 Jun 2021 13:46:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment