New issue
Advanced search Search tips

Issue 783321 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Support powerd in cros_config

Project Member Reported by sjg@chromium.org, Nov 9 2017

Issue description

At present powerd keeps its read-only config in files that we install on the root filesystem 

https://cs.corp.google.com/chromeos_public/src/platform2/power_manager/common/prefs.cc

Some default prefs are here:

https://cs.corp.google.com/chromeos_public/src/platform2/power_manager/default_prefs/

Most of these are integers. We should support putting read-only preferences in the master config through a binding that supports existing preferences.

It should be fairly easy to add CrosConfig as another source for preferences in GetPrefStrings, for example.

One question is whether we should put the preferences in sub-nodes, like:

power {
   powerd {
      battery-stabilized-after {
         power-connected-ms = <123>;
         power-disconnected-ms = <234>;
         };
      keyboard-backlight {
         ais-steps = <2>;
         keep-on-ms = <10>;
      };
   };
};


Steps:
- Write up a little proposal doc (go/greendoc)
- Writing the binding / add to cros_config schema
- Implement in powerd
- Convert reef-uni

 

Comment 1 by lannm@google.com, Nov 16 2017

Design: go/powerd-cros-config
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21 2017

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

commit 6b2eb55c9d11404823eae71388a1a3fbd4876f94
Author: Lann Martin <lannm@chromium.org>
Date: Tue Nov 21 02:13:44 2017

power_manager: refactor prefs code to allow multiple source types

The prefs code already allowed for multiple directory sources, but we
want to add a new source type (in a future change). This change
introduces some abstraction in the form of PrefsSourceInterface and
PrefsStoreInterface, and moves the file-based pref code into
FilePrefsStore without changing overall functionality.

Also includes some miscellaneous modernization from review feedback.

BUG= chromium:783321 
TEST=cros_workon_make --board=reef-uni power_manager --test

Change-Id: I48af942e5a4f68834bd5e8584dbdd6369866dc07
Reviewed-on: https://chromium-review.googlesource.com/775781
Commit-Ready: Lann Martin <lannm@chromium.org>
Tested-by: Lann Martin <lannm@chromium.org>
Reviewed-by: Lann Martin <lannm@chromium.org>

[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/tools/power_supply_info.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/tools/dump_power_status.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/tools/backlight_tool.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/common/prefs.h
[add] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/common/file_prefs_store.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/common/prefs.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/powerd/main.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/power_manager.gyp
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/common/prefs_unittest.cc
[modify] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/tools/check_powerd_config.cc
[add] https://crrev.com/6b2eb55c9d11404823eae71388a1a3fbd4876f94/power_manager/common/file_prefs_store.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 30 2017

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

commit 24451c8cfc140f391b036a70c7301689e6d9dc3f
Author: Lann Martin <lannm@chromium.org>
Date: Thu Nov 30 09:13:03 2017

chromeos-config: add power schema

These new properties correspond to those power_manager preferences in
use by reef.

Added new PropFloat which validates float value ranges.

Made special-cased audio-type phandle following slightly less
special-cased to follow power-type as well.

Cleared up a little space/tab confusion.

BUG= chromium:783321 
TEST=./run_tests.sh

Change-Id: I89b461ff29ee7e374edec4b3186e617bcea8393e
Reviewed-on: https://chromium-review.googlesource.com/786074
Commit-Ready: Lann Martin <lannm@chromium.org>
Tested-by: Lann Martin <lannm@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/libcros_config/cros_config_unittest.cc
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/README.md
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/cros_config_host/validate_config_unittest.py
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/cros_config_host/validate_schema.py
[modify] https://crrev.com/24451c8cfc140f391b036a70c7301689e6d9dc3f/chromeos-config/cros_config_host/validate_config.py

Comment 5 by la...@chromium.org, Dec 4 2017

Owner: jclinton@chromium.org
This should be the last patch that needs to land here, fixing CrosConfig::Init in powerd: https://crrev.com/c/794990
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 5 2017

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

commit 6415a4c2d4909019cdc5e5c770630af3ac5fe58c
Author: Lann Martin <lannm@chromium.org>
Date: Tue Dec 05 14:25:01 2017

chromeos-config: store mosys platform details on disk

Prior to this change, CrosConfig::InitModel shelled out to mosys to read
platform id details. Since mosys can only run as root, this caused
InitModel to fail when run as non-root.

As a workaround, change InitModel to read platform details from
/run/mosys/platform_id, which gets populated from mosys during boot.

BUG= chromium:783321 
TEST=deploy chromeos-init and chromeos-config-tools to pyro;
     manually check power_manager logs for errors
CQ-DEPEND=CL:804795

Change-Id: I856f2c29fb31374333645552167c8789347db095
Reviewed-on: https://chromium-review.googlesource.com/794990
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Lann Martin <lannm@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6415a4c2d4909019cdc5e5c770630af3ac5fe58c/init/chromeos_startup
[modify] https://crrev.com/6415a4c2d4909019cdc5e5c770630af3ac5fe58c/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/6415a4c2d4909019cdc5e5c770630af3ac5fe58c/chromeos-config/libcros_config/cros_config.h

Status: Fixed (was: Untriaged)

Sign in to add a comment