Attention is currently required from: Paul Menzel.
Nicholas Chin has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/82601?usp=email )
Change subject: util/autoport: Remove bd82x6x/pch.h from generated mainboard.c
......................................................................
Patch Set 2:
(1 comment)
File util/autoport/bd82x6x.go:
https://review.coreboot.org/c/coreboot/+/82601/comment/751d6f2b_ecbd24d7?us… :
PS1, Line 300: #include <northbridge/intel/sandybridge/raminit_native.h>
> CB:82405 also removes this
Resolved
--
To view, visit https://review.coreboot.org/c/coreboot/+/82601?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I904c95394b4fea73b4990342e647595b5f10335f
Gerrit-Change-Number: 82601
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sun, 23 Jun 2024 15:04:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Nicholas Chin, Paul Menzel.
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/82601?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Paul Menzel, Verified+1 by build bot (Jenkins)
Change subject: util/autoport: Remove bd82x6x/pch.h from generated mainboard.c
......................................................................
util/autoport: Remove bd82x6x/pch.h from generated mainboard.c
The southbridge/intel/bd82x6x/pch.h header was previously used to
configure a few registers in SPIBAR, but these have since been moved to
PCH code and the devicetree, making it unnecessary in mainboard.c
Change-Id: I904c95394b4fea73b4990342e647595b5f10335f
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M util/autoport/bd82x6x.go
1 file changed, 0 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/82601/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/82601?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I904c95394b4fea73b4990342e647595b5f10335f
Gerrit-Change-Number: 82601
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Felix Held, Nicholas Sudsgaard, Paul Menzel.
Felix Singer has posted comments on this change by Nicholas Sudsgaard. ( https://review.coreboot.org/c/coreboot/+/80332?usp=email )
Change subject: device/azalia: Separate codec checking and initialization
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80332?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I92b6d184abccdbe0e1bfce98a2c959a97a618a29
Gerrit-Change-Number: 80332
Gerrit-PatchSet: 10
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sun, 23 Jun 2024 13:04:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Angel Pons, Elyes Haouas, Felix Singer, Pratikkumar V Prajapati.
Máté Kukri has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83110?usp=email )
Change subject: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83110/comment/27e2c6ce_6b050f3a?us… :
PS2, Line 45: 0x00e0: 0x100c0009 (PCCTL)
I think it's entirely believable from the description that this actually works, and these values do not seem useful for someone looking through the commit history in the future.
File util/inteltool/lpc.c:
https://review.coreboot.org/c/coreboot/+/83110/comment/5da6e34b_d36774b8?us… :
PS1, Line 90: case PCI_DEVICE_ID_INTEL_H67:
> I am not sure if I understand you correctly. […]
I think adding just one device ID is even misleading, the LPC controller is identical on all SKUs of CPT, and this suggests that H67 is somehow special.
Maybe just add all CPT SKUs, and change the commit message to "inteltool/lpc: Add CPT device IDs"
https://review.coreboot.org/c/coreboot/+/83110/comment/e35646f5_fc3eea0a?us… :
PS1, Line 120: bc = pci_read_long(dev, SUNRISE_LPC_BC);
> It definitely is in scope of this patch. […]
I don't see how it is out of scope either:
- Before the code was correct because it was only reading registers on hardware where it exists.
- Adding this device ID makes the code incorrect by reading non-existent registers.
- I would think a commit exposing a code path to different hardware should also fix such problems.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83110?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I142393bd323418b8723d5bb364e919e1ece1e54d
Gerrit-Change-Number: 83110
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 23 Jun 2024 08:58:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Elyes Haouas, Felix Singer, Pratikkumar V Prajapati.
Angel Pons has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83110?usp=email )
Change subject: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67
......................................................................
Patch Set 2: Code-Review-1
(2 comments)
File util/inteltool/lpc.c:
https://review.coreboot.org/c/coreboot/+/83110/comment/9c24ab8b_4fbef1dc?us… :
PS1, Line 90: case PCI_DEVICE_ID_INTEL_H67:
> looks better to me
I am not sure if I understand you correctly. Are you saying that **NOT** adding some more `case PCI_DEVICE_ID_INTEL_*:` lines for CPT (Cougar Point, aka 6 series PCH) "looks better to you"?
https://review.coreboot.org/c/coreboot/+/83110/comment/04d758a8_6ad755be?us… :
PS1, Line 120: bc = pci_read_long(dev, SUNRISE_LPC_BC);
> Out of scope of current patch
It definitely is in scope of this patch. The bit you are checking has a different meaning in CPT (Cougar Point, aka 6 series PCH): https://i.imgur.com/joVk40u.png
This is not the only difference. Here is the full datasheet for your hardware: https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/6-c…
--
To view, visit https://review.coreboot.org/c/coreboot/+/83110?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I142393bd323418b8723d5bb364e919e1ece1e54d
Gerrit-Change-Number: 83110
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 23 Jun 2024 08:40:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Angel Pons, Felix Singer, Pratikkumar V Prajapati.
Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83110?usp=email )
Change subject: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83110/comment/c8e1a291_6a609b94?us… :
PS1, Line 9: bh67bl
> Did you mean `DH67BL`?
Yep :)
Thx
--
To view, visit https://review.coreboot.org/c/coreboot/+/83110?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I142393bd323418b8723d5bb364e919e1ece1e54d
Gerrit-Change-Number: 83110
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Sun, 23 Jun 2024 08:25:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Angel Pons, Felix Singer, Pratikkumar V Prajapati.
Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83110?usp=email )
Change subject: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83110/comment/c1228db9_bfd1e0e5?us… :
PS1, Line 11: ./inteltool -l
> Do we need the inteltool output in the commit message?
it doesn't hurt :)
File util/inteltool/lpc.c:
https://review.coreboot.org/c/coreboot/+/83110/comment/bae1e31b_514e0d01?us… :
PS1, Line 90: case PCI_DEVICE_ID_INTEL_H67:
> Why not add at least the rest of CPT IDs?
looks better to me
https://review.coreboot.org/c/coreboot/+/83110/comment/728d76c9_548ddf2c?us… :
PS1, Line 120: bc = pci_read_long(dev, SUNRISE_LPC_BC);
> Does this actually exist on CPT? I would prefer to have a special case for older platforms that only […]
Out of scope of current patch
--
To view, visit https://review.coreboot.org/c/coreboot/+/83110?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I142393bd323418b8723d5bb364e919e1ece1e54d
Gerrit-Change-Number: 83110
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Sun, 23 Jun 2024 08:24:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Elyes Haouas, Pratikkumar V Prajapati.
Hello Pratikkumar V Prajapati, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83027?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: [test] Add DH67BL mainboard
......................................................................
[test] Add DH67BL mainboard
based on this log https://fileport.io/TdudfPex37MY
Change-Id: If466e834aae0a980941fadb18635e90280119088
Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr>
---
A src/mainboard/intel/dh67bl/Kconfig
A src/mainboard/intel/dh67bl/Kconfig.name
A src/mainboard/intel/dh67bl/Makefile.mk
A src/mainboard/intel/dh67bl/acpi/ec.asl
A src/mainboard/intel/dh67bl/acpi/platform.asl
A src/mainboard/intel/dh67bl/acpi/superio.asl
A src/mainboard/intel/dh67bl/acpi_tables.c
A src/mainboard/intel/dh67bl/board_info.txt
A src/mainboard/intel/dh67bl/devicetree.cb
A src/mainboard/intel/dh67bl/dsdt.asl
A src/mainboard/intel/dh67bl/early_init.c
A src/mainboard/intel/dh67bl/gma-mainboard.ads
A src/mainboard/intel/dh67bl/gpio.c
A src/mainboard/intel/dh67bl/hda_verb.c
A src/mainboard/intel/dh67bl/mainboard.c
M util/inteltool/lpc.c
16 files changed, 489 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/83027/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/83027?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If466e834aae0a980941fadb18635e90280119088
Gerrit-Change-Number: 83027
Gerrit-PatchSet: 7
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>