[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4hS44abMCs1sxdLW0YgMjP33Td0LbEWhXi9jqJ_ARU7CA@mail.gmail.com>
Date: Tue, 1 Mar 2016 10:14:32 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Toshi Kani <toshi.kani@....com>
Cc: "Moore, Robert" <robert.moore@...el.com>,
"rjw@...ysocki.net" <rjw@...ysocki.net>,
"Zheng, Lv" <lv.zheng@...el.com>,
"elliott@....com" <elliott@....com>,
"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devel@...ica.org" <devel@...ica.org>
Subject: Re: [PATCH v2 1/3] ACPI/NFIT: Update Control Region Structure to
comply ACPI 6.1
On Tue, Mar 1, 2016 at 9:36 AM, Toshi Kani <toshi.kani@....com> wrote:
> On Tue, 2016-03-01 at 16:03 +0000, Moore, Robert wrote:
>> We have a bunch of macros in include/acmacros.h -- like this:
>>
>> ACPI_MOVE_16_TO_16(d, s)
>
> There is a problem in using the ACPICA byte-swap macros. ACPI is little-
> endian arch, so the macros are set to perform byte-swappings when the CPU
> arch is big-endian. This case, however, is the other way around. The
> fields in question are defined & stored as arrays of bytes. If you treat
> them as multi-bytes numeric values, then you need to byte-swap them when
> the CPU arch is little-endian because arrays of bytes have the same
> addressing as big-endian.
>
> Another issue is that it is not clear who needs to perform the byte-
> swapping among ACPICA and drivers. If ACPICA, drivers must agree that
> these fields are always treated as multi-bytes numeric values despite of
> the spec. If drivers, we need to make sure that only a single driver
> performs this byte-swapping one time as ACPI tables are global structures.
>
> I think it is much clearer to define the structure according to the ACPI
> spec.
I think the "ACPI tables are little-endian" assumption is pervasive
throughout the implementation.
Toshi, it seems all we need is conversions like:
- sprintf(buf, "%#x\n", dcr->vendor_id);
+ sprintf(buf, "%#x\n", le16_to_cpu(dcr->vendor_id));
...for the values exported to userspace through sysfs, but otherwise
leave the base table definitions as is. Will this suffice?
Powered by blists - more mailing lists