Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33661 )
Change subject: vendorcode/google: load sar config from CBFS first then VPD ......................................................................
Patch Set 1:
The reason for this CL is to set WIFI SAR in CBFS to be higher priority then VPD in RO. As a result, we don't consider to update FW RO actually; we update CBFS in RW to override the SAR value instead if we need to.
This CL is a special case that people have VPD incorrectly set. And this is an example how communication with partner would lead to error, and how complicated the whole chromebook manufacturing is that you can't expect everyone to know every details - and things will go wrong sometimes.
With VPD, there's no way to fix (except special workarounds like this CL) it properly. But if it's CBFS, just one RW update would solve it.
Regarding review effort, no matter in factory test or CBFS we all need someone to review so there is no difference. There are others in VPD RO like region and MAC so ODM should take responsibility for them.
It's true there are other VPDs, and we've seen them went wrong - for example the keyboard layout before we deprecate it with region. So the goal should be to minimize things we provisioned, just like how we abstracted all regional fields into single 'region' and have a clear way to make sure it won't go out of range (so even if partner set it wrong, it'll be very easy to catch, or has very limited impact). MAC and SNs are in the case that there's no better way to handle them. But SAR can be easily handled inside CBFS.
Regarding review, if it's in CBFS then we only need the RF engineer to review what's written in firmware source is correct. And it's in Google source repo, easily checked, one time effort. If it's a VPD process, you'll need: (1) ODM or RF eng to propose value, (2) communication to partner factory shopfloor eng to set this value in their backend, (3) write a test or something to validate if the value is correct. The value may go wrong any time in these steps, even in copy-paste. Also, (2) and (3) may be only in partner's local source folder, never goes back to Google to review. RF engineer will only see the failure when they received devices (or never).
For example, there is a next wave device like laser which is just new SKU IDs of phaser360. Since phaser360 was shipped, the cherry-pick policy to model.yaml of this device is strict which introduce troubles for development cycle if we can only set SAR in CBFS.
That's the nature of device management. If this is a new device, then it should be a new model - which can have its own set of firmware (and RO). And we can easily create a per-SKU SAR table in new firmware, and select it accordingly. Or am I missing anything? (Not quite following why model.yaml is mentioned here)