9 comments:
This leads to all sorts of problems and errors, for example:
- links get lost
- only 1.5 or 3 Gbps instead of 6 Gbps
- "Unaligned Write" errors in Linux
- ...
You told me this only happens when you enable power management in SATA (e.g. SALP), so why don't you mention it anywhere?
provide an option
for breaking coreboot
That is not its intended purpose, yet your wording makes it sound as it were. I would suggest a less charged wording:
There is no reason to have debugging switches as devicetree options, anyway.
Its actual purpose would be to disable power management stuff for debugging purposes. Power management mechanisms in the hardware are increasingly complex, so there's lots of things that could go wrong. Unfortunately, there are undocumented interdependencies between FSP UPDs. In this case, looks like enabling SATA power management but disabling power management programming steps is an invalid configuration, but nothing says a word about that. Moreover, the FSP integration in coreboot results in default values of zero for the devicetree fields that are not explicitly assigned on each mainboard. So, even if FSP had the correct default value in the UPDs, it would get overwritten by coreboot.
Surprisingly cml and cnl are not affected, even though they share mostly
the same reference code in this regard.
That nobody reported any problems doesn't mean that it's not affected. There's no guarantee that things remain like this in the future, so I would omit this part.
Patch Set #3, Line 26: Thus, only skl gets changed.
A more compelling reason to only change SKL for now is because it has been tested and works: as per IRC, this change fixes problems on two different types of platforms without any noticeable downsides.
File src/soc/intel/skylake/chip.c:
Patch Set #3, Line 264: (link)
Why the parentheses?
Patch Set #3, Line 264: * For unknown reasons FSP skips SATA init steps, resulting in (link) errors, unaligned write
line over 96 characters
Please fix
Patch Set #3, Line 264: unaligned write
What's an unaligned write error?
Patch Set #3, Line 264: For unknown reasons FSP skips SATA init steps
Does it always skip those SATA init steps? If not, when does it skip such steps? The current wording of this sentence does not make this clear enough.
Patch Set #3, Line 265: the SATA SIR
Only one SATA SIR?
To view, visit change 40877. To unsubscribe, or for help writing mail filters, visit settings.