dbus-sensors:hwmontemp: additional attribute proposal

Jason Ling jasonling at google.com
Sat Aug 8 04:29:45 AEST 2020


>
> What would happen if something wasn't in the approved devices list?
> Print a warning? Assert? I don't have any problems with this approach.
>
I think currently in PSUSensors if the name of the device isn't in
pmBusNames
<https://github.com/openbmc/dbus-sensors/blob/master/src/PSUSensorMain.cpp#L59>
then PSUSensors refuses to monitor that particular device (
https://github.com/openbmc/dbus-sensors/blob/master/src/PSUSensorMain.cpp#L269
).

It would be nice if this list could be (in the future) extended to all
> sensor types.

Sure, I'll start with PSUSensors first.


A quick word about the original topic of this thread; we picked
hwmontemp sensors where possible because we've seen performance issues for
PSUSensors (could be N seconds before Sensor.Value gets updated to dbus).
This has undesirable implications for PID loops..
thought was that spreading temp sensors over into hwmontemp sensor where
possible would help mitigate the performance issue until we can figure out
a better long term solution.


On Fri, Aug 7, 2020 at 11:17 AM James Feist <james.feist at linux.intel.com>
wrote:

> On 8/7/2020 11:04 AM, Jason Ling wrote:
> >
> >     Yeah that's what I meant, a generic PSUSensor Type with a field
> called
> >     'Export' or something that EM can use to get the Export type.
> >
> > Sure, I think that handles the EM side of things.
> >
> >     I'm conflicted on that.  Part of the reason that list is there, and
> >     not in the config files directly, is to convey that those are the
> >     types that have been tested to work correctly, and the types that the
> >     config files "promise" to work sanely.  The other thing is, if you've
> >     done the testing, adding to that list is (should be) relatively
> >     trivial and straightforward.  Opening up that list to runtime parsing
> >     opens a lot of security questions I'm not prepared to answer.
> >
> > Sure, let's scrap runtime parsing and go for something far simpler.
> > (1) have a base type, devices list that represents the approved device
> list.
> > (2) have an empty extended type, device list that represents user
> > specified extensions.
> > (3) factor these out into separate files and provide a method that
> > returns the union of the base and extended types/devices.
> >
> > now we have a base type/device list that contains supported guaranteed
> > devices and another extended type/device list that is easier to maintain
> > patches for.
> > I think that's a rather small change and accomplishes what I'd like
> > while preserving the intent of keeping a list of supported types/devices.
> > What do you think?
>
> What would happen if something wasn't in the approved devices list?
> Print a warning? Assert? I don't have any problems with this approach.
> It would be nice if this list could be (in the future) extended to all
> sensor types.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20200807/668daf57/attachment.htm>


More information about the openbmc mailing list