phosphor-hwmon refactoring proposal

Brad Bishop bradleyb at fuzziesquirrel.com
Fri Oct 18 00:16:13 AEDT 2019


Hi everyone

I have a quick phosphor-hwmon change proposal I’d like to get feedback on.

One issue with phosphor-hwmon that has been brought up a couple times now  
is that it gets its configuration settings from a file with a path in the  
filesystem that mirrors the path to the hwmon device in the device tree.   
This is problematic because the device tree path is not required to be  
stable, so whenever things move around there, the config files all have to  
be moved.

Unfortunately there are > 200 config files like this scattered throughout  
the openbmc source tree.  So my proposal addresses the limitation in a way  
that allows users to move over to the new config file locations on their  
own schedule.

Presently phosphor-hwmon gets its configuration from the environment,  
provided by systemd’s EnvironmentFile option:

EnvironmentFile=/etc/default/obmc/hwmon/%I.conf
ExecStart=/usr/bin/phosphor-hwmon-readd -o %I

The proposal is simply to quit passing the configuration via the  
environment and instead look for a config file, the location specified via  
logic in the application, with a path like:

/usr/share/phosphor-hwmon/i2c/2-004c.conf

This is the path to the hwmon parent device relative to /sys/bus e.g.  
/sys/bus/i2c/devices/2-004c/.  All the logic to do this would be added to  
the application itself, the sole input being the /sys/devices/... path  
provided by udev.  libsystemd has an sd-device subsystem that could be used  
to do the /sys/devices/… -> /sys/bus/… traversal if that makes things any  
easier.

The new config file could keep the same format as today, or we could move  
it to json and parse it with nlohmann?  If json we could preserve the  
current flat configuration or have dictionaries?

While we are poking around in here, I’d also recommend swapping out the  
current helper script with a SYSTEMD_WANTS as systemd has been updated to  
handle templates in udev rules since this was all originally implemented.

Finally, to enabled a staged migration, the new unit file and udev rules  
would be installed with a new feature flag (e.g. meson option) in place of  
the old.

Please poke holes.  thanks!

-brad


More information about the openbmc mailing list