Attention is currently required from: Kangheui Won, Tim Wawrzynczak, Reka Norman, Ivy Jian, Eric Lai. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63774 )
Change subject: lib/spd: Change log prefix for ddr4 params ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
As I mentioned in the issue, we still lack of jedec official SPD document for the LPDDR5 and DDR5... […]
I agree with Tim, I'm not very familiar with this matter but it looks like you're taking the wrong approach here. If returning true here is intentional for LPDDR5, then clearly you should just be adding another case label for that. If we don't know whether that's the right thing to do, then do we really want to ship hardware with this? The whole point of having tests that check for warnings in the log is that we make sure we resolve these kinds of questions before release, not that we just delete/downgrade a perfectly valid warning just so we can check the "no warnings on boot" box on a release checklist.
Besides, in general we should not be changing messages in upstream coreboot just to make a Chrome OS test pass unless that change also makes sense from the upstream perspective. In this case, it doesn't seem like it does. If you ever have a situation where it makes sense for a log message to be counted as "error" according to upstream coreboot's understanding of log levels, but for Chrome OS we really decided that having this message in the boot log doesn't signify a problem or something that should be solved more cleanly in a different way, then you should add an allowlist for that specific log message to the Chrome OS test, not change the behavior for everyone upstream.