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

[Toshi Kani]

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.

Thanks,
-Toshi
--
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/