Re: [PATCH v4] ACPI / bus: Introduce a list of ids for "always present" devices

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

 



Hi,

On 18-04-17 10:31, Andy Shevchenko wrote:
On Mon, 2017-04-10 at 17:49 +0200, Hans de Goede wrote:
Several cherrytrail devices (all of which ship with windows 10) hide
the
lpss pwm controller in ACPI, typically the _STA method looks like
this:

CherryTrail
PWM
LPSS

All fixed.



    Method (_STA, 0, NotSerialized)  // _STA: Status
    {
        If (OSID == One)
        {
            Return (Zero)
        }

        Return (0x0F)
    }

Where OSID is some dark magic seen in all cherrytrail ACPI tables
making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.

This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.

Since loading the driver for this does no harm (the only in kernel
user
of it is the i915 driver, which will only use it when it needs it),
this
commit makes acpi_bus_get_status() always set status to
ACPI_STA_DEFAULT
for the 80862288 device, fixing the lack of backlight control.

+#ifdef CONFIG_X86
+/*
+ * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es
because
+ * some recent windows drivers bind to one device but poke at
multiple

Windows

Fixed for v6.

+ * devices at the same time, so the others get hidden.
+ * We work around this by always reporting ACPI_STA_DEFAULT for these
+ * devices. Note this MUST only be done for devices where this is
safe.
+ *
+ * This forcing of devices to be present is limited to specific CPU
(SoC)
+ * models both to avoid potentially causing trouble on other models
and
+ * because some HIDs are re-used on different SoCs for completely
+ * different devices.
+ */
+struct always_present_device_id {
+	struct acpi_device_id hid[2];
+	struct x86_cpu_id cpu_id[2];
+};
+

+#define ENTRY(hid, cpu_model) {					

	\
+	{ { hid, }, {} },						
\

+	{ { X86_VENDOR_INTEL, 6, cpu_model, X86_FEATURE_ANY, }, {} },
	\

Can we use separate macro for this. i.e. ICPU() ?

Fixed for v5.

Perhaps at some point we might switch to set of generic ICPU()-like
macros.

Yes if something like a generic form of the ICPU macro ever becomes
available then switching to it would be a good idea.

Moreover, it seems you may change it to use only one existing structure

ICPU(model, hid)

+}
+
+static const struct always_present_device_id
always_present_device_ids[] = {
+	/*
+	 * Cherrytrail pwm directly poked by GPU driver in win10,
+	 * but Linux uses a separate pwm driver, harmless if not
used.
+	 */
+	ENTRY("80862288", INTEL_FAM6_ATOM_AIRMONT),
+};
+#endif
+
+void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+	u32 *status = (u32 *)&adev->status;
+#ifdef CONFIG_X86
+	int i;
+
+	/* acpi_match_device_ids checks status, so start with default
*/
+	*status = ACPI_STA_DEFAULT;

+	for (i = 0; i < ARRAY_SIZE(always_present_device_ids); i++) {
+		if (acpi_match_device_ids(adev,
+			always_present_device_ids[i].hid) == 0 &&
+		    x86_match_cpu(always_present_device_ids[i].cpu_id
)) {

I don't like this. It looks a bit hackish. If we need more, than one hid
per CPU, we might just supply an array

Note this started with just HID matching, later the cpu-id match
got added for 2 reasons:
-Extra safety check to not force enable devices on other models then for
 which the quirk is intended
-Some HIDs get re-used (by Intel) on different platforms for completely
 different devices. E.g. the INT0002 HID is used on both Intel Menlow
 for the Menlow thermal management extension and on Bay Trail / Cherry
 Trail for a "virtual gpio" controller which is involved in wakeup
 from suspend

Basically the HID is leading, as we want to have a quirk for a
specific HID to get enabled even if its _STA returns 0 and the CPU-ID
check is an extra check, so for v6 I've modified it to take an array
of cpu-ids, so that we do not need duplicate table entries for when
we want to check the same HID on 2 CPU models.

Regards,

Hans





ICPU(model, hids)

+			dev_info(&adev->dev, "Device [%s] is in
always present list setting status [%08x]\n",
+				 adev->pnp.bus_id, ACPI_STA_DEFAULT);
+			return;
+		}
+	}
+#endif

--
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



[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