Attention is currently required from: Kapil Porwal, Pranava Y N, Sowmya Aralguppe.
Subrata Banik has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/85531?usp=email )
Change subject: soc/intel/pantherlake: Remove hardcoded value for child nodes ......................................................................
Patch Set 4:
(3 comments)
File src/soc/intel/pantherlake/crashlog.c:
https://review.coreboot.org/c/coreboot/+/85531/comment/bb2853f7_adbae9d1?usp... : PS4, Line 319: cpu_cl_disc_tab.header.fields.count = CRASHLOG_NODES_COUNT; I'm wondering why the previously discussed items are now being ignored. Earlier, you were hardcoding the value to limit the child nodes to 2 (PUNIT & Uncore of SoC-N Die). This was done because we had seen issues where without the hardcoding we were reading some other junk nodes that were not capable of reporting the crashlog and we were seeing crashes/timeouts.
``` /* Crashlog record of PUNIT & Uncore of SoC-N Die */ ```
https://review.coreboot.org/c/coreboot/+/83106/
https://review.coreboot.org/c/coreboot/+/85531/comment/12602a16_2b08c275?usp... : PS4, Line 316: printk(BIOS_DEBUG, "cpu_crashlog_discovery_table buffer count: 0x%x\n", : cpu_cl_disc_tab.header.fields.count); what is the point of showing buffer count? earlier, it was linked here due to overriding
``` cpu_cl_disc_tab.header.fields.count = CRASHLOG_NODES_COUNT; ```
I'm afraid we will go back to the previous problem which was fixed with https://review.coreboot.org/c/coreboot/+/83106/26..29. otherwise there was no need to hardcode fixed value https://b.corp.google.com/issues/346676856#comment33
https://review.coreboot.org/c/coreboot/+/85531/comment/b7c6a4ef_5ef4b22f?usp... : PS4, Line 319: u32 cur_offset = 0; why we are changing this ? can still stays at line 318