Hi, > -----Original Message----- > From: Hans de Goede [mailto:hdegoede@xxxxxxxxxx] > Sent: Saturday, January 20, 2018 4:48 AM > To: Schmauss, Erik <erik.schmauss@xxxxxxxxx> > Cc: Rafael J . Wysocki <rjw@xxxxxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>; > Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; Moore, Robert > <robert.moore@xxxxxxxxx>; Zheng, Lv <lv.zheng@xxxxxxxxx>; linux- > acpi@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx > Subject: Re: ACPI: Do not call _STA on battery devices with unmet dependencies > > Hi, > > On 19-01-18 22:03, Schmauss, Erik wrote: > > > >> -----Original Message----- > >> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- > >> owner@xxxxxxxxxxxxxxx] On Behalf Of Bjorn Helgaas > >> Sent: Thursday, January 18, 2018 11:11 AM > >> To: Hans de Goede <hdegoede@xxxxxxxxxx> > >> Cc: Rafael J . Wysocki <rjw@xxxxxxxxxxxxx>; Len Brown > >> <lenb@xxxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; Moore, Robert > >> <robert.moore@xxxxxxxxx>; Zheng, Lv <lv.zheng@xxxxxxxxx>; linux- > >> acpi@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx > >> Subject: Re: ACPI: Do not call _STA on battery devices with unmet > >> dependencies > >> > >> On Thu, Jan 18, 2018 at 05:03:55PM +0100, Hans de Goede wrote: > > > > Hi Hans, > > > >>> Hi All, > >>> > >>> The ACPI code already contains quite a bit of code to not bind the > >>> ACPI-battery until all deps for an ACPI battery device have been > >>> met, but on some devices calling _STA before all deps are met is a > >>> problem too because the _STA method uses an i2c OpRegion there. > > > > Could you explain why _STA method using an I2C OpRegion is problematic? > > It is problematic if we call the _STA method before the handler for the OpRegion > has been installed, this series delays / avoids calling _STA before the OpRegion > has been installed. Thanks for the explanation. I'm looking at the ACPI spec and from what it said about _STA, I gathered that _STA is allowed to have side-effects to named objects within the AML namespace. So removing _STA from acpi_get_object_info seems a little dangerous... Is there a reason why you cannot remove or delay the call to acpi_get_object_info instead? Chapter 6 section 1 of the ACPI spec also states that it's possible for _HID and _ADR to read from operation regions as well. Is it common convention that these _HID and _ADR objects do not access operation regions? Thanks, Erik > > Regards, > > Hans > > > > > > Thanks, > > Erik > > > >>> > >>> Here is the DSDT of the device I'm seeing this on: > >>> https://fedorapeople.org/~jwrdegoede/toshiba-click-mini-dsdt.dsl > >> > >> This looks like interesting info, but (a) this link isn't mentioned > >> in the actual patches, and (b) it's conceivable that fedorapeople.org could go > away someday. > >> If this were a PCI series, I would suggest opening a report at > >> bugzilla.kernel.org, attaching the DSL there, and including the link in the > patch changelog. > >> > >>> This series modifies the kernel to not call _STA until all deps are > >>> met, mirroring the binding behavior of the battery driver. > >>> > >>> Without this series a total of 32 ACPI errors get printend to the > >>> console on boot, there are 4 errors per _STA call, 2 battery devices > >>> on this system and 4 _STA calls per battery device. > >>> > >>> The first commit is a preparation commit for making the ACPICA > >>> changes in the 4th commit, this commit is necessary to not break > >>> things after the ACPICA changes. > >>> > >>> The second commit modifies acpi_bus_get_status to not call _STA on > >>> battery devices until all deps are met. This fixes 2 of the 4 too > >>> early _STA calls triggering these errors. > >>> > >>> The third commit makes the device instantiation code use > >>> acpi_bus_get_status instead of acpi_bus_get_status_handle so that > >>> the code to get the initial status also does not makes 1 too early _STA call. > >>> > >>> The fourth commit changes the ACPICA acpi_get_object_info function > >>> to not call _STA. Only 1 user (which is fixed in the first commit) > >>> cares about acpi_device_info.current_status. And the ACPICA code has > >>> this > >> comment: > >>> > >>> * Note: This interface is intended to be used during the initial > >>> device > >>> * discovery namespace traversal. Therefore, no complex methods can be > >>> * executed, especially those that access operation regions. > >>> Therefore, do > >>> * not add any additional methods that could cause problems in this area. > >>> * Because of this reason support for the following methods has > >>> been > >> removed: > >>> * this was the fate of the _SUB method which was found to cause such > >>> * problems and was removed (11/2015). > >>> > >>> The described problems with the _SUB method clearly also apply to > >>> the _STA method, so removing it from acpi_get_object_info seems like > >>> it is the right thing to do here. This too fixes 1 too early _STA > >>> call, so that with all > >>> 4 patches in place we've fixed all 4 too early _STA calls. > >>> > >>> Regards, > >>> > >>> Hans > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" > >>> in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > >>> info at http://vger.kernel.org/majordomo-info.html > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" > >> in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > >> info at http://vger.kernel.org/majordomo-info.html
