ACPICA version 20180927 released
by Moore, Robert
27 September 2018. Summary of changes for version 20180927:
This release is available at https://acpica.org/downloads
1) ACPICA kernel-resident subsystem:
Updated the GPE support to clear the status of all ACPI events when entering any/all sleep states in order to avoid premature wakeups. In theory, this may cause some wakeup events to be missed, but the likelihood of this is small. This change restores the original behavior of the ACPICA code in order to fix a regression seen from the previous "Stop unconditionally clearing ACPI IRQs during suspend/resume" change. This regression could cause some systems to incorrectly wake immediately.
Updated the execution of the _REG methods during initialization and namespace loading to bring the behavior into closer conformance to the ACPI specification and other ACPI implementations:
>From the ACPI specification 6.2A, section 6.5.4 "_REG (Region):
"Control methods must assume all operation regions are inaccessible until the _REG(RegionSpace, 1) method is executed"
"The exceptions to this rule are:
1. OSPM must guarantee that the following operation regions are always accessible:
SystemIO operation regions.
SystemMemory operation regions when accessing memory returned by the System Address Map reporting interfaces."
Since the state of both the SystemIO and SystemMemory address spaces are defined by the specification to never change, this ACPICA change ensures that now _REG is never called on them. This solves some problems seen in the field and provides compatibility with other ACPI implementations. An update to the upcoming new version of the ACPI specification will help clarify this behavior.
Updated the implementation of support for the Generic Serial Bus. For the "bidirectional" protocols, the internal implementation now automatically creates a return data buffer of the maximum size (255). This handles the worst-case for data that is returned from the serial bus handler, and fixes some problems seen in the field. This new buffer is directly returned to the ASL. As such, there is no true "bidirectional" buffer, which matches the ACPI specification. This is the reason for the "double store" seen in the example ASL code in the specification, shown below:
Word Process Call (AttribProcessCall):
OperationRegion(TOP1, GenericSerialBus, 0x00, 0x100)
Field(TOP1, BufferAcc, NoLock, Preserve)
{
FLD1, 8, // Virtual register at command value 1.
}
Name(BUFF, Buffer(20){}) // Create GenericSerialBus data buffer as BUFF
CreateWordField(BUFF, 0x02, DATA) // DATA = Data (Word)
Store(0x5416, DATA) // Save 0x5416 into the data buffer
Store(Store(BUFF, FLD1), BUFF) // Invoke a write/read Process Call transaction
// This is the "double store". The write to
// FLD1 returns a new buffer, which is stored
// back into BUFF with the second Store.
2) iASL Compiler/Disassembler and Tools:
iASL: Implemented detection of extraneous/redundant uses of the Offset() operator within a Field Unit list. A remark is now issued for these. For example, the first two of the Offset() operators below are extraneous. Because both the compiler and the interpreter track the offsets automatically, these Offsets simply refer to the current offset and are unnecessary. Note, when optimization is enabled, the iASL compiler will in fact remove the redundant Offset operators and will not emit any AML code for them.
OperationRegion (OPR1, SystemMemory, 0x100, 0x100)
Field (OPR1)
{
Offset (0), // Never needed
FLD1, 32,
Offset (4), // Redundant, offset is already 4 (bytes)
FLD2, 8,
Offset (64), // OK use of Offset.
FLD3, 16,
}
dsdt.asl 14: Offset (0),
Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4),
Remark 2158 - ^ Unnecessary/redundant use of Offset operator
4 years
[pm:bleeding-edge 124/125] drivers//acpi/x86/utils.c:55:25: error: implicit declaration of function 'INTEL_CPU_FAM6_NODATA'; did you mean 'INTEL_CPU_FAM_ANY'?
by kbuild test robot
tree: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
head: a7bd624d55825520984461c8abe93aebbe955242
commit: cb2ed44786c43d6c96eb8bee39604bd1b949c9df [124/125] ACPI / x86: utils: Get rid of custom ICPU() macro
config: x86_64-randconfig-x019-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout cb2ed44786c43d6c96eb8bee39604bd1b949c9df
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> drivers//acpi/x86/utils.c:55:25: error: implicit declaration of function 'INTEL_CPU_FAM6_NODATA'; did you mean 'INTEL_CPU_FAM_ANY'? [-Werror=implicit-function-declaration]
ENTRY("80860F09", "1", INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), {}),
^
drivers//acpi/x86/utils.c:45:4: note: in definition of macro 'ENTRY'
{ cpu_models, {} }, \
^~~~~~~~~~
>> drivers//acpi/x86/utils.c:55:47: error: 'ATOM_SILVERMONT1' undeclared here (not in a function)
ENTRY("80860F09", "1", INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), {}),
^
drivers//acpi/x86/utils.c:45:4: note: in definition of macro 'ENTRY'
{ cpu_models, {} }, \
^~~~~~~~~~
>> drivers//acpi/x86/utils.c:56:47: error: 'ATOM_AIRMONT' undeclared here (not in a function); did you mean 'ATOM_SILVERMONT1'?
ENTRY("80862288", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {}),
^
drivers//acpi/x86/utils.c:45:4: note: in definition of macro 'ENTRY'
{ cpu_models, {} }, \
^~~~~~~~~~
>> drivers//acpi/x86/utils.c:68:47: error: 'HASWELL_ULT' undeclared here (not in a function)
ENTRY("SYNA7500", "1", INTEL_CPU_FAM6_NODATA(HASWELL_ULT), {
^
drivers//acpi/x86/utils.c:45:4: note: in definition of macro 'ENTRY'
{ cpu_models, {} }, \
^~~~~~~~~~
cc1: some warnings being treated as errors
vim +55 drivers//acpi/x86/utils.c
42
43 #define ENTRY(hid, uid, cpu_models, dmi...) { \
44 { { hid, }, {} }, \
> 45 { cpu_models, {} }, \
46 { { .matches = dmi }, {} }, \
47 uid, \
48 }
49
50 static const struct always_present_id always_present_ids[] = {
51 /*
52 * Bay / Cherry Trail PWM directly poked by GPU driver in win10,
53 * but Linux uses a separate PWM driver, harmless if not used.
54 */
> 55 ENTRY("80860F09", "1", INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), {}),
> 56 ENTRY("80862288", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {}),
57 /*
58 * The INT0002 device is necessary to clear wakeup interrupt sources
59 * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
60 */
61 ENTRY("INT0002", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {}),
62 /*
63 * On the Dell Venue 11 Pro 7130 and 7139, the DSDT hides
64 * the touchscreen ACPI device until a certain time
65 * after _SB.PCI0.GFX0.LCD.LCD1._ON gets called has passed
66 * *and* _STA has been called at least 3 times since.
67 */
> 68 ENTRY("SYNA7500", "1", INTEL_CPU_FAM6_NODATA(HASWELL_ULT), {
69 DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
70 DMI_MATCH(DMI_PRODUCT_NAME, "Venue 11 Pro 7130"),
71 }),
72 ENTRY("SYNA7500", "1", INTEL_CPU_FAM6_NODATA(HASWELL_ULT), {
73 DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
74 DMI_MATCH(DMI_PRODUCT_NAME, "Venue 11 Pro 7139"),
75 }),
76
77 /*
78 * The GPD win BIOS dated 20170221 has disabled the accelerometer, the
79 * drivers sometimes cause crashes under Windows and this is how the
80 * manufacturer has solved this :| Note that the the DMI data is less
81 * generic then it seems, a board_vendor of "AMI Corporation" is quite
82 * rare and a board_name of "Default String" also is rare.
83 *
84 * Unfortunately the GPD pocket also uses these strings and its BIOS
85 * was copy-pasted from the GPD win, so it has a disabled KIOX000A
86 * node which we should not enable, thus we also check the BIOS date.
87 */
88 ENTRY("KIOX000A", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {
89 DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
90 DMI_MATCH(DMI_BOARD_NAME, "Default string"),
91 DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
92 DMI_MATCH(DMI_BIOS_DATE, "02/21/2017")
93 }),
94 ENTRY("KIOX000A", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {
95 DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
96 DMI_MATCH(DMI_BOARD_NAME, "Default string"),
97 DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
98 DMI_MATCH(DMI_BIOS_DATE, "03/20/2017")
99 }),
100 ENTRY("KIOX000A", "1", INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), {
101 DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
102 DMI_MATCH(DMI_BOARD_NAME, "Default string"),
103 DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
104 DMI_MATCH(DMI_BIOS_DATE, "05/25/2017")
105 }),
106 };
107
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
4 years
[pm:bleeding-edge 123/125] drivers//acpi/acpi_lpss.c:293:2: error: implicit declaration of function 'INTEL_CPU_FAM6_NODATA'; did you mean 'INTEL_CPU_FAM_ANY'?
by kbuild test robot
tree: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
head: a7bd624d55825520984461c8abe93aebbe955242
commit: 6aee9dc9f7d06eb1f9dc443d89ed5b9faf2a773f [123/125] ACPI / LPSS: Get rid of custom ICPU() macro
config: x86_64-randconfig-x013-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 6aee9dc9f7d06eb1f9dc443d89ed5b9faf2a773f
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> drivers//acpi/acpi_lpss.c:293:2: error: implicit declaration of function 'INTEL_CPU_FAM6_NODATA'; did you mean 'INTEL_CPU_FAM_ANY'? [-Werror=implicit-function-declaration]
INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), /* Valleyview, Bay Trail */
^~~~~~~~~~~~~~~~~~~~~
INTEL_CPU_FAM_ANY
>> drivers//acpi/acpi_lpss.c:293:24: error: 'ATOM_SILVERMONT1' undeclared here (not in a function)
INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), /* Valleyview, Bay Trail */
^~~~~~~~~~~~~~~~
>> drivers//acpi/acpi_lpss.c:294:24: error: 'ATOM_AIRMONT' undeclared here (not in a function); did you mean 'ATOM_SILVERMONT1'?
INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), /* Braswell, Cherry Trail */
^~~~~~~~~~~~
ATOM_SILVERMONT1
cc1: some warnings being treated as errors
vim +293 drivers//acpi/acpi_lpss.c
291
292 static const struct x86_cpu_id lpss_cpu_ids[] = {
> 293 INTEL_CPU_FAM6_NODATA(ATOM_SILVERMONT1), /* Valleyview, Bay Trail */
> 294 INTEL_CPU_FAM6_NODATA(ATOM_AIRMONT), /* Braswell, Cherry Trail */
295 {}
296 };
297
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
4 years
Re: [Devel] [PATCH v2] ACPI: surface3_power: MSHW0011 rev-eng implementation
by Moore, Robert
> -----Original Message-----
> From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com]
> Sent: Friday, August 31, 2018 7:55 AM
> To: Andy Shevchenko <andy.shevchenko(a)gmail.com>
> Cc: Hans de Goede <hdegoede(a)redhat.com>; Bastien Nocera
> <hadess(a)hadess.net>; stephenjust(a)gmail.com; Sebastian Reichel
> <sre(a)kernel.org>; Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>;
> lenb(a)kernel.org; Moore, Robert <robert.moore(a)intel.com>;
> lv.zheng(a)intel.com; mika.westerberg(a)linux.intel.com; linux-
> acpi(a)vger.kernel.org; devel(a)acpica.org; linux-pm(a)vger.kernel.org; lkml
> <linux-kernel(a)vger.kernel.org>
> Subject: Re: [PATCH v2] ACPI: surface3_power: MSHW0011 rev-eng
> implementation
>
> Hi Andy,
>
> I am resurrecting this thread now that ACPICA seemed to finally have
> fixed the bug that prevent the driver to work.
> The patch I submitted was reverted shortly after, which lead me to
> ignore this review until ACPICA was fixed. It took a lot of effort from
> Hans to have a fix accepted, so now we can hope to upstream this driver.
>
[Moore, Robert]
The worst part of all this is that the ACPI specification is so ambiguous in this area, that we were forced to *guess* at certain elements of the implementation.
So, if anyone knows of any ASL/machines that use the serial bus stuff, please forward the code to me.
This includes:
GenericSerialBus
SMBus
IPMI
And maybe:
GeneralPurposeIo
For completeness.
> On Fri, Jun 30, 2017 at 6:37 PM Andy Shevchenko
> <andy.shevchenko(a)gmail.com> wrote:
> >
> > On Fri, Jun 30, 2017 at 6:57 PM, Benjamin Tissoires
> > <benjamin.tissoires(a)redhat.com> wrote:
> > > On Jun 29 2017 or thereabouts, Andy Shevchenko wrote:
> > >> On Thu, Jun 29, 2017 at 3:10 PM, Benjamin Tissoires
> > >> <benjamin.tissoires(a)redhat.com> wrote:
> >
> > >> What devices (laptops, tablets) have it?
> > >> Surface 3. What else?
> > >
> > > So far, Surface 3 only. It's a Microsoft PNPId, so I guess they
> > > control which device has it. Maybe the model after the Surface 3
> > > (reduced
> > > platform) will have such chip, but for now, it's unknown.
> >
> > Please, extend introduction in commit message to state above.
>
> OK. On this note, I have been mentioned that the Surface Pro 2017 uses a
> similar mechanism as in it's also using an operation region handler, but
> this time over UART, not I2C :)
>
> >
> > >> > I couldn't manage to get the IRQ correctly triggered, so I am
> > >> > using a good old polling thread to check for changes.
> > >>
> > >> It might be
> >
> > It seems I didn't finished the sentence here.
> >
> > I though it might be actually ACPI event, GPE or direct IRQ (when GPIO
> > chip should not disable it, though if it's the case it likely a BIOS
> > bug for this hardware).
>
> If you don't mind, I'd rather have the polling version that seems to be
> working first. I haven't touched the logs I had from Windows since last
> year, so I am a little bit rusty on debugging this.
> FWIW, /proc/interrupts doesn't change a bit when I unplug/replug the
> power cable.
>
> My guess is that the Windows driver initializes the chip in a different
> way and this enables the cable detection.
>
> >
> > >> > + help
> > >> > + Select this option to enable support for ACPI operation
> > >> > + region of the Surface 3 battery platform driver.
> > >>
> > >> > +/*
> > >> > + * Supports for the power IC on the Surface 3 tablet.
> > >>
> > >> Shouldn't it go to drivers/acpi/pmic folder ?
> > >
> > > Already answered later in the thread, so yes, I'll move it there.
> >
> > Actually Hans did a good point, so, feel free to use
> drivers/platform/x86.
>
> Roger that!
>
> >
> > >> And did you check if it have actual chip IP vendor name and model?
> > >> Most likely it's a TI (based?) solution.
> > >
> > > As mentioned, I have strictly no idea. I can not crack open the
> > > Surface
> > > 3 without breaking the warranty (I already had to return it once
> > > because the disk crashed).
> >
> > We have one indeed cracked (screen is broken for good :-) ), so, I
> > would check what I can find there.
> >
> > > And I do not find anything brand-related under Windows either:
> > > - it's called "Surface Platform Power Driver"
> > > - and the driver is provided by Microsoft
> >
> > Fair enough.
> >
> > >> > +static int mshw0011_bix(struct mshw0011_data *cdata, struct bix
> > >> > +*bix) {
> > >>
> > >> > + memcpy(bix->serial, buf + 7, 3);
> > >> > + memcpy(bix->serial + 3, buf, 6);
> > >> > + bix->serial[9] = '\0';
> > >>
> > >> snprintf()?
> > >
> > > probably :)
> >
> > I would do this until we have an evidence that it contains
> > non-printable symbols (or, in case you want to fix ahead, make the
> > buffer 4 times bigger and use %*pE)
>
> I can't really make the buffer 4 time bigger. The buffer is then used by
> the DSDT table to report the _BIX status, so the length of 10 is
> mandatory.
> It doesn't seem to hurt, and worse case, we will just strip the serial,
> not a big deal IMO.
>
> >
> > >> > + memcpy(bix->OEM, buf, 3);
> > >> > + bix->OEM[4] = '\0';
> > >>
> > >> snprintf() ?
> >
> > Ditto.
> >
> > >> > + snprintf(prefix, ARRAY_SIZE(prefix), "%s: ", bat0->name);
> > >>
> > >> > + prefix[127] = '\0';
> > >>
> > >> Why?
> > >
> > > Just me being paranoid in case the code doesn't follow the spec...
> > > Yeah, I'll remove it.
> >
> > snprintf() despite n in the name takes care of terminating NUL.
> >
> > >> > +static int mshw0011_probe(struct i2c_client *client,
> > >> > + const struct i2c_device_id *id) {
> > >>
> > >> > + data->notify_version = version == MSHW0011_EV_2_5;
> > >>
> > >> 0x1ff as version sounds hmm suspicious.
> > >
> > > So after a little bit of digging, it appears those values were taken
> > > from the DSDT:
> > > https://bugzilla.kernel.org/attachment.cgi?id=187171 line 11694.
> > >
> > > It appears 0x3F is EV 2.1 and before, and 0x1FF is EV 2.5 and above.
> > > The returned value is not a version of the chip, just a flag to know
> > > which path we are taking in the DSM.
> > >
> > > The name is probably not the best.
> >
> > 63 and 511 looks too suspicious to be a version. Rather block size, a
> > mask or alike.
>
> I replaced the 'version' by 'mask' in v3. It doesn't hurt to do so.
>
> >
> > >> > +static const struct i2c_device_id mshw0011_id[] = {
> > >> > + { }
> > >> > +};
> > >> > +MODULE_DEVICE_TABLE(i2c, mshw0011_id);
> > >>
> > >> ->probe_new(), please.
> > >
> > > Correct
> > >
> > >>
> > >> If I2C framework is _still_ broken we need to fix that part.
> > >
> > > I haven't check, so let's see for v3.
> >
> > Cc: Wolfram for v3 and ask him directly. Last time I checked it looks
> > like I2C core doesn't care about ACPI when ->probe_new() is used.
>
> Looks like things are working fine now. So I can just submit the driver
> without bothering the I2C core team :)
>
> Cheers,
> Benjamin
4 years