Attention is currently required from: Raul Rangel, Martin Roth, Paul Menzel, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54639 )
Change subject: mb/google/guybrush: Add helpers for cbi fw_config settings. ......................................................................
Patch Set 5:
(1 comment)
File src/mainboard/google/guybrush/variants/guybrush/helpers.c:
https://review.coreboot.org/c/coreboot/+/54639/comment/d6f50746_211d7270 PS4, Line 44: get_variant_wwan_type
My counter proposal to your change is this:
Can you please expand on this a bit more with some examples?
Each variant still supplies the option bit definitions as we do now.
Is this only for the fw_config fields that are applicable to the variant.
All fw_config settings are defined in devicetree at a platform level - not about which bits are used for the settings, just having a definition so that it can be accessed by all variants.
Can you please provide more details of what this would look like? What changes are expected in the fw_config syntax as it currently exists in the devicetree and what it translates to as part of static.c or fw_config.c. Some examples would be helpful in understanding the proposal.
The top level helpers can just check fw_config for anything they need to, and it's expected that they'd get back a response. If the config option doesn't exist on that platform, it's just returned as it's not enabled.
Can you please provide an example what the helper would really look like and what you mean by "get back a response"? What support would be required to determine config option doesn't exist on the platform.
What I've proposed above doesn't require that we throw away all of the experience we have about this right now - it's an almost trivial expansion on what we already have.
It isn't really clear what the devicetree/static.c/fw_config.c/helpers would look like in this proposal. Can you please provide more details? I am all ears to understand the proposal.