| Message ID | 1461620099-11933-2-git-send-email-toshi.kani@hpe.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Apr 25, 2016 at 2:43 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 (SPD values) are stored as arrays of bytes (i.e. big-endian > format). The spec clarifies that they need to be represented > as arrays of bytes as well. > Circling back on this a couple years too late... where are you reading this "arrays of bytes" note. As far as I can see this is wrong. JEDEC says that vendor id is stored LSB of the id is stored at the lowest byte in SPD, which is little endian. So it seems Linux has showing the incorrect value for a long time now. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2018-06-18 at 12:01 -0700, Dan Williams wrote: > On Mon, Apr 25, 2016 at 2:43 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 (SPD values) are stored as arrays of bytes (i.e. big-endian > > format). The spec clarifies that they need to be represented > > as arrays of bytes as well. > > > > Circling back on this a couple years too late... where are you reading > this "arrays of bytes" note. As far as I can see this is wrong. JEDEC > says that vendor id is stored LSB of the id is stored at the lowest > byte in SPD, which is little endian. So it seems Linux has showing the > incorrect value for a long time now. This follows ACPI 6.2a section 5.2.25.10 NVDIMM Representation Format, which Robert cited in his comment below: https://patchwork.kernel.org/patch/10237609/ Thanks, -Toshi
On Mon, Jun 18, 2018 at 12:39 PM, Kani, Toshi <toshi.kani@hpe.com> wrote: > On Mon, 2018-06-18 at 12:01 -0700, Dan Williams wrote: >> On Mon, Apr 25, 2016 at 2:43 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 (SPD values) are stored as arrays of bytes (i.e. big-endian >> > format). The spec clarifies that they need to be represented >> > as arrays of bytes as well. >> > >> >> Circling back on this a couple years too late... where are you reading >> this "arrays of bytes" note. As far as I can see this is wrong. JEDEC >> says that vendor id is stored LSB of the id is stored at the lowest >> byte in SPD, which is little endian. So it seems Linux has showing the >> incorrect value for a long time now. > > This follows ACPI 6.2a section 5.2.25.10 NVDIMM Representation Format, > which Robert cited in his comment below: > https://patchwork.kernel.org/patch/10237609/ Right, the representation format has the fields big-endian for some reason, but the individual values for sysfs should be show little-endian as far as I can see. What am I missing? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Dan Williams [mailto:dan.j.williams@intel.com] > Sent: Monday, June 18, 2018 2:47 PM > To: Kani, Toshi <toshi.kani@hpe.com> > Cc: linux-kernel@vger.kernel.org; linux-nvdimm@lists.01.org; Moore, Robert > <robert.moore@intel.com>; Li, Juston <juston.li@intel.com>; > rjw@rjwysocki.net; linux-acpi@vger.kernel.org; Elliott, Robert (Persistent > Memory) <elliott@hpe.com> > Subject: Re: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with > ACPI 6.1 > > On Mon, Jun 18, 2018 at 12:39 PM, Kani, Toshi <toshi.kani@hpe.com> wrote: > > On Mon, 2018-06-18 at 12:01 -0700, Dan Williams wrote: > >> On Mon, Apr 25, 2016 at 2:43 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 (SPD values) are stored as arrays of bytes (i.e. big-endian > >> > format). The spec clarifies that they need to be represented > >> > as arrays of bytes as well. > >> > > >> > >> Circling back on this a couple years too late... where are you reading > >> this "arrays of bytes" note. As far as I can see this is wrong. JEDEC > >> says that vendor id is stored LSB of the id is stored at the lowest > >> byte in SPD, which is little endian. So it seems Linux has showing the > >> incorrect value for a long time now. > > > > This follows ACPI 6.2a section 5.2.25.10 NVDIMM Representation Format, > > which Robert cited in his comment below: > > https://patchwork.kernel.org/patch/10237609/ > > Right, the representation format has the fields big-endian for some > reason, but the individual values for sysfs should be show > little-endian as far as I can see. What am I missing? In practice, the serial numbers from three major DDR4 DIMM manufacturers are being assigned as big-endian, like in this set of NVDIMM-Ns: /sys/bus/nd/devices/nmem0/nfit/serial:0x122f8255 /sys/bus/nd/devices/nmem1/nfit/serial:0x122f7f5e /sys/bus/nd/devices/nmem2/nfit/serial:0x122f818f /sys/bus/nd/devices/nmem3/nfit/serial:0x122f821c /sys/bus/nd/devices/nmem4/nfit/serial:0x122f817e /sys/bus/nd/devices/nmem5/nfit/serial:0x122f81cd /sys/bus/nd/devices/nmem6/nfit/serial:0x122f821e /sys/bus/nd/devices/nmem7/nfit/serial:0x122f819b /sys/bus/nd/devices/nmem8/nfit/serial:0x122f81a2 /sys/bus/nd/devices/nmem9/nfit/serial:0x122f8198 /sys/bus/nd/devices/nmem10/nfit/serial:0x122f8193 /sys/bus/nd/devices/nmem11/nfit/serial:0x122f7f58 /sys/bus/nd/devices/nmem12/nfit/serial:0x122f81cb /sys/bus/nd/devices/nmem13/nfit/serial:0x122f8181 /sys/bus/nd/devices/nmem14/nfit/serial:0x122f8210 /sys/bus/nd/devices/nmem15/nfit/serial:0x122f821f and this set of regular DIMMs: 396851B4 3968134C 396852DA 396850AB 39685A13 39685317 396852DD 396852D9 Of the possible approaches for the sysfs nfit field decodes: fixed big-endian: matches printed label content (text and barcode) matches ACPI display advice for management tools probably matches SMBIOS Serial Number string format (although that depends on the system firmware) requires user to know that this OS uses big-endian has been upstream for a while now fixed little-endian: harder to see that cd812f12 matches 122f81cd seen elsewhere harder to notice that B4516839 is a peer of 4C136839 might match other little-endian-only OSes requires user to know that this OS uses little-endian native endian: most confusing config/status files referencing the DIMMs are not portable requires user to know that this OS uses native endianness requires user to know the CPU endianness was upstream several years ago
On Mon, Jun 18, 2018 at 2:37 PM, Elliott, Robert (Persistent Memory) <elliott@hpe.com> wrote: > > >> -----Original Message----- >> From: Dan Williams [mailto:dan.j.williams@intel.com] >> Sent: Monday, June 18, 2018 2:47 PM >> To: Kani, Toshi <toshi.kani@hpe.com> >> Cc: linux-kernel@vger.kernel.org; linux-nvdimm@lists.01.org; Moore, Robert >> <robert.moore@intel.com>; Li, Juston <juston.li@intel.com>; >> rjw@rjwysocki.net; linux-acpi@vger.kernel.org; Elliott, Robert (Persistent >> Memory) <elliott@hpe.com> >> Subject: Re: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with >> ACPI 6.1 >> >> On Mon, Jun 18, 2018 at 12:39 PM, Kani, Toshi <toshi.kani@hpe.com> wrote: >> > On Mon, 2018-06-18 at 12:01 -0700, Dan Williams wrote: >> >> On Mon, Apr 25, 2016 at 2:43 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 (SPD values) are stored as arrays of bytes (i.e. big-endian >> >> > format). The spec clarifies that they need to be represented >> >> > as arrays of bytes as well. >> >> > >> >> >> >> Circling back on this a couple years too late... where are you reading >> >> this "arrays of bytes" note. As far as I can see this is wrong. JEDEC >> >> says that vendor id is stored LSB of the id is stored at the lowest >> >> byte in SPD, which is little endian. So it seems Linux has showing the >> >> incorrect value for a long time now. >> > >> > This follows ACPI 6.2a section 5.2.25.10 NVDIMM Representation Format, >> > which Robert cited in his comment below: >> > https://patchwork.kernel.org/patch/10237609/ >> >> Right, the representation format has the fields big-endian for some >> reason, but the individual values for sysfs should be show >> little-endian as far as I can see. What am I missing? > > In practice, the serial numbers from three major DDR4 DIMM manufacturers > are being assigned as big-endian, like in this set of NVDIMM-Ns: > /sys/bus/nd/devices/nmem0/nfit/serial:0x122f8255 > /sys/bus/nd/devices/nmem1/nfit/serial:0x122f7f5e > /sys/bus/nd/devices/nmem2/nfit/serial:0x122f818f > /sys/bus/nd/devices/nmem3/nfit/serial:0x122f821c > /sys/bus/nd/devices/nmem4/nfit/serial:0x122f817e > /sys/bus/nd/devices/nmem5/nfit/serial:0x122f81cd > /sys/bus/nd/devices/nmem6/nfit/serial:0x122f821e > /sys/bus/nd/devices/nmem7/nfit/serial:0x122f819b > /sys/bus/nd/devices/nmem8/nfit/serial:0x122f81a2 > /sys/bus/nd/devices/nmem9/nfit/serial:0x122f8198 > /sys/bus/nd/devices/nmem10/nfit/serial:0x122f8193 > /sys/bus/nd/devices/nmem11/nfit/serial:0x122f7f58 > /sys/bus/nd/devices/nmem12/nfit/serial:0x122f81cb > /sys/bus/nd/devices/nmem13/nfit/serial:0x122f8181 > /sys/bus/nd/devices/nmem14/nfit/serial:0x122f8210 > /sys/bus/nd/devices/nmem15/nfit/serial:0x122f821f > > and this set of regular DIMMs: > 396851B4 > 3968134C > 396852DA > 396850AB > 39685A13 > 39685317 > 396852DD > 396852D9 Let's take something simple like Vendor ID. What is the Vendor ID for these DIMMs and what does Linux print in sysfs? > Of the possible approaches for the sysfs nfit field decodes: > fixed big-endian: > matches printed label content (text and barcode) > matches ACPI display advice for management tools > probably matches SMBIOS Serial Number string format (although > that depends on the system firmware) > requires user to know that this OS uses big-endian > has been upstream for a while now > fixed little-endian: > harder to see that cd812f12 matches 122f81cd seen elsewhere > harder to notice that B4516839 is a peer of 4C136839 > might match other little-endian-only OSes > requires user to know that this OS uses little-endian > native endian: > most confusing > config/status files referencing the DIMMs are not portable > requires user to know that this OS uses native endianness > requires user to know the CPU endianness > was upstream several years ago The time upstream is painful, but if Linux is needlessly swizzling the bits this needs to be fixed. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogRGFuIFdpbGxpYW1zIFtt YWlsdG86ZGFuLmoud2lsbGlhbXNAaW50ZWwuY29tXQ0KPiBTZW50OiBNb25kYXksIEp1bmUgMTgs IDIwMTggNDo0NyBQTQ0KPiBUbzogRWxsaW90dCwgUm9iZXJ0IChQZXJzaXN0ZW50IE1lbW9yeSkg PGVsbGlvdHRAaHBlLmNvbT4NCj4gQ2M6IEthbmksIFRvc2hpIDx0b3NoaS5rYW5pQGhwZS5jb20+ OyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4gbnZkaW1tQGxpc3RzLjAx Lm9yZzsgTW9vcmUsIFJvYmVydCA8cm9iZXJ0Lm1vb3JlQGludGVsLmNvbT47IExpLCBKdXN0b24N Cj4gPGp1c3Rvbi5saUBpbnRlbC5jb20+OyByandAcmp3eXNvY2tpLm5ldDsgbGludXgtYWNwaUB2 Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2MyAxLzJdIGFjcGkvbmZpdDog VXBkYXRlIG5maXQgZHJpdmVyIHRvIGNvbXBseSB3aXRoDQo+IEFDUEkgNi4xDQoNCg0KPiBMZXQn cyB0YWtlIHNvbWV0aGluZyBzaW1wbGUgbGlrZSBWZW5kb3IgSUQuIFdoYXQgaXMgdGhlIFZlbmRv ciBJRCBmb3INCj4gdGhlc2UgRElNTXMgYW5kIHdoYXQgZG9lcyBMaW51eCBwcmludCBpbiBzeXNm cz8NCg0KSGVyZSBhcmUgc29tZSBleGFtcGxlcyAoa2VybmVsIDQuMTcpOg0KDQokIGNkIC9zeXMv YnVzL25kL2RldmljZXMvbm1lbTAvbmZpdA0KJCBncmVwIC1zIC4gKg0KZGV2aWNlOjB4MzE0ZQ0K ZHNtX21hc2s6MHgzYzc2DQpmYW1pbHk6MQ0KZmxhZ3M6c21hcnRfbm90aWZ5DQpmb3JtYXQ6MHgw MTAxDQpmb3JtYXRzOjENCmhhbmRsZToweDENCmlkOjgwMmMtMGYtMTYxMi0xMjJmODI1NQlbU1BE IGJ5dGVzIDMyMC0zMjgsIGluIHRoYXQgb3JkZXIgbGVmdC10by1yaWdodF0NCnBoeXNfaWQ6MHgx Ng0KcmV2X2lkOjB4MzEwMA0Kc2VyaWFsOjB4MTIyZjgyNTUNCnN1YnN5c3RlbV9kZXZpY2U6MHgz MTQxDQpzdWJzeXN0ZW1fcmV2X2lkOjB4MDEwMA0Kc3Vic3lzdGVtX3ZlbmRvcjoweDgwMzQJW0N5 cHJlc3MgU2VtaWNvbmR1Y3Rvcl0NCnZlbmRvcjoweDgwMmMJCVtNaWNyb25dDQoNCiQgY2QgL3N5 cy9idXMvbmQvZGV2aWNlcy9ubWVtMS9uZml0DQokIGdyZXAgLXMgLiAqDQpkZXZpY2U6MHgzMTRl DQpkc21fbWFzazoweDNjNzYNCmZhbWlseToxDQpmbGFnczpzbWFydF9ub3RpZnkNCmZvcm1hdDow eDAxMDENCmZvcm1hdHM6MQ0KaGFuZGxlOjB4Mg0KaWQ6ODAyYy0wZi0xNjEyLTEyMmY3ZjVlDQpw aHlzX2lkOjB4MTUNCnJldl9pZDoweDMxMDANCnNlcmlhbDoweDEyMmY3ZjVlDQpzdWJzeXN0ZW1f ZGV2aWNlOjB4MzE0MQ0Kc3Vic3lzdGVtX3Jldl9pZDoweDAxMDANCnN1YnN5c3RlbV92ZW5kb3I6 MHg4MDM0DQp2ZW5kb3I6MHg4MDJjDQoNClNvbWUgY29ycmVzcG9uZGluZyBpbmZvcm1hdGlvbiBm b3IgdGhvc2UgTlZESU1NLU5zIGFzIHJlcG9ydGVkIGJ5IGRtaWRlY29kZToNCg0KSGFuZGxlIDB4 MDBBOCwgRE1JIHR5cGUgMjM3LCA5IGJ5dGVzDQpPRU0tc3BlY2lmaWMgVHlwZQ0KICAgICAgICBI ZWFkZXIgYW5kIERhdGE6DQogICAgICAgICAgICAgICAgRUQgMDkgQTggMDAgMTYgMDAgMDEgMDIg MDMNCiAgICAgICAgU3RyaW5nczoNCiAgICAgICAgICAgICAgICBNaWNyb24JCVtNb2R1bGUgbWFu dWZhY3R1cmVyXQ0KICAgICAgICAgICAgICAgIDE4QVNGMUc3MlhGMTJHMVkxMUFBCVtNb2R1bGUg cGFydCBudW1iZXJdDQogICAgICAgICAgICAgICAgMTIyRjgyNTUJW01vZHVsZSBzZXJpYWwgbnVt YmVyXQ0KDQpIYW5kbGUgMHgwMEE3LCBETUkgdHlwZSAyMzcsIDkgYnl0ZXMNCk9FTS1zcGVjaWZp YyBUeXBlDQogICAgICAgIEhlYWRlciBhbmQgRGF0YToNCiAgICAgICAgICAgICAgICBFRCAwOSBB NyAwMCAxNSAwMCAwMSAwMiAwMw0KICAgICAgICBTdHJpbmdzOg0KICAgICAgICAgICAgICAgIE1p Y3Jvbg0KICAgICAgICAgICAgICAgIDE4QVNGMUc3MlhGMTJHMVkxMUFBDQogICAgICAgICAgICAg ICAgMTIyRjdGNUUNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 19, 2018 at 7:31 AM, Elliott, Robert (Persistent Memory) <elliott@hpe.com> wrote: > > >> -----Original Message----- >> From: Dan Williams [mailto:dan.j.williams@intel.com] >> Sent: Monday, June 18, 2018 4:47 PM >> To: Elliott, Robert (Persistent Memory) <elliott@hpe.com> >> Cc: Kani, Toshi <toshi.kani@hpe.com>; linux-kernel@vger.kernel.org; linux- >> nvdimm@lists.01.org; Moore, Robert <robert.moore@intel.com>; Li, Juston >> <juston.li@intel.com>; rjw@rjwysocki.net; linux-acpi@vger.kernel.org >> Subject: Re: [PATCH v3 1/2] acpi/nfit: Update nfit driver to comply with >> ACPI 6.1 > > >> Let's take something simple like Vendor ID. What is the Vendor ID for >> these DIMMs and what does Linux print in sysfs? > > Here are some examples (kernel 4.17): > > $ cd /sys/bus/nd/devices/nmem0/nfit > $ grep -s . * > device:0x314e > dsm_mask:0x3c76 > family:1 > flags:smart_notify > format:0x0101 > formats:1 > handle:0x1 > id:802c-0f-1612-122f8255[SPD bytes 320-328, in that order left-to-right] > phys_id:0x16 > rev_id:0x3100 > serial:0x122f8255 > subsystem_device:0x3141 > subsystem_rev_id:0x0100 > subsystem_vendor:0x8034[Cypress Semiconductor] > vendor:0x802c[Micron] 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? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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.
On Tue, Jun 19, 2018 at 12:53 PM, Elliott, Robert (Persistent Memory) <elliott@hpe.com> wrote: > > >> -----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 Ok, JEDEC defines byte 320 as the LSB, so the fact that Linux is showing 0x2c as the LSB is wrong. Linux needs to be fixed. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c index d0f35e6..5dc243c 100644 --- a/drivers/acpi/nfit.c +++ b/drivers/acpi/nfit.c @@ -816,7 +816,7 @@ 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%04x\n", be16_to_cpu(dcr->vendor_id)); } static DEVICE_ATTR_RO(vendor); @@ -825,7 +825,7 @@ 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%04x\n", be16_to_cpu(dcr->revision_id)); } static DEVICE_ATTR_RO(rev_id); @@ -834,7 +834,7 @@ 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%04x\n", be16_to_cpu(dcr->device_id)); } static DEVICE_ATTR_RO(device); @@ -843,7 +843,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%04x\n", be16_to_cpu(dcr->code)); } static DEVICE_ATTR_RO(format); @@ -852,7 +852,7 @@ 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%08x\n", be32_to_cpu(dcr->serial_number)); } static DEVICE_ATTR_RO(serial);
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 (SPD values) are stored as arrays of bytes (i.e. big-endian format). The spec clarifies that they need to be represented as arrays of bytes as well. This patch makes the following changes to support this update. - Change the NFIT driver to show SPD ID values in big-endian format. - Change sprintf format to use "0x" instead of "#" since "%#02x" does not prepend '0'. 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> --- drivers/acpi/nfit.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html