mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Elliott, Robert (Persistent Memory)" <elliott@hpe.com>
To: 'Dan Williams' <dan.j.williams@intel.com>
Cc: "Kani, Toshi" <toshi.kani@hpe.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"Moore, Robert" <robert.moore@intel.com>,
	"Li, Juston" <juston.li@intel.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>
Subject: RE: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with ACPI 6.1
Date: Tue, 19 Jun 2018 19:53:11 +0000	[thread overview]
Message-ID: <AT5PR8401MB1169CBD96EE6C152A24D9FD3AB700@AT5PR8401MB1169.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CAPcyv4joz7OemcKvJMJVhqKEVgSMpfLbm2DRP9B=S3vRPUAGkQ@mail.gmail.com>



> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Dan Williams
> Sent: Tuesday, June 19, 2018 10:29 AM
> Subject: Re: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with
> ACPI 6.1
...
> >
> > Here are some examples (kernel 4.17):

Note that these values were as reported on a little-endian system.

> Ok, so the lowest significant byte of the Micron id is supposed to be
> 0x2c and this text representation matches that. So the bytes are being
> endian swapped when written to the SPD?

SPD byte 320 is 0x80. That's the bank number byte (with odd parity).
SPD byte 321 is 0x2c. That's the manufacturer code byte (with odd parity).

If treated as a single 2-byte value, that is:
* 0x802c (32812 in decimal) if interpreted as big-endian 
* 0x2c80 (11392 in decimal) if interpreted as little-endian

Reviewing the _show() functions in drivers/acpi/nfit/core.c:
  BE device:0x314e
  NE dsm_mask:0x3c76
  NE family:1
  N/A flags:smart_notify
  LE format:0x0101
  N/A formats:1
  BE handle:0x1
  BE id:802c-0f-1612-122f8255 [SPD bytes 320-328]
  NE phys_id:0x16
  BE rev_id:0x3100
  BE serial:0x122f8255
  BE subsystem_device:0x3141
  BE subsystem_rev_id:0x0100
  BE subsystem_vendor:0x8034 [Cypress Semiconductor]
  BE vendor:0x802c [Micron]

(BE=fixed big-endian, LE=fixed little-endian, NE=native-endian,
 N/A=not applicable)

BE and LE will print the same regardless of the endianness
of the system, while NE will vary.

Reviewing the NE sysfs files:

phys_id:
This NFIT field reports an SMBIOS handle (instance ID), a LE number
from 0..n (0x103 on one of my systems).  dmidecode shows handles
like this:
  Handle 0x0016, DMI type 17, 40 bytes
  Memory Device
        Array Handle: 0x000A

On a big-endian system, I think we'll see
  phys_id:0x1600
which means 5632 decimal, which is misleading. Fixed LE would
be better.

dsm_mask:
The code that parses _DSM function 0 output, acpi_check_dsm(),
correctly interprets the bitmask as little-endian for internal
calculations.  I think a big-endian system will print 
dsm_mask:0x763c, which is confusing. Fixed LE would be better.  
The override_dsm_mask module parameter needs to match.

family:
This is an internal value, so NE is fine.


  reply	other threads:[~2018-06-19 19:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-25 21:34 [PATCH v3 0/2] Support ACPI 6.1 update in NFIT Control Region Structure Toshi Kani
2016-04-25 21:34 ` [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with ACPI 6.1 Toshi Kani
2018-06-18 19:01   ` Dan Williams
2018-06-18 19:39     ` Kani, Toshi
2018-06-18 19:46       ` Dan Williams
2018-06-18 21:37         ` Elliott, Robert (Persistent Memory)
2018-06-18 21:46           ` Dan Williams
2018-06-19 14:31             ` Elliott, Robert (Persistent Memory)
2018-06-19 15:28               ` Dan Williams
2018-06-19 19:53                 ` Elliott, Robert (Persistent Memory) [this message]
2018-06-19 23:46                   ` Dan Williams
2016-04-25 21:34 ` [PATCH v3 2/2] acpi/nfit: Add sysfs "id" for NVDIMM ID Toshi Kani
2016-04-26 23:54 ` [PATCH v3 0/2] Support ACPI 6.1 update in NFIT Control Region Structure Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AT5PR8401MB1169CBD96EE6C152A24D9FD3AB700@AT5PR8401MB1169.NAMPRD84.PROD.OUTLOOK.COM \
    --to=elliott@hpe.com \
    --cc=dan.j.williams@intel.com \
    --cc=juston.li@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=rjw@rjwysocki.net \
    --cc=robert.moore@intel.com \
    --cc=toshi.kani@hpe.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®