Attention is currently required from: Martin Roth, Subrata Banik, Andrey Petrov, Patrick Rudolph.
6 comments:
Commit Message:
There are currently various callbacks and hooks for chipsets and
mainboards in various places around the FSP calls, but I need some
mainboard hooks immediately before and after the FSP calls.
From the follow-up changes, it looks like you are using these callbacks for configuring pads before the FSP call. This is currently achieved by using the calls:
`platform_fsp_memory_init_params_cb` for FSP-M
`platform_fsp_silicon_init_params_cb` for FSP-S
What is the significance of performing the GPIO initialization just before the jump to FSP-M/S? Not completely against having the hooks, but currently there is one function call into SoC (which in turn can call mainboard) that allows each to perform any initialization that is required before the FSP calls. It also ensures that proper ordering is maintained between SoC and mainboard w.r.t. initializing UPD params.
Patch Set #1, Line 13: This allows for GPIO initialization before the calls as required
That can be achieved by using the call chain for platform_fsp_memory_init_params_cb and platform_fsp_silicon_init_params_cb
Patch Set #1, Line 14: easy analysis and updates of register changes within the FSP.
If this is the intent, I think a better place to put the callbacks would be fsp_debug_{before|after}_{memory|silicon}_init(). It will allow you to dump or check any registers or configurations before and after FSP calls. Example: Recently the debug path was updated to snapshot and verify any GPIO changes across FSP calls: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/drivers/intel/fsp2_0/debug.c#19. Similarly, other hooks can be added as required.
File src/drivers/intel/fsp2_0/include/fsp/api.h:
Patch Set #1, Line 75: print values
This is better suited for the debug path in my opinion.
Patch Set #1, Line 75: save and restore registers
This seems like a problem with the FSP implementation itself. If there are certain things FSP must not touch, then we should be pushing the SoC vendor to fix that rather than adding permanent workarounds in coreboot. I understand that we have to deal with such situations and we have intentionally kept this outside the FSP driver to ensure that those actually get addressed rather than accepting it as the right solution. Example: CB:52248
Patch Set #1, Line 75: other initialization
Other initialization can already be handled via the callback path we already have.
To view, visit change 52866. To unsubscribe, or for help writing mail filters, visit settings.