Attention is currently required from: Cliff Huang, Jérémy Compostella, Kapil Porwal, Pranava Y N, Saurabh Mishra.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83798?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage
......................................................................
Patch Set 86:
(5 comments)
Patchset:
PS81:
> @mishra.saurabh@intel.com, can you please ensure you are following CB:84182 and CB:84183 while creating ramstage CL for panther lake?
Also, CB:84217 and CB:84220
File src/soc/intel/pantherlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/83798/comment/c03367eb_2ee1fec7?us… :
PS50, Line 15:
> I am checking this data from PNP team.
ping, if this is good time to check this ?
File src/soc/intel/pantherlake/elog.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/ef4164a5_6a1c68bf?us… :
PS71, Line 140:
> Yes. The code is ready once GPE1 changes CLs are merged, I will work with […]
Acknowledged
File src/soc/intel/pantherlake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/11f79fe1_a5fd4f20?us… :
PS71, Line 110:
> Yes. The code is ready once GPE1 changes CLs are merged, I will work with […]
Acknowledged
File src/soc/intel/pantherlake/retimer.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/aa49213d_260e592a?us… :
PS71, Line 19:
: for (uint8_t i = 0; i < MAX_TYPE_C_PORTS; i++) {
> Hi, declaring variables of the same type together in for loop is accepted in c.
> Here, variable i is "uint8_t " type, while ec_port variable is declared "int".
> This will give an error since mixing types in the initialization section is not allowed.
>
> Can be used as below?
> ````
> for (int i = 0, ec_port = 0; i < MAX_TYPE_C_PORTS; i++) {
> ````
sure
--
To view, visit https://review.coreboot.org/c/coreboot/+/83798?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: Idc6fb11e9e84c28c7567ae2b7abc1ab832a88362
Gerrit-Change-Number: 83798
Gerrit-PatchSet: 86
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 10:01:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Keith Short, Krishna P Bhat D, Nick Vaccaro, Paul Menzel, Rishika Raj, Ronak Kanabar, Shelley Chen, Sowmya Aralguppe, Sowmya Aralguppe.
Subrata Banik has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83752?usp=email )
Change subject: mb/google/brox: Fix booting to kernel without battery
......................................................................
Patch Set 22:
(1 comment)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/8bc70074_5b7c4a8d?us… :
PS19, Line 57: * PL4 calculations - For USB C charger (Max Power):
> Subrata - the boot time increase was not consistently coming in 15:starting LZMA decompress (ignore for x86) -- after starting to load payload
> 1050:finished reading kernel from disk
> So i could not root cause sorry
I'm unable to follow your statement. If you're saying that boot time has regressed after limiting PL4 at a certain level, then there must be an explanation. Often, we've seen boot time regress because the CPU is operating at low speed, the clock has been reduced, or there's an idle delay. I'm asking you to narrow down the observation by looking at the factors that become prevalent when you limit PL4. Otherwise, this is just another observation without a root cause.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83752?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: I58e136c607ea9290ecac0cee453d6632760a6433
Gerrit-Change-Number: 83752
Gerrit-PatchSet: 22
Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Herrmann <eherrmann(a)google.com>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com>
Gerrit-CC: Dengwu Yu <yudengwu(a)huaqin.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Gerrit-CC: Vinay Kumar <vinay.kumar(a)intel.com>
Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Eric Herrmann <eherrmann(a)google.com>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Keith Short <keithshort(a)google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 09:57:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Elyes Haouas, Jamie Ryu, Jérémy Compostella, Kapil Porwal, Paul Menzel, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Wonkyu Kim.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83789?usp=email )
Change subject: soc/intel/ptl: Add GPIOs for Panther Lake SOC
......................................................................
Patch Set 66:
(5 comments)
File src/soc/intel/pantherlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/83789/comment/b94d7299_985f199c?us… :
PS49, Line 436: Name (JTAG, Package (0x02)
this is mostly for FW debugging. why would kernel gpio controller driver would like to expose via ACPI. IMO, we should only expose GPIO PINs those are meaningful for kernel driver bindings and configuration. There is absolute no need to expose JTAG native PADs for kernel.
File src/soc/intel/pantherlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/83789/comment/29916c31_338ffba1?us… :
PS55, Line 135: GPPV
> Acknowledged
I don't see a comment section as request and you have marked resolved?
File src/soc/intel/pantherlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/83789/comment/8bd01501_553ced98?us… :
PS65, Line 770: GPIO
`VGP0` aka virtual GPIO
https://review.coreboot.org/c/coreboot/+/83789/comment/eceddf51_b80f98e8?us… :
PS65, Line 805: GADD
can you please move these ACPI methods before exposing the `GPIO` device ?
File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/aef4bac8_72c23638?us… :
PS49, Line 31: 1
> We don't have COMM2, so GPP_COMM3 cannot use this macro, though.
best to my understanding `GPIO community 2` is always `GPD` and we don't need to expose those as it seems not required to configure the PADs. There is no harm of defining
```
#define COMM_0 0
#define COMM_1 INC(COMM_0)
#define COMM_3 INC(COMM_1)
#define COMM_4 INC(COMM_3)
#define COMM_5 INC(COMM_4)
```
The following code relies on the above macro definition. As long as you pass the correct PID for GPIO Community 3 onward, there's no problem maintaining the INC macro here.
https://review.coreboot.org/c/coreboot/+/83789/55..65/src/soc/intel/panther…
Additionally, there is also no harm to keep `COMM_2` in between because this GPIO community exists but not exposed for FW/SW configuration.
same logic is true for `GPP_COMM3_ID` as well. you just need a unique number for `_UID`
--
To view, visit https://review.coreboot.org/c/coreboot/+/83789?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: Iae1bc072841214efaec7a10719dbc742f2da795b
Gerrit-Change-Number: 83789
Gerrit-PatchSet: 66
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 05 Sep 2024 09:18:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Keith Short, Krishna P Bhat D, Nick Vaccaro, Paul Menzel, Rishika Raj, Ronak Kanabar, Shelley Chen, Sowmya Aralguppe, Sowmya Aralguppe.
Subrata Banik has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83752?usp=email )
Change subject: mb/google/brox: Fix booting to kernel without battery
......................................................................
Patch Set 22:
(1 comment)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/e9d4a515_171403a5?us… :
PS19, Line 57: * PL4 calculations - For USB C charger (Max Power):
> 1) Setting PL4 as 9 which is the PL4 value for 30watts is increasing boot time significantly (more than 2 secs) for default adaptor (65w)
I guess my question was "did you get a chance to root cause or find the reason why the boot time has regressed only during IO bound operation like reading from a block device while reducing the PL4 limit ?"
> 2) Several iterations were recorded and the increase in the boottime is varying between modules
> >>However the difference between 32 and 36 efficiency for 60/65 watts is not very significant .Hence removed the if condition and all the adaptors will have same efficiency (32%)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83752?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: I58e136c607ea9290ecac0cee453d6632760a6433
Gerrit-Change-Number: 83752
Gerrit-PatchSet: 22
Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Herrmann <eherrmann(a)google.com>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com>
Gerrit-CC: Dengwu Yu <yudengwu(a)huaqin.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Gerrit-CC: Vinay Kumar <vinay.kumar(a)intel.com>
Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Eric Herrmann <eherrmann(a)google.com>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Keith Short <keithshort(a)google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 09:11:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.corp-partner.google.com>
Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>