Aren't you already using some sort of lib-cros-config in mosys now to process the copy of the dtb that has been added? That should address your code duplication problem (at least in terms of typing code, maybe not disk space).

Leaving that aside, as long as the produced shellball remains totally self-contained and you don't break anything along the way I'm relatively indifferent. For systems that don't use UB it may not be very practical though.

For non-unibuild systems cros_config doesn't produce anything useful at present so we cannot use it. I suppose I could adjust it to call out to mosys just so there is one interface, but it would use up extra space (about 515KB I think).

Yes mosys does use the master config so we are at least using the same config in both places. We are not currently using libcros_config in mosys though - we just duplicate the code. Also libcros_config is a C++ library, but I suppose we could make that work and it would reduce code duplication. I've been reluctant to introduce that dependency but perhaps it is worth it. I'll take a look.

Wait, you're thinking of linking mosys to libcros_config? Please don't do that. It's going to make it harder to unwind the bidirectional interdependency between mosys and cros_config. Also, C++ library usage in C has some gotchas.

OTOH, I believe that cros_config can be reduced by using LTO and stripping debug symbols, so we can reduce far beyond current 515KB estimate.

Yes that was the idea. At present cros_config depends on mosys but not the other way around. Yes, mosys depends on the master configuration, but that is not in cros_config - it is in the board overlays (produced by the chromeos-config ebuild).

As you say, linking mosys to libcros_config *would* create a bidirectional dependency, since libcros_config calls 'mosys platform id'. That would be bad.

I've been thinking we should change that though, since:

- mosys seems to take a long time to run (about 65% of the time to run cros_config seems to be mosys)
- we have to add vpd access
- mosys is just reading tables anyway

Unfortunately, mosys's command line interface is its API surface and really the only thing that's guaranteed to work and stay stable without further investment in mosys.

As such, this situation needs to be improved (among many other problems with mosys). So, I have a proposal and prototype in the works to fix that but it will take some time to come to consensus. Let's not do anything on this bug right now. The proposal will not be incompatible with the Config V2 proposal with mosys and cros-config owning their respective data schema domains (while still maintaining a single config source).

The only catch here is that I need to do something for this whitelabel thing, ideally before Monday...

mosys already supports getting customization_id from VPD: http://cs/chromeos_public/src/platform/mosys/lib/misc/sku.c?l=105&rcl=4a6f78bf288e2e8ba8cad8279233cae6048910b2

I prefer to keep firmware updater simple and straight forward with less dependency, instead of binding to things in binary format and really hard to change.

There may be many places we have to run firmware updater to "a different target" for example the proto units may have something wrong that we have to override the values like which image to write or which model to expect, so having a very simple list of things that updater will read and ways to override them would be convenient.

Hung-Te, thanks for the info. To be a bit more specific, see this:

https://chromium-review.googlesource.com/c/chromiumos/platform/firmware/+/735334/12/pack_dist/updater4.sh

It calls out to vpd to get the signature ID. This works OK, but there is a request to put this logic in mosys (or cros_config) instead. Strictly speaking, this isn't necessary since VPD will contain the signature ID (e.g. 'whitetip1') but it would be nice to check the signature ID is actually known and valid.

With zero-touch whitelabels, we have to read the VPD in order to figure out the model (or whitelabel tag). That's the only way we can access the brand code, for example. As an aside, if we could use 'cros_config / brand-code' to access the brand code, instead of 'mosys platform brand' then we could drop the support in mosys. In the firmware updater we could do 'cros_config /firmware sig-id', for example instead of the vpd call.

Re your point about simple and straightforward, the current dependencies are that mosys depends on the master configuration. To update the SKU ID mapping, etc. you must change the master configuration (a text file) and then 'emerge-$board chromeos-config-bsp chromeos-config mosys'. We we spoke about this you mentioned that you sometimes recompile mosys to put in hacks. Does that approach work for you?

What are you referring to with 'binding to things in binary format and really hard to change'? But mosys is in binary format, right?

As you can tell I am finding all the different requests quite confusing. I would like 'mosys platform' to play a smaller role in the future with unified builds (see my doc about that).

True mosys is a compiled program, but it was made only to get few very primitive identifiers like model / sku, and would be the only place if we need to hack stuff.

What I mean was, if cros_config (another binary, I'd assume) will be added, then it's more complicated that we have to figure out another program, and where its configs will live, especially that the config is device tree binary, and if the config contains anything other than model/sku like complex mapping of which firmware / key to select.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/firmware/+/437624395f6c91562201b41958615f6afdf016cc

commit 437624395f6c91562201b41958615f6afdf016cc
Author: Simon Glass <sjg@chromium.org>
Date: Sat Nov 18 04:18:25 2017

pack_firmware: Show the model name with firmware update

This is useful information when using firmware update on a unified build.
Display it at the start of the update.

BUG=chromium:783299
TEST=run firmware update on robo and see:
Model is robo

Change-Id: I647614b5f51b23f74eaf8be63520598e78a279f1
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/767374
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/437624395f6c91562201b41958615f6afdf016cc/pack_dist/crosutil.sh

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/firmware/+/7bd124d866df83946694832bb03311f8c8ef4e41

commit 7bd124d866df83946694832bb03311f8c8ef4e41
Author: Simon Glass <sjg@chromium.org>
Date: Mon Dec 11 09:06:38 2017

pack_firmware: Show the model name with firmware update

This is useful information when using firmware update on a unified build.
Display it at the start of the update.

BUG=chromium:783299
TEST=run firmware update on robo and see:
Model is robo

Change-Id: I647614b5f51b23f74eaf8be63520598e78a279f1
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/767374
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/794731
Commit-Queue: Marco Chen <marcochen@chromium.org>
Tested-by: Marco Chen <marcochen@chromium.org>

[modify] https://crrev.com/7bd124d866df83946694832bb03311f8c8ef4e41/pack_dist/crosutil.sh