Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "Rafael J . Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>, linux-acpi@vger.kernel.org
Subject: [PATCH 0/7] ACPI: scan: Split root scanning into 2 steps
Date: Sat, 21 Nov 2020 21:30:33 +0100
Message-ID: <20201121203040.146252-1-hdegoede@redhat.com> (raw)

Hi Rafael,

A while ago (almost 2 years ago) I discussed an issue with you about
some devices, where some of the methods used during device-addition
(such as _HID) may rely on OpRegions of other devices:

https://www.spinics.net/lists/linux-acpi/msg86303.html

An example of this is the Acer Switch 10E SW3-016 model. The _HID method
of the ACPI node for the UART attached Bluetooth, reads GPIOs to detect
the installed wifi chip and update the _HID for the Bluetooth's ACPI node
accordingly. The current ACPI scan code calls _HID before the GPIO
controller's OpRegions are available, leading to the wrong _HID being
used and Bluetooth not working.

Last week I bought a second hand Acer device, not knowing it was this
exact model. Since I now have access to affected hardware I decided to
take a shot at fixing this.

In the discussion you suggested to split the acpi_bus_scan of the root
into 2 steps, first scan devices with an empty _DEP, putting other
acpi_handle-s on a list of deferred devices and then in step 2 scan the
rest.

I'm happy to report that, at least on the affected device, this works
nicely. While working on this I came up with a less drastic way to
deal with this. As you will see in patch 4 of this series, I decided
to first add a more KISS method of deciding which devices to defer
to the second scan step by matching by HID. This has the disadvantage
of not being a generic solution. But it has the advantage of being a
solution which does not potentially regress other devices.

Then in patch 5 I actually do add the option to defer or not based on
_DEP being empty. I've put this behind a kernel commandline option as
I'm not sure we should do this everywhere by default. At least no without
a lot more testing.

Patch 6 fixes an issue with patch 5 which causes battery devices to stop
working.

And patch 7 adds some extra HIDs to the list of HIDs which should be
ignored when checking if the _DEP list is empty from Linux' pov, iow
some extra HIDs which Linux does not bind to.

Please let me know what you think about this patch-set. I would be happy
to see just patches 1-4 merged.

If you dislike the HID match approach I can drop that and add a DMI-quirk
list of devices which need the new 2-step process (for now), to fix those
without regressing the OOTB experience on other devices.

Or we could just entirely switch to the new scheme in one big step, but
that seems a bit too adventurous.

Regards,

Hans


             reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21 20:30 Hans de Goede [this message]
2020-11-21 20:30 ` [PATCH 1/7] ACPI: scan: Add an acpi_info_matches_hids() helper Hans de Goede
2020-11-21 20:30 ` [PATCH 2/7] ACPI: scan: Call acpi_get_object_info() from acpi_add_single_object() Hans de Goede
2020-11-21 20:30 ` [PATCH 3/7] ACPI: scan: Add a separate cleanup exit-path to acpi_scan_init() Hans de Goede
2020-11-21 20:30 ` [PATCH 4/7] ACPI: scan: Split root scanning into 2 steps Hans de Goede
2020-11-21 20:30 ` [PATCH 5/7] ACPI: scan: Add support for deferring adding devices to the second scan phase based on the _DEP list Hans de Goede
2020-11-23 12:17   ` Rafael J. Wysocki
2020-11-23 13:30     ` Hans de Goede
2020-11-23 12:41       ` Rafael J. Wysocki
2020-11-23 13:49         ` Hans de Goede
2020-11-21 20:30 ` [PATCH 6/7] ACPI: scan: Fix battery devices not working with acpi.defer_scan_based_on_dep=1 Hans de Goede
2020-12-02 13:52   ` Rafael J. Wysocki
2020-11-21 20:30 ` [PATCH 7/7] ACPI: scan: Add some HIDs which are never bound on Cherry Trail devices to acpi_ignore_dep_hids Hans de Goede
2020-12-02 13:49 ` [PATCH 0/7] ACPI: scan: Split root scanning into 2 steps Rafael J. Wysocki
2020-12-02 15:51   ` Rafael J. Wysocki
2020-12-02 19:46     ` Rafael J. Wysocki
2020-12-02 19:39   ` Hans de Goede
2020-12-02 19:57     ` Rafael J. Wysocki
2020-12-03  9:53       ` Hans de Goede
2020-12-03 14:27         ` Rafael J. Wysocki
2020-12-05 15:44           ` Rafael J. Wysocki
2020-12-05 17:02             ` Hans de Goede
2020-12-07 17:27               ` Rafael J. Wysocki
2020-12-07 18:15                 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201121203040.146252-1-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git