Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1338539
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/2] ACPI/NFIT: Update Control Region Structure per ACPI 6.1 |
| Date | 2016-02-19 23:40 +0100 |
| Message-ID | <r41Sp-3SM-1@gated-at.bofh.it> (permalink) |
| References | <r41z3-3I8-3@gated-at.bofh.it> <r41z3-3I8-1@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, Feb 19, 2016 at 3:08 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
> ACPI 6.1, Table 5-133, updates NVDIMM Control Region Structure
> as follows.
> - Valid Fields, Manufacturing Location, and Manufacturing Date
> are added from reserved range. No change in the structure size.
> - IDs defined as SPD values are arrays of bytes. The spec
> clarified that they need to be represented as arrays of bytes
> as well.
>
> This patch makes the following changes to support this update.
> - Change 'struct acpi_nfit_control_region' to reflect the update.
> SPD IDs are defined as arrays of bytes, so that they can be
> treated in the same way regardless of CPU endianness and are
> not miss-treated as little-endian numeric values.
> - Change the NFIT driver to show SPD ID values as array of bytes
> in sysfs.
> - Change the NFIT driver to show Manufacturing Location and
> Manufacturing Date when they are valid.
> - Change the NFIT driver to show an NVDIMM unique ID as defined
> in section 5.2.25.9 of the spec.
> - Change sprintf format to use "0x" instead of "#" since "%#02x"
> does not prepend '0' in some reason.
>
> link: http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Robert Elliott <elliott@hpe.com>
> Cc: <devel@acpica.org>
> ---
> drivers/acpi/nfit.c | 75 ++++++++++++++++++++++++++++++++++++++++++-------
> include/acpi/actbl1.h | 24 ++++++++++------
> 2 files changed, 80 insertions(+), 19 deletions(-)
Hi Toshi,
I general looks good, but note that changes to include/acpi/actbl1.h
need to go through the ACPICA project. Then their normal import
process will bring it into the kernel. I added Lv to the cc.
>
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index ad6d8c6..06677d9 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -722,7 +722,8 @@ static ssize_t vendor_show(struct device *dev,
> {
> struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
>
> - return sprintf(buf, "%#x\n", dcr->vendor_id);
> + return sprintf(buf, "0x%02x%02x\n",
> + dcr->vendor_id[0], dcr->vendor_id[1]);
> }
> static DEVICE_ATTR_RO(vendor);
>
> @@ -731,7 +732,8 @@ static ssize_t rev_id_show(struct device *dev,
> {
> struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
>
> - return sprintf(buf, "%#x\n", dcr->revision_id);
> + return sprintf(buf, "0x%02x%02x\n",
> + dcr->revision_id[0], dcr->revision_id[1]);
> }
> static DEVICE_ATTR_RO(rev_id);
>
> @@ -740,7 +742,8 @@ static ssize_t device_show(struct device *dev,
> {
> struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
>
> - return sprintf(buf, "%#x\n", dcr->device_id);
> + return sprintf(buf, "0x%02x%02x\n",
> + dcr->device_id[0], dcr->device_id[1]);
> }
> static DEVICE_ATTR_RO(device);
>
> @@ -749,7 +752,7 @@ static ssize_t format_show(struct device *dev,
> {
> struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
>
> - return sprintf(buf, "%#x\n", dcr->code);
> + return sprintf(buf, "0x%02x%02x\n", dcr->code[0], dcr->code[1]);
> }
> static DEVICE_ATTR_RO(format);
>
> @@ -758,7 +761,9 @@ static ssize_t serial_show(struct device *dev,
> {
> struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
>
> - return sprintf(buf, "%#x\n", dcr->serial_number);
> + return sprintf(buf, "0x%02x%02x%02x%02x\n",
> + dcr->serial_number[0], dcr->serial_number[1],
> + dcr->serial_number[2], dcr->serial_number[3]);
> }
> static DEVICE_ATTR_RO(serial);
These fixups look good.
Can we split the new sysfs attributes below into their own patch? One
more comments below:
>
> @@ -776,6 +781,45 @@ static ssize_t flags_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(flags);
>
> +static ssize_t mfg_location_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
> +
> + return sprintf(buf, "0x%02x\n", dcr->manufacturing_location);
> +}
> +static DEVICE_ATTR_RO(mfg_location);
> +
> +static ssize_t mfg_date_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
> +
> + return sprintf(buf, "0x%02x%02x\n",
> + dcr->manufacturing_date[0], dcr->manufacturing_date[1]);
> +}
> +static DEVICE_ATTR_RO(mfg_date);
> +
> +static ssize_t unique_id_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
> +
> + if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
> + return sprintf(buf, "%02x%02x-%02x-%02x%02x-%02x%02x%02x%02x\n",
> + dcr->vendor_id[0], dcr->vendor_id[1],
> + dcr->manufacturing_location,
> + dcr->manufacturing_date[0], dcr->manufacturing_date[1],
> + dcr->serial_number[0], dcr->serial_number[1],
> + dcr->serial_number[2], dcr->serial_number[3]);
> + else
> + return sprintf(buf, "%02x%02x-%02x%02x%02x%02x\n",
> + dcr->vendor_id[0], dcr->vendor_id[1],
> + dcr->serial_number[0], dcr->serial_number[1],
> + dcr->serial_number[2], dcr->serial_number[3]);
> +}
> +static DEVICE_ATTR_RO(unique_id);
Let's just call it 'id' as not to confuse it with a 'uuid'. Also if
the manufacturing info is incorporated into the id then I don't think
we need separate 'mfg_location' and 'mfg_date' attributes. If
userspace really wants those separately it can just dump the NFIT
table.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 1/2] ACPI/NFIT: Update Control Region Structure per ACPI 6.1 Toshi Kani <toshi.kani@hpe.com> - 2016-02-19 23:20 +0100
Re: [PATCH 1/2] ACPI/NFIT: Update Control Region Structure per ACPI 6.1 Dan Williams <dan.j.williams@intel.com> - 2016-02-19 23:40 +0100
Re: [PATCH 1/2] ACPI/NFIT: Update Control Region Structure per ACPI 6.1 Toshi Kani <toshi.kani@hpe.com> - 2016-02-20 00:00 +0100
RE: [PATCH 1/2] ACPI/NFIT: Update Control Region Structure per ACPI 6.1 "Moore, Robert" <robert.moore@intel.com> - 2016-02-20 00:00 +0100
csiph-web