9:20 a.m.
ACPI 6.0 NFIT Memory Device State Flags in Table 5-129 defines
NVDIMM status. User can check the NVDIMM status from sysfs and
dmesg output, but the output can be confusing at this point.
Patch 01 clarifies the output strings from the sysfs and dmesg.
Patch 02 clarifies the define name of not-arm bit in the flags.
This patchset is based on libnvdimm-pending of the nvdimm tree.
---
Toshi Kani (2):
1/2 nfit: Clarify memory device state flags strings
2/2 acpica/nfit: Rename not-armed bit definition
---
drivers/acpi/nfit.c | 25 +++++++++++++------------
drivers/acpi/nfit.h | 2 +-
include/acpi/actbl1.h | 2 +-
tools/testing/nvdimm/test/nfit.c | 2 +-
4 files changed, 16 insertions(+), 15 deletions(-)
9:20 a.m.
New subject: [PATCH 1/2]: nfit: Clarify memory device state flags strings
ACPI 6.0 NFIT Memory Device State Flags in Table 5-129 defines
NVDIMM status as follows. These bits indicate multiple info,
such as failures, pending event, and capability.
Bit [0] set to 1 to indicate that the previous SAVE to the
Memory Device failed.
Bit [1] set to 1 to indicate that the last RESTORE from the
Memory Device failed.
Bit [2] set to 1 to indicate that platform flush of data to
Memory Device failed. As a result, the restored data content
may be inconsistent even if SAVE and RESTORE do not indicate
failure.
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.
Bit [4] set to 1 to indicate that the Memory Device observed
SMART and health events prior to OSPM handoff.
Bit [5] is set to 1 to indicate that platform firmware is
enabled to notify OSPM on SMART and health events related to
the memory device using Notify codes as specified in Section
5.6.6.
/sys/bus/nd/devices/nmemX/nfit/flags shows this flags info.
The output strings associated with the bits are "save", "restore",
"smart", etc., which can be confusing as they may be interpreted
as positive status, i.e. save succeeded.
Change the strings to be more descriptive per the ACPI spec.
Also add a string to bit 5 for completeness.
Change also the dev_info() message in acpi_nfit_register_dimms()
to be consistent with the sysfs flags strings.
Reported-by: Robert Elliott <elliott(a)hp.com>
Signed-off-by: Toshi Kani <toshi.kani(a)hp.com>
Cc: Dan Williams <dan.j.williams(a)intel.com>
---
drivers/acpi/nfit.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index c3fe206..6993ff2 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -701,12 +701,13 @@ static ssize_t flags_show(struct device *dev,
{
u16 flags = to_nfit_memdev(dev)->flags;
- return sprintf(buf, "%s%s%s%s%s\n",
- flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save " : "",
- flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore " : "",
- flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush " : "",
- flags & ACPI_NFIT_MEM_ARMED ? "arm " : "",
- flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart " : "");
+ return sprintf(buf, "%s%s%s%s%s%s\n",
+ flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
+ flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail " : "",
+ flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " : "",
+ flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "",
+ flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart_event " : "",
+ flags & ACPI_NFIT_MEM_HEALTH_ENABLED ? "notify_enabled " :
"");
}
static DEVICE_ATTR_RO(flags);
@@ -834,11 +835,11 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc
*acpi_desc)
continue;
dev_info(acpi_desc->dev, "%s: failed: %s%s%s%s\n",
- nvdimm_name(nvdimm),
- mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save " : "",
- mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore " : "",
- mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush " : "",
- mem_flags & ACPI_NFIT_MEM_ARMED ? "arm " : "");
+ nvdimm_name(nvdimm),
+ mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
+ mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail
":"",
+ mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " : "",
+ mem_flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "");
}
8:07 p.m.
New subject: [PATCH 1/2]: nfit: Clarify memory device state flags strings
On Wed, Aug 26, 2015 at 10:20:23AM -0600, Toshi Kani wrote:
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index c3fe206..6993ff2 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -701,12 +701,13 @@ static ssize_t flags_show(struct device *dev,
{
u16 flags = to_nfit_memdev(dev)->flags;
- return sprintf(buf, "%s%s%s%s%s\n",
- flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save " : "",
- flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore " : "",
- flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush " : "",
- flags & ACPI_NFIT_MEM_ARMED ? "arm " : "",
- flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart " : "");
+ return sprintf(buf, "%s%s%s%s%s%s\n",
+ flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
+ flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail " : "",
+ flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " : "",
+ flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "",
Assuming we do want to update these strings to be more friendly, "not_armed"
probably makes more sense than "not_arm". Also applies to the 2nd hunk below.
+ flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart_event
" : "",
+ flags & ACPI_NFIT_MEM_HEALTH_ENABLED ? "notify_enabled " :
"");
}
static DEVICE_ATTR_RO(flags);
@@ -834,11 +835,11 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc
*acpi_desc)
continue;
dev_info(acpi_desc->dev, "%s: failed: %s%s%s%s\n",
- nvdimm_name(nvdimm),
- mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save " : "",
- mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore " : "",
- mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush " : "",
- mem_flags & ACPI_NFIT_MEM_ARMED ? "arm " : "");
+ nvdimm_name(nvdimm),
+ mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
+ mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail
":"",
+ mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " :
"",
+ mem_flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "");
While you're in here, is there a reason not to include the last two flags
(smart_event and notify_enabled) in this dev_info() output?
7:18 a.m.
New subject: [PATCH 1/2]: nfit: Clarify memory device state flags strings
On Wed, 2015-08-26 at 21:07 -0600, Ross Zwisler wrote:
On Wed, Aug 26, 2015 at 10:20:23AM -0600, Toshi Kani wrote:
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index c3fe206..6993ff2 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -701,12 +701,13 @@ static ssize_t flags_show(struct device *dev,
> {
> u16 flags = to_nfit_memdev(dev)->flags;
>
> - return sprintf(buf, "%s%s%s%s%s\n",
> - flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save " :
> "",
> - flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore
> " : "",
> - flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush " :
> "",
> - flags & ACPI_NFIT_MEM_ARMED ? "arm " : "",
> - flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart
> " : "");
> + return sprintf(buf, "%s%s%s%s%s%s\n",
> + flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
> + flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail "
> : "",
> + flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " :
> "",
> + flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "",
Assuming we do want to update these strings to be more friendly,
"not_armed" probably makes more sense than
"not_arm". Also applies to the
2nd hunk below.
Agreed. (Will update if this patch gets ever resurrected. :-)
> + flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart_event
" > > : "",
> + flags & ACPI_NFIT_MEM_HEALTH_ENABLED ? "notify_enabled
> " : "");
> }
> static DEVICE_ATTR_RO(flags);
>
> @@ -834,11 +835,11 @@ static int acpi_nfit_register_dimms(struct
> acpi_nfit_desc *acpi_desc)
> continue;
>
> dev_info(acpi_desc->dev, "%s: failed: %s%s%s%s\n",
> - nvdimm_name(nvdimm),
> - mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save "
> : "",
> - mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ?
> "restore " : "",
> - mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush
> " : "",
> - mem_flags & ACPI_NFIT_MEM_ARMED ? "arm " : "");
> + nvdimm_name(nvdimm),
> + mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail "
> : "",
> + mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ?
> "restore_fail ":"",
> + mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail
> " : "",
> + mem_flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "");
While you're in here, is there a reason not to include the last two flags
(smart_event and notify_enabled) in this dev_info() output?
This dev_info() logs any failure in NVDIMM, and the last two flags are not
failure conditions.
Thanks,
-Toshi
11:57 a.m.
New subject: [PATCH 1/2]: nfit: Clarify memory device state flags strings
On Wed, Aug 26, 2015 at 9:20 AM, Toshi Kani <toshi.kani(a)hp.com> wrote:
ACPI 6.0 NFIT Memory Device State Flags in Table 5-129 defines
NVDIMM status as follows. These bits indicate multiple info,
such as failures, pending event, and capability.
Bit [0] set to 1 to indicate that the previous SAVE to the
Memory Device failed.
Bit [1] set to 1 to indicate that the last RESTORE from the
Memory Device failed.
Bit [2] set to 1 to indicate that platform flush of data to
Memory Device failed. As a result, the restored data content
may be inconsistent even if SAVE and RESTORE do not indicate
failure.
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.
Bit [4] set to 1 to indicate that the Memory Device observed
SMART and health events prior to OSPM handoff.
Bit [5] is set to 1 to indicate that platform firmware is
enabled to notify OSPM on SMART and health events related to
the memory device using Notify codes as specified in Section
5.6.6.
/sys/bus/nd/devices/nmemX/nfit/flags shows this flags info.
The output strings associated with the bits are "save", "restore",
"smart", etc., which can be confusing as they may be interpreted
as positive status, i.e. save succeeded.
Change the strings to be more descriptive per the ACPI spec.
Also add a string to bit 5 for completeness.
Ok I'm going to push this upstream for 4.2 with the "not_armed" fixup
that Ross suggested, but I'll defer adding bit5 since that is separate
from the urgent fix to get the polarities properly reflected and we
can add support for it later.
12:02 p.m.
New subject: [PATCH 1/2]: nfit: Clarify memory device state flags strings
On Thu, 2015-08-27 at 11:57 -0700, Dan Williams wrote:
On Wed, Aug 26, 2015 at 9:20 AM, Toshi Kani <toshi.kani(a)hp.com>
wrote:
> ACPI 6.0 NFIT Memory Device State Flags in Table 5-129 defines
> NVDIMM status as follows. These bits indicate multiple info,
> such as failures, pending event, and capability.
>
> Bit [0] set to 1 to indicate that the previous SAVE to the
> Memory Device failed.
> Bit [1] set to 1 to indicate that the last RESTORE from the
> Memory Device failed.
> Bit [2] set to 1 to indicate that platform flush of data to
> Memory Device failed. As a result, the restored data content
> may be inconsistent even if SAVE and RESTORE do not indicate
> failure.
> 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.
> Bit [4] set to 1 to indicate that the Memory Device observed
> SMART and health events prior to OSPM handoff.
> Bit [5] is set to 1 to indicate that platform firmware is
> enabled to notify OSPM on SMART and health events related to
> the memory device using Notify codes as specified in Section
> 5.6.6.
>
> /sys/bus/nd/devices/nmemX/nfit/flags shows this flags info.
> The output strings associated with the bits are "save",
"restore",
> "smart", etc., which can be confusing as they may be interpreted
> as positive status, i.e. save succeeded.
>
> Change the strings to be more descriptive per the ACPI spec.
> Also add a string to bit 5 for completeness.
Ok I'm going to push this upstream for 4.2 with the "not_armed" fixup
that Ross suggested, but I'll defer adding bit5 since that is separate
from the urgent fix to get the polarities properly reflected and we
can add support for it later.
Great!! Thanks Dan! (and sorry for making more work to you...)
-Toshi
9:20 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
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(a)hp.com>
Cc: Dan Williams <dan.j.williams(a)intel.com>
Cc: Bob Moore <robert.moore(a)intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
---
drivers/acpi/nfit.c | 6 +++---
drivers/acpi/nfit.h | 2 +-
include/acpi/actbl1.h | 2 +-
tools/testing/nvdimm/test/nfit.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 6993ff2..1eb3654 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -705,7 +705,7 @@ static ssize_t flags_show(struct device *dev,
flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail " : "",
flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " : "",
- flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "",
+ flags & ACPI_NFIT_MEM_NOT_ARMED ? "not_arm " : "",
flags & ACPI_NFIT_MEM_HEALTH_OBSERVED ? "smart_event " : "",
flags & ACPI_NFIT_MEM_HEALTH_ENABLED ? "notify_enabled " :
"");
}
@@ -815,7 +815,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
flags |= NDD_ALIASING;
mem_flags = __to_nfit_memdev(nfit_mem)->flags;
- if (mem_flags & ACPI_NFIT_MEM_ARMED)
+ if (mem_flags & ACPI_NFIT_MEM_NOT_ARMED)
flags |= NDD_UNARMED;
rc = acpi_nfit_add_dimm(acpi_desc, nfit_mem, device_handle);
@@ -839,7 +839,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? "save_fail " : "",
mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? "restore_fail
":"",
mem_flags & ACPI_NFIT_MEM_FLUSH_FAILED ? "flush_fail " : "",
- mem_flags & ACPI_NFIT_MEM_ARMED ? "not_arm " : "");
+ mem_flags & ACPI_NFIT_MEM_NOT_ARMED ? "not_arm " : "");
}
diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
index f2c2bb7..de4a00d 100644
--- a/drivers/acpi/nfit.h
+++ b/drivers/acpi/nfit.h
@@ -24,7 +24,7 @@
#define UUID_NFIT_DIMM "4309ac30-0d11-11e4-9191-0800200c9a66"
#define ACPI_NFIT_MEM_FAILED_MASK (ACPI_NFIT_MEM_SAVE_FAILED \
| ACPI_NFIT_MEM_RESTORE_FAILED | ACPI_NFIT_MEM_FLUSH_FAILED \
- | ACPI_NFIT_MEM_ARMED)
+ | ACPI_NFIT_MEM_NOT_ARMED)
enum nfit_uuids {
NFIT_SPA_VOLATILE,
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index fcd5709..238754e 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -1012,7 +1012,7 @@ struct acpi_nfit_memory_map {
#define ACPI_NFIT_MEM_SAVE_FAILED (1) /* 00: Last SAVE to Memory Device failed */
#define ACPI_NFIT_MEM_RESTORE_FAILED (1<<1) /* 01: Last RESTORE from Memory
Device failed */
#define ACPI_NFIT_MEM_FLUSH_FAILED (1<<2) /* 02: Platform flush failed */
-#define ACPI_NFIT_MEM_ARMED (1<<3) /* 03: Memory Device observed to be
not armed */
+#define ACPI_NFIT_MEM_NOT_ARMED (1<<3) /* 03: Memory Device observed to be
not armed */
#define ACPI_NFIT_MEM_HEALTH_OBSERVED (1<<4) /* 04: Memory Device observed
SMART/health events */
#define ACPI_NFIT_MEM_HEALTH_ENABLED (1<<5) /* 05: SMART/health events enabled
*/
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 28dba91..9495249 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -998,7 +998,7 @@ static void nfit_test1_setup(struct nfit_test *t)
memdev->interleave_ways = 1;
memdev->flags = ACPI_NFIT_MEM_SAVE_FAILED | ACPI_NFIT_MEM_RESTORE_FAILED
| ACPI_NFIT_MEM_FLUSH_FAILED | ACPI_NFIT_MEM_HEALTH_OBSERVED
- | ACPI_NFIT_MEM_ARMED;
+ | ACPI_NFIT_MEM_NOT_ARMED;
offset += sizeof(*memdev);
/* dcr-descriptor0 */
10:16 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, Aug 26, 2015 at 9:20 AM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com>
Cc: Dan Williams <dan.j.williams(a)intel.com>
Cc: Bob Moore <robert.moore(a)intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
---
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.
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().
12:59 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On 8/26/2015 1:16 PM, Dan Williams wrote:
On Wed, Aug 26, 2015 at 9:20 AM, Toshi Kani <toshi.kani(a)hp.com>
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(a)hp.com>
> Cc: Dan Williams <dan.j.williams(a)intel.com>
> Cc: Bob Moore <robert.moore(a)intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> ---
> 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.
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.
I think there's a difference between unfriendly and misleading or confusing.
If names didn't matter at all we could just call them bit0, bit1, bit2,...
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().
It would be so nice for scripts and humans if the sysfs names made as
much sense.
-- ljk
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm(a)lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
2:12 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
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(a)hp.com>
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(a)hp.com>
> Cc: Dan Williams <dan.j.williams(a)intel.com>
> Cc: Bob Moore <robert.moore(a)intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> ---
> 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
2:30 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, Aug 26, 2015 at 2:12 PM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com> 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(a)hp.com>
> > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > Cc: Bob Moore <robert.moore(a)intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > ---
> > 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.
2:44 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, 2015-08-26 at 14:30 -0700, Dan Williams wrote:
On Wed, Aug 26, 2015 at 2:12 PM, Toshi Kani <toshi.kani(a)hp.com>
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(a)hp.com> 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(a)hp.com>
> > > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > > Cc: Bob Moore <robert.moore(a)intel.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > > ---
> > > 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.
Okay, I guess I will have to jump on the bandwagon and discourage people to
look at sysfs... ;-P
Thanks,
-Toshi
3 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, Aug 26, 2015 at 2:44 PM, Toshi Kani <toshi.kani(a)hp.com> wrote:
On Wed, 2015-08-26 at 14:30 -0700, Dan Williams wrote:
> On Wed, Aug 26, 2015 at 2:12 PM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com>
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(a)hp.com>
> > > > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > > > Cc: Bob Moore <robert.moore(a)intel.com>
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > > > ---
> > > > 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.
Okay, I guess I will have to jump on the bandwagon and discourage people to
look at sysfs... ;-P
That said, I'm not opposed to looking at something like Python-binding
for libndctl to make scripting easier.
7:43 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On 8/26/2015 6:00 PM, Dan Williams wrote:
On Wed, Aug 26, 2015 at 2:44 PM, Toshi Kani <toshi.kani(a)hp.com>
wrote:
> On Wed, 2015-08-26 at 14:30 -0700, Dan Williams wrote:
>> On Wed, Aug 26, 2015 at 2:12 PM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com>
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(a)hp.com>
>>>>> Cc: Dan Williams <dan.j.williams(a)intel.com>
>>>>> Cc: Bob Moore <robert.moore(a)intel.com>
>>>>> Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
>>>>> ---
>>>>> 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.
>
> Okay, I guess I will have to jump on the bandwagon and discourage people to
> look at sysfs... ;-P
>
That said, I'm not opposed to looking at something like Python-binding
for libndctl to make scripting easier.
I don't see why we can't fix the names so they make sense now before there
is hardware in the market. People doing testing and debugging look at stuff
in /sys and they write their own scripts too, not necessarily in python.
If they only make sense to someone using your library, I think we've missed the
mark. Toshi is reacting to feedback we're getting from people are starting
to test this stuff.
-- ljk
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
8:30 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Thu, Aug 27, 2015 at 7:43 AM, Linda Knippers <linda.knippers(a)hp.com> wrote:
I don't see why we can't fix the names so they make sense now
before there
is hardware in the market. People doing testing and debugging look at stuff
in /sys and they write their own scripts too, not necessarily in python.
The practical concern at this point is that it is too late in the 4.2
development cycle, in my opinion, to push for a cosmetic change like
this. We also have versions of libndctl starting to leak into
distributions. Changing this in 4.3 means breaking the ABI from 4.2
and managing both ways in future versions of the library as well as
getting all distributions to update. Not insurmountable, but also
something I don't want to take on just for a few characters in a sysfs
file. I think this is better fixed with documentation which I still
owe to the the Documentation/ABI/ directory.
8:35 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On 8/27/2015 11:30 AM, Dan Williams wrote:
On Thu, Aug 27, 2015 at 7:43 AM, Linda Knippers
<linda.knippers(a)hp.com> wrote:
> I don't see why we can't fix the names so they make sense now before there
> is hardware in the market. People doing testing and debugging look at stuff
> in /sys and they write their own scripts too, not necessarily in python.
The practical concern at this point is that it is too late in the 4.2
development cycle, in my opinion, to push for a cosmetic change like
this. We also have versions of libndctl starting to leak into
distributions. Changing this in 4.3 means breaking the ABI from 4.2
and managing both ways in future versions of the library as well as
getting all distributions to update. Not insurmountable, but also
something I don't want to take on just for a few characters in a sysfs
file. I think this is better fixed with documentation which I still
owe to the the Documentation/ABI/ directory.
Is there any distribution that is going to enable this with 4.2? I know
we're using it for testing now but there is still quite a bit of work
queued up for 4.3 and more still left to be done.
-- ljk
8:54 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Thu, Aug 27, 2015 at 8:35 AM, Linda Knippers <linda.knippers(a)hp.com> wrote:
On 8/27/2015 11:30 AM, Dan Williams wrote:
> On Thu, Aug 27, 2015 at 7:43 AM, Linda Knippers <linda.knippers(a)hp.com> wrote:
>> I don't see why we can't fix the names so they make sense now before
there
>> is hardware in the market. People doing testing and debugging look at stuff
>> in /sys and they write their own scripts too, not necessarily in python.
>
> The practical concern at this point is that it is too late in the 4.2
> development cycle, in my opinion, to push for a cosmetic change like
> this. We also have versions of libndctl starting to leak into
> distributions. Changing this in 4.3 means breaking the ABI from 4.2
> and managing both ways in future versions of the library as well as
> getting all distributions to update. Not insurmountable, but also
> something I don't want to take on just for a few characters in a sysfs
> file. I think this is better fixed with documentation which I still
> owe to the the Documentation/ABI/ directory.
Is there any distribution that is going to enable this with 4.2? I know
we're using it for testing now but there is still quite a bit of work
queued up for 4.3 and more still left to be done.
I'm not happy that this is confusing folks, and it is unfortunate that
the polarity is reversed.
However, I'm more concerned about the fact that I'd be knowingly
breaking ABI from 4.2 to 4.3. I originally thought "no one" was using
e820 type-12 for enumerating nvdimm resources, but I got shouted down
when trying to convert that exclusively to the ACPI 6 definition. The
Linus edict of "we don't break released ABI" is ringing in my ears.
The current name is also consistent with the current / released ACPICA
definition.
9:32 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On 8/27/2015 11:54 AM, Dan Williams wrote:
On Thu, Aug 27, 2015 at 8:35 AM, Linda Knippers
<linda.knippers(a)hp.com> wrote:
> On 8/27/2015 11:30 AM, Dan Williams wrote:
>> On Thu, Aug 27, 2015 at 7:43 AM, Linda Knippers <linda.knippers(a)hp.com>
wrote:
>>> I don't see why we can't fix the names so they make sense now before
there
>>> is hardware in the market. People doing testing and debugging look at stuff
>>> in /sys and they write their own scripts too, not necessarily in python.
>>
>> The practical concern at this point is that it is too late in the 4.2
>> development cycle, in my opinion, to push for a cosmetic change like
>> this. We also have versions of libndctl starting to leak into
>> distributions. Changing this in 4.3 means breaking the ABI from 4.2
>> and managing both ways in future versions of the library as well as
>> getting all distributions to update. Not insurmountable, but also
>> something I don't want to take on just for a few characters in a sysfs
>> file. I think this is better fixed with documentation which I still
>> owe to the the Documentation/ABI/ directory.
>
> Is there any distribution that is going to enable this with 4.2? I know
> we're using it for testing now but there is still quite a bit of work
> queued up for 4.3 and more still left to be done.
I'm not happy that this is confusing folks, and it is unfortunate that
the polarity is reversed.
However, I'm more concerned about the fact that I'd be knowingly
breaking ABI from 4.2 to 4.3. I originally thought "no one" was using
e820 type-12 for enumerating nvdimm resources, but I got shouted down
when trying to convert that exclusively to the ACPI 6 definition. The
Linus edict of "we don't break released ABI" is ringing in my ears.
Maybe not too late for 4.2 then. They're just string changes.
The current name is also consistent with the current / released
ACPICA
definition.
They seem to be open to fixing it.
I know this seems like a nit but we've going to live with this stuff for
a long time.
-- ljk
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
10:04 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Thu, Aug 27, 2015 at 9:32 AM, Linda Knippers <linda.knippers(a)hp.com> wrote:
I know this seems like a nit but we've going to live with this
stuff for
a long time.
Along those lines, Bob just came by my office and gave me a decisive
argument that I think trumps my ABI concerns, and one I'd be willing
to argue for 4.2 inclusion. Some BIOS implementers may do the work to
get the polarity correct others may just read the flag name and get it
wrong. Once that happens the field loses all meaning regardless of
whether Linux interprets it correctly.
10:09 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On 8/27/2015 1:04 PM, Dan Williams wrote:
On Thu, Aug 27, 2015 at 9:32 AM, Linda Knippers
<linda.knippers(a)hp.com> wrote:
> I know this seems like a nit but we've going to live with this stuff for
> a long time.
Along those lines, Bob just came by my office and gave me a decisive
argument that I think trumps my ABI concerns, and one I'd be willing
to argue for 4.2 inclusion. Some BIOS implementers may do the work to
get the polarity correct others may just read the flag name and get it
wrong. Once that happens the field loses all meaning regardless of
whether Linux interprets it correctly.
BIOS developers, BIOS testers, and the OS folks working with those teams
will be grateful.
Thanks,
-- ljk
4:16 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, Aug 26, 2015 at 11:12 PM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com> 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(a)hp.com>
> > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > Cc: Bob Moore <robert.moore(a)intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > ---
> > 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.
A good way to cause that to happen would be to send a patch to the
ACPICA development list + maintainers as per MAINTAINERS.
Thanks,
Rafael
4:29 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Thu, 2015-08-27 at 01:16 +0200, Rafael J. Wysocki wrote:
On Wed, Aug 26, 2015 at 11:12 PM, Toshi Kani
<toshi.kani(a)hp.com> 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(a)hp.com> 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(a)hp.com>
> > > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > > Cc: Bob Moore <robert.moore(a)intel.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > > ---
> > > 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.
A good way to cause that to happen would be to send a patch to the
ACPICA development list + maintainers as per MAINTAINERS.
Oh, I see. I did run get_maintainer.pl for this patch, but devel(a)acpica.org
did not come out in output... So, I did not realize this email list.
Thanks for the suggestion!
-Toshi
4:35 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, 2015-08-26 at 17:29 -0600, Toshi Kani wrote:
On Thu, 2015-08-27 at 01:16 +0200, Rafael J. Wysocki wrote:
> On Wed, Aug 26, 2015 at 11:12 PM, Toshi Kani <toshi.kani(a)hp.com> 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(a)hp.com>
> > > 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(a)hp.com>
> > > > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > > > Cc: Bob Moore <robert.moore(a)intel.com>
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > > > ---
> > > > 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.
>
> A good way to cause that to happen would be to send a patch to the
> ACPICA development list + maintainers as per MAINTAINERS.
Oh, I see. I did run get_maintainer.pl for this patch, but
devel(a)acpica.org did not come out in output... So, I did not realize this
email list.
Sorry, it was listed in the output. I was simply blinded... :-(
$ scripts/get_maintainer.pl patches-nd-flags/02*
:
linux-nvdimm(a)lists.01.org (open list:LIBNVDIMM BLK: MMIO-APERTURE DRIVER)
linux-acpi(a)vger.kernel.org (open list:ACPI)
linux-kernel(a)vger.kernel.org (open list)
devel(a)acpica.org (open list:ACPI COMPONENT ARCHITECTURE (ACPICA))
Thanks!
-Toshi
6:56 p.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
I don’t have any problem changing this in ACPICA if/when you all agree.
-----Original Message-----
From: Toshi Kani [mailto:toshi.kani@hp.com]
Sent: Wednesday, August 26, 2015 4:36 PM
To: Rafael J. Wysocki
Cc: Williams, Dan J; Wysocki, Rafael J; Moore, Robert; linux-
nvdimm(a)lists.01.org; Linux ACPI; linux-kernel(a)vger.kernel.org; Elliott,
Robert (Server Storage)
Subject: Re: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Wed, 2015-08-26 at 17:29 -0600, Toshi Kani wrote:
> On Thu, 2015-08-27 at 01:16 +0200, Rafael J. Wysocki wrote:
> > On Wed, Aug 26, 2015 at 11:12 PM, Toshi Kani <toshi.kani(a)hp.com>
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(a)hp.com>
> > > > 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(a)hp.com>
> > > > > Cc: Dan Williams <dan.j.williams(a)intel.com>
> > > > > Cc: Bob Moore <robert.moore(a)intel.com>
> > > > > Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
> > > > > ---
> > > > > 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.
> >
> > A good way to cause that to happen would be to send a patch to the
> > ACPICA development list + maintainers as per MAINTAINERS.
>
> Oh, I see. I did run get_maintainer.pl for this patch, but
> devel(a)acpica.org did not come out in output... So, I did not realize
> this email list.
Sorry, it was listed in the output. I was simply blinded... :-(
$ scripts/get_maintainer.pl patches-nd-flags/02*
:
linux-nvdimm(a)lists.01.org (open list:LIBNVDIMM BLK: MMIO-APERTURE DRIVER)
linux-acpi(a)vger.kernel.org (open list:ACPI) linux-kernel(a)vger.kernel.org
(open list) devel(a)acpica.org (open list:ACPI COMPONENT ARCHITECTURE
(ACPICA))
Thanks!
-Toshi
7:32 a.m.
New subject: [PATCH 2/2]: acpica/nfit: Rename not-armed bit definition
On Thu, 2015-08-27 at 01:56 +0000, Moore, Robert wrote:
I don’t have any problem changing this in ACPICA if/when you all
agree.
Great. I will resend this patch alone, assuming the patch can be based on
the Dan's NVDIMM tree (since it changes NFIT files).
Thanks,
-Toshi
2295
days inactive
2296
days old
24 comments
6 participants
participants (6)
-
Dan Williams -
Linda Knippers -
Moore, Robert -
Rafael J. Wysocki -
Ross Zwisler -
Toshi Kani