RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

From: Limonciello, Mario
Date: Tue Sep 01 2020 - 10:34:18 EST


>
> "A read-only attribute enumerating if a reboot is pending on any BIOS attribute
> change."
> does not really seem to make much sense. I guess what this is trying to say is:
>
> "This read-only attribute reads 1 if a reboot is necessary to apply pending BIOS
> attribute changes"?
>
> 0: All BIOS attributes setting are current
> 1: A reboot is necessary to get pending pending BIOS attribute
> changes applied
>
> Or some such. I'm not really happy with my own text either, but I think it
> better explains
> what this attribute is about then the original text, right ?

I think that text does read better, Divya and team will reword it.

<snip>

> > + display_name_language_code: A file that can be read to obtain
> > + the language code corresponding to the "display_name" of the <attr>
>
> This needs to be specified better, e.g. this needs to say that this is an
> ISO 639‑1 language code (or some other language-code specification)

Ack.

>
>
> > +
> > + modifier: A file that can be read to obtain attribute-level
> > + dependency rule which has to be met to configure <attr>
>
> What is the difference between modifier and value_modifier ? Also this need to
> be specified in more detail.

Ack.

>
> > +
> > + possible_value: A file that can be read to obtain the possible
> > + value of the <attr>
>
> This is an enum, so possible value_s_ ? I assume that for a enum this will list
> all possible values, this also needs to specify how the possible values will be
> separated (e.g. using semi-colons or newlines or ...).

Yes correct.

>
>
> > +
> > + value_modifier: A file that can be read to obtain value-level
> > + dependency on a possible value which has to be met to configure
> <attr>
> > +
> > +What: /sys/devices/platform/dell-wmi-
> sysman/attributes/integer/<attr>/
> > +Date: October 2020
> > +KernelVersion: 5.9
> > +Contact: Divya Bharathi <Divya.Bharathi@xxxxxxxx>,
> > + Mario Limonciello <mario.limonciello@xxxxxxxx>,
> > + Prasanth KSR <prasanth.ksr@xxxxxxxx>
> > +Description:
> > + This directory exposes interfaces for interaction with
> > + BIOS integer attributes.
> > +
> > + Integer attributes are settings that accept a range of
> > + numerical values for inputs. Each BIOS integer has a
> > + lower bound and an upper bound on the values that it can take.
> > +
> > + current_value: A file that can be read to obtain the current
> > + value of the <attr>
> > +
> > + This file can also be written to in order to update
> > + the value of an <attr>.
> > +
> > + default_value: A file that can be read to obtain the default
> > + value of the <attr>
> > +
> > + display_name: A file that can be read to obtain a user friendly
> > + description of the at <attr>
> > +
> > + display_name_language_code: A file that can be read to obtain
> > + the language code corresponding to the "display_name" of the <attr>
> > +
> > + lower_bound: A file that can be read to obtain the lower
> > + bound value of the <attr>
> > +
> > + modifier: A file that can be read to obtain attribute-level
> > + dependency rule which has to be met to configure <attr>
> > +
> > + scalar_increment: A file that can be read to obtain the
> > + resolution of the incremental value this attribute accepts.
> > +
> > + upper_bound: A file that can be read to obtain the upper
> > + bound value of the <attr>
>
> Are these integers or also possibly floats? I guess possibly also floats, right?
> Then at a minimum this should specify which decimal-separator is used (I assume
> we will go with the usual '.' as decimal separator).

In practice they're integers, but I don't see why they couldn't be floats.