Attention is currently required from: Arthur Heymans, Haribalaraman Ramasubramanian, Kapil Porwal, Nick Vaccaro, Shelley Chen, Subrata Banik.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80504?usp=email )
Change subject: soc/intel/alderlake: Remove CNVi assertions
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80504/comment/403791d1_2b1d537e :
PS4, Line 12: This seems to make sense and allows us
: to enable CNVi bluetooth when necessary.
> > Intel uploaded another option that you may find more acceptable. See CB:80564. […]
Thanks. Shelley, Can you please abandon this change then?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80504?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: I822a4e360fde100b8289cacf10a01f6d97facbb4
Gerrit-Change-Number: 80504
Gerrit-PatchSet: 4
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:31:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74370?usp=email )
Change subject: soc/intel/meteorlake: Replace assert with error message
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> > asserts are not failing boot by default and are more appropriate than error messages. Even when asserts are halting boot, they pinpoint you to an issue. With error messages you can have many other ones and debugging is harder.
>
> the point here is that, we don't want to boot halt the device (like what asset does) as any such UPD misalignment is not a boot critical failure.
The default behavior for assert is not to fail for that exact reason. It's used on many non critical boot failures.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74370?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49a988b7eda009456d438ba7be0d2918826e1c36
Gerrit-Change-Number: 74370
Gerrit-PatchSet: 5
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:30:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74370?usp=email )
Change subject: soc/intel/meteorlake: Replace assert with error message
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> asserts are not failing boot by default and are more appropriate than error messages. Even when asserts are halting boot, they pinpoint you to an issue. With error messages you can have many other ones and debugging is harder.
the point here is that, we don't want to boot halt the device (like what asset does) as any such UPD misalignment is not a boot critical failure.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74370?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49a988b7eda009456d438ba7be0d2918826e1c36
Gerrit-Change-Number: 74370
Gerrit-PatchSet: 5
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:27:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Haribalaraman Ramasubramanian, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Shelley Chen, Subrata Banik.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80504?usp=email )
Change subject: soc/intel/alderlake: Remove CNVi assertions
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80504/comment/b802fc42_7e7f8312 :
PS4, Line 12: This seems to make sense and allows us
: to enable CNVi bluetooth when necessary.
> Intel uploaded another option that you may find more acceptable. See CB:80564.
Yes I like that approach better :-)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80504?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: I822a4e360fde100b8289cacf10a01f6d97facbb4
Gerrit-Change-Number: 80504
Gerrit-PatchSet: 4
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:26:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74370?usp=email )
Change subject: soc/intel/meteorlake: Replace assert with error message
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
asserts are not failing boot by default and are more appropriate than error messages. Even when asserts are halting boot, they pinpoint you to an issue. With error messages you can have many other ones and debugging is harder.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74370?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49a988b7eda009456d438ba7be0d2918826e1c36
Gerrit-Change-Number: 74370
Gerrit-PatchSet: 5
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:18:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Haribalaraman Ramasubramanian, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Subrata Banik.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80504?usp=email )
Change subject: soc/intel/alderlake: Remove CNVi assertions
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80504/comment/ccc41694_b3151ff2 :
PS4, Line 12: This seems to make sense and allows us
: to enable CNVi bluetooth when necessary.
> > > Do you want to have some mechanism at runtime to change them? […]
Intel uploaded another option that you may find more acceptable. See CB:80564.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80504?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: I822a4e360fde100b8289cacf10a01f6d97facbb4
Gerrit-Change-Number: 80504
Gerrit-PatchSet: 4
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:16:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Haribalaraman Ramasubramanian, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Subrata Banik.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80504?usp=email )
Change subject: soc/intel/alderlake: Remove CNVi assertions
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80504/comment/f0791568_e0e3ae95 :
PS4, Line 12: This seems to make sense and allows us
: to enable CNVi bluetooth when necessary.
> > Do you want to have some mechanism at runtime to change them?
>
> Yes. Currently in Brox some SKUs within a same design use discrete WiFi/BT module and some use CNVi/BT as indicated in the TEST message. This is decided by probing the FW_CONFIG mask at run-time.
I still don't understand. You want to change from not booting due to asserts to booting with error messages? That hardly sounds like a good idea. Why not fix the mainboard code and update the config at runtime, rather than have the common code accept invalid configurations?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80504?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: I822a4e360fde100b8289cacf10a01f6d97facbb4
Gerrit-Change-Number: 80504
Gerrit-PatchSet: 4
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 15 Feb 2024 17:14:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment