RE: ACPI: Do not call _STA on battery devices with unmet dependencies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux