Attention is currently required from: Michał Żygowski, Paul Menzel.
Patch set 5:Code-Review +1
4 comments:
Patchset:
Thanks for also looking at the registers. I guess we could add a TODO
for earlier chipsets that support eSPI. Currently we might add LPC/FWH
buses by accident for them. For these transition chipsets, we should
probably check a strap if it's using eSPI or LPC.
File chipset_enable.c:
Patch Set #3, Line 659: { "eSPI", BUS_LPC | BUS_FWH } };
Not sure about this one, whether eSPI should be mapped to LPC and FWH
No, it shouldn't. We use these flags to decide which chips to probe
for, and I guess even if an eSPI chip might work similarly (IIRC,
LPC/FWH just had memory mapped registers) it would be a different
set of chips (does anybody make/use eSPI NOR flash?).
File ich_descriptors.c:
Patch Set #5, Line 278: return "reserved";
I'd suggest "unknown" if we don't want to add a table above. Note, that
the call to this function for `freq_read` is spurious since, I guess,
Cannon Point and should probably be guarded.
Patch Set #5, Line 967: return CHIPSET_500_SERIES_TIGER_POINT;
As suggested somewhere else, I would make this the new fall-back, i.e.
if (ICCRIBA == 0x34)
return CANNON_POINT;
if (ICCRIBA != 0x11)
msg_pwarn(...);
return TIGER_POINT;
That should make things more future-proof.
Calling it ICCRIBA for Tiger Point is wrong ofc. but that's just cosmetics.
We could add another union in `ich_descriptors.h`.
To view, visit change 55578. To unsubscribe, or for help writing mail filters, visit settings.