New issue
Advanced search Search tips

Issue 798566 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

mosys cros_config support needs to check the platform name

Project Member Reported by sjg@chromium.org, Jan 2 2018

Issue description

At present cros_config_read_sku_info() checks the master config SKU table and returns a model that matches.

If the Fizz code calls cros_config_read_sku_info() with the Coral config, it will still match, since there is no check that the Coral config is for 'Fizz'. Then, we will use the Fizz platform info on coral.

This problem has been fixed by adding back in the for() loop to call probe_smbios(). But this is not really the correct way to do it. Instead, we should pass a list of valid platforms to cros_config_read_sku_info() so that it can do this check itself.

 
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 6 2018

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

commit 19cbc0da7ccce325ec7c147cd5423b3377483e94
Author: Simon Glass <sjg@chromium.org>
Date: Sat Jan 06 05:14:12 2018

mosys: Allow cros_config_read_sku_info() to be used safely

At present this function takes no notice of the platform that the tables
are intended for. Fix this by passing in a list of valid platforms. This
prevents (e.g.) Fizz tables being used on a Coral.

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

Change-Id: I0ae9f3ad8eca8f6d5764be1dbdb4ca723d39af4f
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847959
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>

[modify] https://crrev.com/19cbc0da7ccce325ec7c147cd5423b3377483e94/include/lib/cros_config.h
[modify] https://crrev.com/19cbc0da7ccce325ec7c147cd5423b3377483e94/lib/cros_config/cros_config.c
[modify] https://crrev.com/19cbc0da7ccce325ec7c147cd5423b3377483e94/tests/simple_tests.c
[modify] https://crrev.com/19cbc0da7ccce325ec7c147cd5423b3377483e94/platform/reef/reef.c
[modify] https://crrev.com/19cbc0da7ccce325ec7c147cd5423b3377483e94/platform/fizz/fizz.c

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 6 2018

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

commit 44f51d6dce16191194ab3ab812b2f2260904c1ea
Author: Simon Glass <sjg@chromium.org>
Date: Sat Jan 06 05:14:12 2018

mosys: Tidy up fizz and reef detection code

Now that the cros_config problem is fixed, we can drop the additional
checking.

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

Change-Id: Id80c089913f69fa0e6bd504456b2fe101caa865d
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847960
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/44f51d6dce16191194ab3ab812b2f2260904c1ea/platform/fizz/fizz.c
[modify] https://crrev.com/44f51d6dce16191194ab3ab812b2f2260904c1ea/platform/platform_list.c
[modify] https://crrev.com/44f51d6dce16191194ab3ab812b2f2260904c1ea/platform/reef/reef.c
[modify] https://crrev.com/44f51d6dce16191194ab3ab812b2f2260904c1ea/platform/fizz/sys.c

Comment 4 by sjg@chromium.org, Jan 29 2018

Status: Fixed (was: Started)
This fixes a bug that came to light in coral when grunt was added to mosys.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 6 2018

Labels: merge-merged-factory-coral-10122.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/bc8f8a33a5bb407a9ea525d45aea768434f0fad8

commit bc8f8a33a5bb407a9ea525d45aea768434f0fad8
Author: Simon Glass <sjg@chromium.org>
Date: Tue Feb 06 06:24:34 2018

mosys: Allow cros_config_read_sku_info() to be used safely

At present this function takes no notice of the platform that the tables
are intended for. Fix this by passing in a list of valid platforms. This
prevents (e.g.) Fizz tables being used on a Coral.

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

Change-Id: I0ae9f3ad8eca8f6d5764be1dbdb4ca723d39af4f
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847959
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
(cherry picked from commit 19cbc0da7ccce325ec7c147cd5423b3377483e94)
Reviewed-on: https://chromium-review.googlesource.com/903251
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Commit-Queue: Patrick Berny <pberny@chromium.org>
Tested-by: Patrick Berny <pberny@chromium.org>
Trybot-Ready: Patrick Berny <pberny@chromium.org>

[modify] https://crrev.com/bc8f8a33a5bb407a9ea525d45aea768434f0fad8/include/lib/cros_config.h
[modify] https://crrev.com/bc8f8a33a5bb407a9ea525d45aea768434f0fad8/lib/cros_config/cros_config.c
[modify] https://crrev.com/bc8f8a33a5bb407a9ea525d45aea768434f0fad8/tests/simple_tests.c
[modify] https://crrev.com/bc8f8a33a5bb407a9ea525d45aea768434f0fad8/platform/reef/reef.c
[modify] https://crrev.com/bc8f8a33a5bb407a9ea525d45aea768434f0fad8/platform/fizz/fizz.c

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 6 2018

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

commit b02f04111f9057c31bcfc44b2dd77625e5b53f31
Author: Simon Glass <sjg@chromium.org>
Date: Tue Feb 06 06:24:36 2018

mosys: Tidy up fizz and reef detection code

Now that the cros_config problem is fixed, we can drop the additional
checking.

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

Change-Id: Id80c089913f69fa0e6bd504456b2fe101caa865d
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847960
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
(cherry picked from commit 44f51d6dce16191194ab3ab812b2f2260904c1ea)
Reviewed-on: https://chromium-review.googlesource.com/903252
Commit-Queue: Patrick Berny <pberny@chromium.org>
Tested-by: Patrick Berny <pberny@chromium.org>
Trybot-Ready: Patrick Berny <pberny@chromium.org>

[modify] https://crrev.com/b02f04111f9057c31bcfc44b2dd77625e5b53f31/platform/fizz/fizz.c
[modify] https://crrev.com/b02f04111f9057c31bcfc44b2dd77625e5b53f31/platform/platform_list.c
[modify] https://crrev.com/b02f04111f9057c31bcfc44b2dd77625e5b53f31/platform/reef/reef.c
[modify] https://crrev.com/b02f04111f9057c31bcfc44b2dd77625e5b53f31/platform/fizz/sys.c

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 27 2018

Labels: merge-merged-factory-fizz-10167.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/d589e4dce125981dc51d0b360503fdab2ec95b82

commit d589e4dce125981dc51d0b360503fdab2ec95b82
Author: Simon Glass <sjg@chromium.org>
Date: Tue Mar 27 01:14:55 2018

mosys: Allow cros_config_read_sku_info() to be used safely

At present this function takes no notice of the platform that the tables
are intended for. Fix this by passing in a list of valid platforms. This
prevents (e.g.) Fizz tables being used on a Coral.

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

Change-Id: I0ae9f3ad8eca8f6d5764be1dbdb4ca723d39af4f
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847959
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
(cherry picked from commit 19cbc0da7ccce325ec7c147cd5423b3377483e94)
Reviewed-on: https://chromium-review.googlesource.com/981209
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/d589e4dce125981dc51d0b360503fdab2ec95b82/include/lib/cros_config.h
[modify] https://crrev.com/d589e4dce125981dc51d0b360503fdab2ec95b82/lib/cros_config/cros_config.c
[modify] https://crrev.com/d589e4dce125981dc51d0b360503fdab2ec95b82/tests/simple_tests.c
[modify] https://crrev.com/d589e4dce125981dc51d0b360503fdab2ec95b82/platform/reef/reef.c
[modify] https://crrev.com/d589e4dce125981dc51d0b360503fdab2ec95b82/platform/fizz/fizz.c

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 27 2018

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

commit 7d9a19f1179b83dde31f0b8b1ac3b2536ed97eb9
Author: Simon Glass <sjg@chromium.org>
Date: Tue Mar 27 01:14:56 2018

mosys: Tidy up fizz and reef detection code

Now that the cros_config problem is fixed, we can drop the additional
checking.

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

Change-Id: Id80c089913f69fa0e6bd504456b2fe101caa865d
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/847960
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
(cherry picked from commit 44f51d6dce16191194ab3ab812b2f2260904c1ea)
Reviewed-on: https://chromium-review.googlesource.com/981210
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/7d9a19f1179b83dde31f0b8b1ac3b2536ed97eb9/platform/fizz/fizz.c
[modify] https://crrev.com/7d9a19f1179b83dde31f0b8b1ac3b2536ed97eb9/platform/platform_list.c
[modify] https://crrev.com/7d9a19f1179b83dde31f0b8b1ac3b2536ed97eb9/platform/reef/reef.c
[modify] https://crrev.com/7d9a19f1179b83dde31f0b8b1ac3b2536ed97eb9/platform/fizz/sys.c

Sign in to add a comment