Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56020 )
Change subject: sb/intel/i82801gx: Prepare for x86_64
......................................................................
Patch Set 3:
(1 comment)
File src/southbridge/intel/i82801gx/azalia.c:
https://review.coreboot.org/c/coreboot/+/56020/comment/5bf3ee70_ef634306
PS3, Line 196: u32
> Actually it's `unsigned int` just happens to match for both x86 and x86_64.
We could simply drop the casts use %p
--
To view, visit https://review.coreboot.org/c/coreboot/+/56020
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idebfe4669854b307bee653df6d93e46ae3f39dec
Gerrit-Change-Number: 56020
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 02 Jul 2021 19:26:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Patrick Rudolph.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56020 )
Change subject: sb/intel/i82801gx: Prepare for x86_64
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/southbridge/intel/i82801gx/azalia.c:
https://review.coreboot.org/c/coreboot/+/56020/comment/1491cdf6_d401033c
PS3, Line 196: u32
Actually it's `unsigned int` just happens to match for both x86 and x86_64.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56020
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idebfe4669854b307bee653df6d93e46ae3f39dec
Gerrit-Change-Number: 56020
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 02 Jul 2021 19:04:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Patrick Rudolph.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56021 )
Change subject: cpu/intel/car/p4: Add x86_64 support
......................................................................
Patch Set 4: Code-Review+2
(2 comments)
File src/cpu/intel/car/p4-netburst/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/56021/comment/8372a14e_a9afeffe
PS4, Line 364: #include <cpu/x86/64bit/entry64.inc>
Oddly named, I had to think twice to understand what to expect
in there. It's more `enter64`. `entry16.inc` and `entry32.inc`
both define the entry point after switching the mode.
https://review.coreboot.org/c/coreboot/+/56021/comment/bbcb0d70_2e263b8f
PS4, Line 365:
Please add a similar comment as below.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56021
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I77516e3cd5f0d3b7442be660c005a65b00454343
Gerrit-Change-Number: 56021
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 02 Jul 2021 19:02:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Matt DeVillier, Jeremy Soller, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Karthik Ramasubramanian.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56008 )
Change subject: cannonlake mainboards: Set PMC as hidden in devicetree
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
Patchset:
PS2:
> Not true, I wrote the code that switched the meaning of hidden (it was unused at the time in any dev […]
There's something about hidden bridges. Actually, devices directly below
them don't get enumerated. But we still descend down the devices mentioned
in the devicetree.cb via scan_static_bus().
PS2:
I also seem to remember hat the PMC is hidden early on these
platforms. And our ports all set it to `hidden`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56008
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4a20ce9075ce7653388a5d3e281fe774bf89355
Gerrit-Change-Number: 56008
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Fri, 02 Jul 2021 18:46:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Paul Menzel.
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55945 )
Change subject: drivers/intel/usb4/retimer: Update code to assign correct port number
......................................................................
Patch Set 9:
(4 comments)
File src/drivers/intel/usb4/retimer/chip.h:
https://review.coreboot.org/c/coreboot/+/55945/comment/2e16e88a_a21fb61e
PS3, Line 17: /* _PLD setting */
: struct acpi_pld_group group;
> This isn't necessary, it can be retrieved from the typec_port as well, see below
Done
https://review.coreboot.org/c/coreboot/+/55945/comment/4cacca09_5d1ebb00
PS3, Line 17: /* _PLD setting */
: struct acpi_pld_group group;
> This isn't required anymore since it can be derived from the typec_port device?
Ack
File src/drivers/intel/usb4/retimer/retimer.c:
https://review.coreboot.org/c/coreboot/+/55945/comment/f35f48fc_66b7e166
PS3, Line 364: usb_port = config->dfp[dfp_port].typec_port->path.usb.port_id;
> should probably ensure that the device has the correct type first, e.g.: […]
Done
https://review.coreboot.org/c/coreboot/+/55945/comment/47c03201_e1505620
PS3, Line 373: acpi_pld_fill_usb(&pld, UPC_TYPE_PROPRIETARY, &config->dfp[dfp_port].group);
: pld.shape = PLD_SHAPE_OVAL;
: pld.visible = 1;
: acpigen_write_pld(&pld);
> This can also be retrieved from the `typec_port` if you do the following: […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/55945
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib11637ae39046e0afdacd33bc34e8a59e6f2bfb1
Gerrit-Change-Number: 55945
Gerrit-PatchSet: 9
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Abhijeet Rao <abhijeet.rao(a)intel.corp-partner.google.com>
Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 02 Jul 2021 17:55:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Hello build bot (Jenkins), Furquan Shaikh, John Zhao, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/55945
to look at the new patch set (#9).
Change subject: drivers/intel/usb4/retimer: Update code to assign correct port number
......................................................................
drivers/intel/usb4/retimer: Update code to assign correct port number
Since TBT controller can have maximum 2 ports per controller, our
code will loop over DFP structure twice and determine port number.
Retimer driver used to assign port number as below:
1. Check if power GPIO is assigned for particular DFP entry or not
2. If entry is there, assign loop count as port number
Since loop count is 2, retimer will never assign port number = 2
even if it's present. In case of more than 1 controller, port number
assigned will still be 0 or 1 even though actual port index might
be 2 or 3. This will create an issue where even if you do transaction
on device on controller 2 (port index 2 or 3), EC will route it on
port 0 or 1 due to incorrect port index.
Update the driver flow as per below to handle this scenario:
1. Check if power GPIO is assigned for particular DFP entry or not
2. Get USB port number from config since it's stored in usb port
information under devicetree
3. Pass the port number to ACPI SSDT and EC code
Above changes will ensure that we're assigning correct port
number as per calculation and EC will use correct port index.
BUG=b:189476816
BRANCH=None
TEST=Checked that retimer firmware update works on both ports and update
happens on correct port index.
Change-Id: Ib11637ae39046e0afdacd33bc34e8a59e6f2bfb1
Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
---
M src/drivers/intel/usb4/retimer/chip.h
M src/drivers/intel/usb4/retimer/retimer.c
2 files changed, 25 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/55945/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/55945
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib11637ae39046e0afdacd33bc34e8a59e6f2bfb1
Gerrit-Change-Number: 55945
Gerrit-PatchSet: 9
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Abhijeet Rao <abhijeet.rao(a)intel.corp-partner.google.com>
Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak.
Hello build bot (Jenkins), Furquan Shaikh, John Zhao, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/55946
to look at the new patch set (#8).
Change subject: mb/google/brya,primus,voxel: Update controller field for tbt_dma entries
......................................................................
mb/google/brya,primus,voxel: Update controller field for tbt_dma entries
We need to refernce correct USB port number for driver to
identify type-C port number correctly.
BUG=b:189476816
BRANCH=None
TEST=Check the transactions are happening on correct port. Also checked
retimer firmware update on both the ports.
Change-Id: I20c088ee81610155067abad086eba8d72f73ad60
Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
---
M src/mainboard/google/brya/variants/brya0/overridetree.cb
M src/mainboard/google/brya/variants/primus/overridetree.cb
M src/mainboard/google/volteer/variants/voxel/overridetree.cb
3 files changed, 12 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/55946/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/55946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I20c088ee81610155067abad086eba8d72f73ad60
Gerrit-Change-Number: 55946
Gerrit-PatchSet: 8
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Abhijeet Rao <abhijeet.rao(a)intel.corp-partner.google.com>
Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset