Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Subrata Banik, Michael Niewöhner, EricR Lai.
2 comments:
File src/soc/intel/alderlake/romstage/fsp_params.c:
Patch Set #47, Line 34: die("Unsupported pcie_rp_type!");
If die() fixes the compiler warning, probably only because it has attribute((noreturn))
or something like that. It's just deferring the construct that turns the warning off.
But that's not needed, how about a switch/case with default?
switch (type) {
case PCH_PCIE_RP:
return rp_number;
case CPU_PCIE_RP:
default:
return 0x40 + rp_number;
}
This way you could avoid an `else` path and still have `CPU_PCIE_RP` mentioned explicitly.
I only stumbled over this because people seem to be concerned about
code sizes lately. The die() statement probably adds 40B of nothing.
Well, assert() is not a drop-in replacement for die(). I'm not sure what should the assertion be checking, though.
The assertion would be:
assert(type == PCH_PCIE_RP || type == CPU_PCIE_RP);
Assertions are mere documentation (unless you have static code analyzation check them),
so doesn't change anything about the compiler warning. It is also redundant because we
already specify type constraints. Alas, those are not enforced in C :-/
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
Because I don't want to touch get_l1_substate_control() and change it... […]
What I meant is that we have now the flags declaration for board config
on top, then a bunch of unrelated function prototypes and then board
configuration again. I'm also wondering why these things are mixed in a
single header file (IIRC, I already commented elsewhere about that).
To view, visit change 48340. To unsubscribe, or for help writing mail filters, visit settings.