RE: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with ACPI 6.1

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel-
> owner@xxxxxxxxxxxxxxx] 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.