Attention is currently required from: Anil Kumar K.
Karthik Ramasubramanian has posted comments on this change by Anil Kumar K. ( https://review.coreboot.org/c/coreboot/+/81920?usp=email )
Change subject: drivers/soundwire: Support Realtek ALC722 codec
......................................................................
Patch Set 11:
(2 comments)
File src/drivers/soundwire/alc711/alc711.c:
PS11:
> yes, that's the problem i see with this patch […]
I feel the address information can be filled in the devicetree as part of chip_info by adding `address` member into `struct drivers_soundwire_alc711_config`. That address information can then be filled into SSDT.
https://review.coreboot.org/c/coreboot/+/81920/comment/5c15bf15_3f58e527?us… :
PS11, Line 162: .name = "Realtek ALC711 SoundWire Codec",
: #elif CONFIG(DRIVERS_SOUNDWIRE_ALC722)
: .name = "Realtek ALC722 SoundWire Codec",
The name can be consolidated into "Realtek ALC711/722 SoundWire Codec". I dont think this name is used for probing.
That way we can avoid separate Kconfig for ALC711 and ALC722.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81920?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: Ieb16a1c6f3a79321fdc35987468daa8be33b6e49
Gerrit-Change-Number: 81920
Gerrit-PatchSet: 11
Gerrit-Owner: Anil Kumar K <anil.kumar.k(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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-CC: Naveen M <naveen.m(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sathya Prakash M R <sathya.prakash.m.r(a)intel.com>
Gerrit-CC: Sathyanarayana Nujella <sathyanarayana.nujella(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Comment-Date: Wed, 21 Aug 2024 18:36:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Keith Short, Nick Vaccaro, Rishika Raj, Shelley Chen, Sowmya Aralguppe, Subrata Banik.
Sowmya Aralguppe 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 11:
(2 comments)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/0d2370d4_748070b0?us… :
PS11, Line 15: #define SET_PSYSPL2(e, w) ((e) * (w) / 100)
> Are we not proposing a unified solution for all brox variants. […]
I am not sure about this - I wanted to first have a proven solution for Brox and then take it to other variants in a different patch.
But adding the same solution for other variants in this patch is fine by me
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/2fbf93d6_dbb75fb0?us… :
PS9, Line 359: if (CONFIG(EC_GOOGLE_CHROMEEC) && (!google_chromeec_is_battery_present_and_above_critical_threshold())) {
> mainboard_memory_init_params callback exists for the mainboards to override any FSP M Configs in rom […]
Right - I will implement the same in my next patch
--
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: 11
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: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.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: Rishika Raj <rishikaraj(a)google.com>
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: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(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: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Wed, 21 Aug 2024 17:45:21 +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: Karthik Ramasubramanian <kramasub(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, 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 46:
(1 comment)
File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/e8662ce9_191c6b5e?us… :
PS17, Line 544: #define GPP_VGPIO0 246
> Let's the feedback on lack of info in PTL EDS and route this to the proper owner for change. At this point, I totally agree with you, but I need your help from the customer's side bring up the concern. Let me also check if there is document with the info under NDA.
Thanks, I will share the feedback and please check if there is other means to get the doc.
--
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: 46
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: 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: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(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: 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: Wed, 21 Aug 2024 17:31:31 +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: Anil Kumar K, Bora Guvendik, Elyes Haouas, Jamie Ryu, Jérémy Compostella, Kapil Porwal, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Subrata Banik, Wonkyu Kim.
Cliff Huang 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 46:
(1 comment)
File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/d4eba92d_883b9aeb?us… :
PS17, Line 544: #define GPP_VGPIO0 246
> > PTL EDS includes VGPIO_TS0...VGPIO_THC3 in community3. […]
Let's the feedback on lack of info in PTL EDS and route this to the proper owner for change. At this point, I totally agree with you, but I need your help from the customer's side bring up the concern. Let me also check if there is document with the info under NDA.
--
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: 46
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: 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: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.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: 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: Wed, 21 Aug 2024 17:27:53 +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, Nick Vaccaro, Rishika Raj, Shelley Chen, Sowmya Aralguppe, Subrata Banik.
Sowmya Aralguppe 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 11:
(9 comments)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/4e5e0c15_7337e187?us… :
PS11, Line 15: #define SET_PSYSPL2(e, w) ((e) * (w) / 100)
> If not setting Psys please remove it.
Other variant's build were failing - just to make sure other variants are not affected i added this back
https://review.coreboot.org/c/coreboot/+/83752/comment/d687260c_6b6b20fb?us… :
PS11, Line 16: SET_PL2
> If not setting PL2, please remove it.
Other variant's build were failing - just to make sure other variants are not affected i added this back
https://review.coreboot.org/c/coreboot/+/83752/comment/b7efa4d4_c5145a98?us… :
PS11, Line 19: #define TENPOWERSIX 1000000
: #define TENPOWERTHREE 1000
:
> Instead of TENPOWERSIX and TENPOWERTHREE, please define MILLIVOLT_TO_VOLT and MILLIAMP_TO_AMP. […]
Yes right - will do
https://review.coreboot.org/c/coreboot/+/83752/comment/85543f25_6a690fbc?us… :
PS11, Line 82: MILLIWATTS_TO_WATTS);
> Indent it after the DIV_ROUND_UP open parenthesis.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/e9953990_ba2d9c30?us… :
PS11, Line 87: &volts_mv);
> Same comment as above.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/5c58b834_4540c733?us… :
PS11, Line 98: soc_config->tdp_pl4 = SET_PL4(40, watts);
> Can you please add a table at the start of this function to provide an idea of what the PL4 will loo […]
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/ed662272_8213a9cf?us… :
PS11, Line 102: watts);
> Same comment as above.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/45ca2c4b_f942a1c7?us… :
PS11, Line 146: void variant_update_psys_power_limits
> It seems not used anymore. If so, can we remove it. Also remove the function prototype in variants. […]
Other variant's build were failing - just to make sure other variants are not affected i added this back
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/1b208368_76d6034e?us… :
PS9, Line 359: if (CONFIG(EC_GOOGLE_CHROMEEC) && (!google_chromeec_is_battery_present_and_above_critical_threshold())) {
> How about moving inside mainboard_memory_init_params override in brox mainboard. […]
This should be written in romstage .so I am not very sure about writing in mainboard_memory_init_params .let me sync up internally about this and come back with an answer.
The only way i could think of is to keep this exclusive for Brox using Kconfig - but adding too many configuration parameters is not the right approach also ..so i am not pursuing that approach.
--
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: 11
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: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.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: 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: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Eric Herrmann <eherrmann(a)google.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(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: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Wed, 21 Aug 2024 17:15:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, V Sowmya.
Subrata Banik has posted comments on this change by V Sowmya. ( https://review.coreboot.org/c/coreboot/+/84019?usp=email )
Change subject: mb/google/nissa/var/nivviks: Prevent camera LED blinking during boot
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
@digehlot@google.com@google.com can you please test this Code. I assume you have a Tivviks with you
--
To view, visit https://review.coreboot.org/c/coreboot/+/84019?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: I979207d1b6d55f78dea20d3366ef4a833ee9c86d
Gerrit-Change-Number: 84019
Gerrit-PatchSet: 3
Gerrit-Owner: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Wed, 21 Aug 2024 17:14:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Elyes Haouas, Jamie Ryu, Jérémy Compostella, Kapil Porwal, 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 46:
(3 comments)
Patchset:
PS46:
> Subrata, I don't quite get it this time:( I don't see macros from the patchset you mentioned. Can you give me more info? Thx
I'm asking about TCSS.h file inclusion for PTL (I assume that can be done as part of this CL because those macros are now essential for GPIO aux programming)
I assume the PTL TCSS macro would look something like below ?
```
#define BIAS_CTRL_VW_INDEX_SHIFT 24
#define BIAS_CTRL_BIT_POS_SHIFT 16
```
To meet below implementation
```
In PTL SOC:
15:0 GROUP_ID Group ID in PCH GPIO; targeted SB_PORTID
18:16 BIT_NUM Data bit Position in PCH GPIO
31:24 VW_INDEX VW Index in PCH GPIO
```
File src/soc/intel/pantherlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/83789/comment/1d509075_74151f5c?us… :
PS17, Line 54: .port = PID_GPIOCOM0,
> Subrata, I have added these two CLs: […]
Acknowledged
File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/d153de87_5d9f1b4d?us… :
PS17, Line 544: #define GPP_VGPIO0 246
> PTL EDS includes VGPIO_TS0...VGPIO_THC3 in community3. I know we will need the ones for THC for enabling wake. I don't know if we will use any other vGPIO PADs. Let's bring up those info needed for code review from EDS in Monday's meeting.
as I'm repeating myself, any code review need minimum doc access to ensure things are doing in proper. If platform is using GPIO COMM(3) or something else is secondary question because you are upstteaming SoC code and that has to be backed up by the doc. I'm find if you can share the doc with me with any special access buy please do share. I don't follow what is expected from Monday's meeting to continue my code review?
--
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: 46
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: 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: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(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: 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: Wed, 21 Aug 2024 17:13:07 +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>