New issue
Advanced search Search tips

Issue 770183 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Allow mosys to access the master configuration

Project Member Reported by sjg@chromium.org, Sep 29 2017

Issue description

At present mosys does the mapping from SKU ID to model using a private table that is not in the master configuration.

We should move this to the master configuration and allow mosys to access it. This avoids having to touch an extra place to add a new model, and simplifies maintenance.
 

Comment 1 by sjg@chromium.org, Sep 29 2017

Status: Started (was: Untriaged)
I've done some work on this this morning. Initial CLs for mosys here:

https://chromium-review.googlesource.com/c/chromiumos/platform/mosys/+/692478

Needs testing on robo, etc.

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 29 2017

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a0cbb19dfc35d23ceda49290195de06e765f0e65

commit a0cbb19dfc35d23ceda49290195de06e765f0e65
Author: Simon Glass <sjg@chromium.org>
Date: Wed Oct 04 01:31:02 2017

chromeos-config: Add a mapping from SKU to model

Add a way to map from SKU ID to model / sub-model. This will allow us to
unify this config which is currently in mosys.

This uses a simple map. We can devise more complex schemes if the need
arises.

BUG= chromium:770183 
TEST=FEATURES=test emerge-coral chromeos-config-tools

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

[modify] https://crrev.com/a0cbb19dfc35d23ceda49290195de06e765f0e65/chromeos-config/README.md

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 4 2017

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/5f3fe607c49eedeb91e139c84f009878cc83b3a6

commit 5f3fe607c49eedeb91e139c84f009878cc83b3a6
Author: Simon Glass <sjg@chromium.org>
Date: Thu Oct 05 14:33:17 2017

Add support for embedding device trees

At present mosys uses an older version of Kbuild, apparently from Linux
around 2010, before device tree. Add in support for compiling and
embedding device trees, so we can access the master configuration by
embedding it in mosys.

BUG= chromium:770183 
BRANCH=none
TEST=with other changes: FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/5f3fe607c49eedeb91e139c84f009878cc83b3a6/scripts/Makefile.lib

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/403f374a623e9da62f612f5af28565f73b5b98bb

commit 403f374a623e9da62f612f5af28565f73b5b98bb
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 06 20:47:11 2017

Add support for accessing the SKU ID map and brand code

These are the only two pieces of information that mosys currently needs
from the master configuration. Most likely we will use cros_config for
other things.

Add support for reading from an embedded device tree. This will be used to
replace the internal table. The device tree is called config.dtb and is
written to lib/cros_config/ by the ebuild just before the mosys build
starts.

Control this with a new CONFIG_CROS_CONFIG which is off by default. Boards
which use unified builds can enable it (in the ebuild).

BUG= chromium:770183 
BRANCH=none
TEST=with other changes: FEATURES=test emerge-coral --nodeps mosys

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

[add] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/lib/cros_config/Makefile
[modify] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/Kconfig
[modify] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/lib/Makefile
[add] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/lib/Kconfig
[modify] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/Makefile
[add] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/lib/cros_config/cros_config.c
[add] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/include/lib/cros_config.h
[add] https://crrev.com/403f374a623e9da62f612f5af28565f73b5b98bb/lib/cros_config/Kconfig

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/f4303064d9723edb36d2a1e795e03691b1b31fc1

commit f4303064d9723edb36d2a1e795e03691b1b31fc1
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 06 20:47:11 2017

Add tests for cros_config

At present the mosys tests are not ideal. They don't run from the ebuild
and they require cmockery, which does not appear to be available in our
portage. Also there appear to be build errors (discovered after an attempt
to run the tests). It looks like the code that is being tested does not
change much, so perhaps the tests were determined to have little value,
although it seems that quite a bit of effort was put in at some point.

Add a new simple test program, which does not need cmockery, with a single
test of the cros_config library. Include a suitable test configuration
(taken from coral) which exercises the features.

BUG= chromium:770183 
BRANCH=none
TEST=with other changes: FEATURES=test emerge-coral --nodeps mosys

Change-Id: I72b883b6836aab80d7fa69f760ffe4078ff71ad2
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/692477
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: C Shapiro <shapiroc@google.com>

[add] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/tests/test_config.dts
[modify] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/Kconfig
[modify] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/Makefile
[add] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/tests/Kconfig
[add] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/tests/simple_tests.c
[add] https://crrev.com/f4303064d9723edb36d2a1e795e03691b1b31fc1/tests/Makefile

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/d41c477d55c89402123b132e7f3bf70698bf6f1a

commit d41c477d55c89402123b132e7f3bf70698bf6f1a
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 06 20:47:11 2017

Obtain the coral SKU ID table from the master configuration

At present mosys has its own table which maps from SKU ID to model and
brand code. This duplicates information in the master configuration and
goes against a design goal of unified builds, which is to unify the
configuration.

Embed the master configuration into mosys and use that to report the model
and brand code. This means that the SKU mapping is maintained in the same
place as other configuration.

With this extra code and the master configuration itself, the statically
linked mosys_s increases in size by 11400 bytes to 1344736 bytes on coral
at present, which seems acceptable. The code size increment is
approximately 9KB.

BUG= chromium:770183 
BRANCH=none
TEST=with other changes: FEATURES=test emerge-coral --nodeps mosys
Also manual test on robo

Change-Id: Ia9934dd1a8707fd75a20fcbd55d850e9dc0aef1c
Reviewed-on: https://chromium-review.googlesource.com/692478
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/d41c477d55c89402123b132e7f3bf70698bf6f1a/platform/reef/reef.c

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/5c0951b10bb357b7a791011c091f1dc3b1d82a0f

commit 5c0951b10bb357b7a791011c091f1dc3b1d82a0f
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 06 22:51:12 2017

mosys: Add access to the master configuration

With unified builds we want mosys to use the master configuration. Enable
this option and the tests.

BUG= chromium:770183 
CQ-DEPEND=CL:692478
TEST=with other changes: FEATURES=test emerge-coral --nodeps mosys
Also manual test on robo

Change-Id: I0d38cfbe52bf35f5b31bb0fb082fb7ba81505fe9
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/692479

[modify] https://crrev.com/5c0951b10bb357b7a791011c091f1dc3b1d82a0f/sys-apps/mosys/mosys-9999.ebuild

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/8494af9d0f312fbee0ee172afee9e3f08cb484df

commit 8494af9d0f312fbee0ee172afee9e3f08cb484df
Author: Simon Glass <sjg@chromium.org>
Date: Sat Oct 07 04:31:03 2017

Adjust the cros_config API to avoid static data

It's not great to have a static data value in the library. Once we get
cros_config handling the whole SKU decode we don't really need it.

Update the API to accept a parameter instead.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/8494af9d0f312fbee0ee172afee9e3f08cb484df/include/lib/cros_config.h
[modify] https://crrev.com/8494af9d0f312fbee0ee172afee9e3f08cb484df/lib/cros_config/cros_config.c
[modify] https://crrev.com/8494af9d0f312fbee0ee172afee9e3f08cb484df/platform/reef/reef.c

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/a2ed727c9507c3488a481345e0ea57979d4bf280

commit a2ed727c9507c3488a481345e0ea57979d4bf280
Author: Simon Glass <sjg@chromium.org>
Date: Sat Oct 07 04:31:03 2017

Make tests noisier

At present the tests don't actually log much. Update the code to log all
messages, for easier debugging.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys
See that there is more output now:
cros_config_setup_sku: Looking up SKU ID 0
cros_config_setup_sku: Found model 'astronaut'
cros_config_setup_sku: Looking up SKU ID 61
cros_config_setup_sku: Found model 'astronaut'
cros_config_setup_sku: Looking up SKU ID 62
cros_config_setup_sku: Found model 'astronaut'
cros_config_setup_sku: Looking up SKU ID 160
cros_config_setup_sku: Found model 'nasher'
cros_config_setup_sku: Looking up SKU ID 163
cros_config_setup_sku: Found model 'nasher360'
cros_config_setup_sku: Looking up SKU ID 255
cros_config_setup_sku: SKU ID 255 not found in mapping

Signed-off-by: Simon Glass <sjg@chromium.org>

Change-Id: I09f493783af2222d6404e3e548fa281bb28de5c5
Reviewed-on: https://chromium-review.googlesource.com/704096
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/a2ed727c9507c3488a481345e0ea57979d4bf280/tests/simple_tests.c

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/781f14ae777912759acf018f8ce398ec768ca1ce

commit 781f14ae777912759acf018f8ce398ec768ca1ce
Author: Simon Glass <sjg@chromium.org>
Date: Sat Oct 07 04:31:03 2017

Allow cros_config to read the SKU ID

At present the caller passes in the SKU ID. Once we start looking up the
SMBIOS name this will not make sense, since the binding which describes
how to do the lookup is managed by cros_config. The binding currently
says it uses the SMBIOS name and SKU ID, but we may in future adjust it
to support additional methods (e.g. to support ARM machines).

Adjust the API so that cros_config reads whatever it needs to be able to
figure out the model. That will avoid adding more and more things to the
API call, and giving the fall impression that the caller is in charge of
what is needed.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/781f14ae777912759acf018f8ce398ec768ca1ce/include/lib/cros_config.h
[modify] https://crrev.com/781f14ae777912759acf018f8ce398ec768ca1ce/lib/cros_config/cros_config.c
[modify] https://crrev.com/781f14ae777912759acf018f8ce398ec768ca1ce/platform/reef/reef.c

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 9 2017

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

commit 7a2f873caec648e078ea94250f179e2ba22e1d9f
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 09 21:55:02 2017

Move resolution of phandles into a function

Split this code out of cros_config_setup_sku() to reduce the size of that
function, before we add more code. Also correct the 'sub-model' name in
the test config.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/7a2f873caec648e078ea94250f179e2ba22e1d9f/lib/cros_config/cros_config.c
[modify] https://crrev.com/7a2f873caec648e078ea94250f179e2ba22e1d9f/tests/test_config.dts

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/170978eaadeb93a2538478a5db107ee8db63419a

commit 170978eaadeb93a2538478a5db107ee8db63419a
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 09 21:55:03 2017

Support multiple SKU maps

At present mosys only supports a single map. The binding calls for
multiple maps to be scanned.

Refactor the code to handle this. So far it does not support the SMBIOS
name search, so the result is not really any different. Future work will
add that feature.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/170978eaadeb93a2538478a5db107ee8db63419a/lib/cros_config/cros_config.c

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/a5e84a351f2a96d405cc88d0c30700d68ef58f7b

commit a5e84a351f2a96d405cc88d0c30700d68ef58f7b
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 09 21:55:03 2017

Support brand code being in the model

The brand code can be overridden by the submodel, but it is also possible
to use the model's brand code. Add support for this.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys
(later tests actually check this change)

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

[modify] https://crrev.com/a5e84a351f2a96d405cc88d0c30700d68ef58f7b/lib/cros_config/cros_config.c

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c

commit ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 09 21:55:03 2017

Support looking up SKU by SMBIOS name

Implement the full SKU lookup binding so that we can distinguish models
with the same SKU ID but a different SMBIOS name. This is needed for
reef-uni.

Also update the tests to cover the new cases.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c/include/lib/cros_config.h
[modify] https://crrev.com/ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c/lib/cros_config/cros_config.c
[modify] https://crrev.com/ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c/tests/test_config.dts
[modify] https://crrev.com/ef9a8041dbc1b45beca78704cc8faaa5b58a7f2c/tests/simple_tests.c

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/fcb417c4dd2552f11ab8fad17a45e6143ff464e2

commit fcb417c4dd2552f11ab8fad17a45e6143ff464e2
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 09 21:55:03 2017

Correct error return in cros_config_fdt_err()

This function should not return an FDT error. This is not useful and can
only confuse the caller. Return -1 instead.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/fcb417c4dd2552f11ab8fad17a45e6143ff464e2/lib/cros_config/cros_config.c

Running mosys on Nasher, it returns "Platform not supported". 
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/d3e5b30eb84b5712764e3750837ac8c19e1c34a1

commit d3e5b30eb84b5712764e3750837ac8c19e1c34a1
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 13 01:08:33 2017

Use combined SMBIOS/SKU ID for reef

With unified builds we can now use the cros_config lookup mechanism
instead of the various tables for reef, pyro, coral, etc. Update the code
to drop the old way.

This should be suitable for most future x86 machines and possible some
past ones also.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys

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

[modify] https://crrev.com/d3e5b30eb84b5712764e3750837ac8c19e1c34a1/platform/reef/reef.c

Project Member

Comment 20 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/b6bd0605079c647018b2c439f41446472ff626cf

commit b6bd0605079c647018b2c439f41446472ff626cf
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 13 01:08:44 2017

Set up the SKU information

With cros_config the SKU information is not actually linked into the mosys
data structure. Fix this.

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-coral --nodeps mosys
Manual test on robo

Change-Id: I77852b04c098f0145061db49c05857c5721c0239
Reviewed-on: https://chromium-review.googlesource.com/717258
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/b6bd0605079c647018b2c439f41446472ff626cf/platform/reef/reef.c

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/2d283c05bd855b28d0d0cd7be395e51c466fdb32

commit 2d283c05bd855b28d0d0cd7be395e51c466fdb32
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 16 21:14:01 2017

Fix an incorrect prototype

The header does not match the implementation.

See also  crbug.com/772533 

BUG= chromium:770183 
BRANCH=none
TEST=FEATURES=test emerge-reef-uni mosys
See no warnings introduced

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

[modify] https://crrev.com/2d283c05bd855b28d0d0cd7be395e51c466fdb32/include/lib/cros_config.h
[modify] https://crrev.com/2d283c05bd855b28d0d0cd7be395e51c466fdb32/lib/cros_config/cros_config.c

Comment 22 by sjg@chromium.org, Oct 17 2017

Status: Fixed (was: Started)
For unified builds, mosys now embeds the master configuration within itself. The internal tables it had are gone.

Sign in to add a comment