Re: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition

[Dan Williams]

On Wed, Aug 26, 2015 at 2:12 PM, Toshi Kani <toshi.kani@xxxxxx> wrote:
> On Wed, 2015-08-26 at 10:16 -0700, Dan Williams wrote:
>> On Wed, Aug 26, 2015 at 9:20 AM, Toshi Kani <toshi.kani@xxxxxx> wrote:
>> > ACPI 6.0 NFIT Memory Device State Flags in Table 5-129 defines
>> > bit 3 as follows.
>> >
>> >   Bit [3] set to 1 to indicate that the Memory Device is observed
>> >   to be not armed prior to OSPM hand off. A Memory Device is
>> >   considered armed if it is able to accept persistent writes.
>> >
>> > This bit is currently defined as ACPI_NFIT_MEM_ARMED, which can be
>> > confusing as if the Memory Device is armed when this bit is set.
>> >
>> > Change the name to ACPI_NFIT_MEM_NOT_ARMED per the spec.
>> >
>> > Signed-off-by: Toshi Kani <toshi.kani@xxxxxx>
>> > Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
>> > Cc: Bob Moore <robert.moore@xxxxxxxxx>
>> > Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> > ---
>> >  drivers/acpi/nfit.c              |    6 +++---
>> >  drivers/acpi/nfit.h              |    2 +-
>> >  include/acpi/actbl1.h            |    2 +-
>>
>> This file "include/acpi/actbl1.h" is owned by the ACPICA project so
>> any changes need to come through them.  But that said, I'm not sure we
>> need friendly names at this level.
>
> I think the name is misleading, but I agree with the process and this patch2
> can be dropped.  It'd be nice if the ACPICA project can pick it up later
> when they have a chance, though.
>
>> What I usually say about sysfs name changes to be more human friendly
>> is "sysfs is not a UI", i.e. it's not necessarily meant to be user
>> friendly.  As long as the names for the flags are distinct then
>> wrapping descriptive / accurate names around them is the role of
>> libndctl and userspace management software.
>>
>> Similar feedback for patch1 in the sense that I don't think we need to
>> update the sysfs naming.  For example the API to retrieve the state of
>> the "arm" flag in libndctl is ndctl_dimm_failed_arm().
>
> I agree that we do not want to change sysfs API for friendliness, and I
> understand that libndctl already consumes the strings...  But I think they
> can be confusing for the long run, i.e. the flags is likely extended for
> additional info, and more people may be looking at sysfs for the state.
>  It'd be a lot harder to change them later.

The starting premise though is that this will be nicer for scripts
that want to avoid the library.  Properly handling the async device
registration semantics of the libnvdimm-sysfs interface is hard to get
right in a script.  I'm trying my best to discourage raw use of sysfs
for this reason.  Small fixes to the names of flags seems to miss this
wider point.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/