2017-08-01 12:10:46

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 0/5] Apple SPI properties

Retrieve device properties on Macs with an Apple-specific _DSM and
use them in lieu of _CRS data upon SPI slave initialization
in preparation of supporting the SPI keyboard on 2015+ MacBooks, v4.

Please refer to the cover letter of v1 for further details:
http://www.spinics.net/lists/linux-acpi/msg75537.html

Changes v3 -> v4:

Patch 1:
- Extend consolidation of Apple DMI checks to the entire tree
instead of just the ACPI core. (Rafael, Andy, Darren)

Patches 2 - 5 are as before.

Thanks,

Lukas


Lukas Wunner (5):
treewide: Consolidate Apple DMI checks
ACPI / property: Don't evaluate objects for devices w/o handle
ACPI / property: Support Apple _DSM properties
ACPI / scan: Recognize Apple SPI and I2C slaves
spi: Use Apple device properties in absence of ACPI resources

arch/x86/include/asm/setup.h | 1 +
arch/x86/kernel/early-quirks.c | 4 +-
arch/x86/kernel/quirks.c | 10 +++
arch/x86/kernel/setup.c | 2 +
drivers/acpi/Makefile | 1 +
drivers/acpi/internal.h | 6 ++
drivers/acpi/osi.c | 37 ++-------
drivers/acpi/pci_root.c | 4 +-
drivers/acpi/property.c | 6 ++
drivers/acpi/sbs.c | 25 +-----
drivers/acpi/scan.c | 7 ++
drivers/acpi/x86/apple.c | 141 ++++++++++++++++++++++++++++++++
drivers/firmware/efi/apple-properties.c | 5 +-
drivers/pci/quirks.c | 5 +-
drivers/spi/spi.c | 32 ++++++++
drivers/thunderbolt/icm.c | 13 +--
drivers/thunderbolt/tb.c | 4 +-
include/linux/platform_data/x86/apple.h | 13 +++
18 files changed, 244 insertions(+), 72 deletions(-)
create mode 100644 drivers/acpi/x86/apple.c
create mode 100644 include/linux/platform_data/x86/apple.h

--
2.11.0


2017-08-01 12:11:15

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 1/5] treewide: Consolidate Apple DMI checks

We're about to amend ACPI bus scan with DMI checks whether we're running
on a Mac to support Apple device properties in AML. The DMI checks are
performed for every single device, adding overhead for everything x86
that isn't Apple, which is the majority. Rafael and Andy therefore
request to perform the DMI match only once and cache the result.

Outside of ACPI various other Apple DMI checks exist and it seems
reasonable to use the cached value there as well. Rafael, Andy and
Darren suggest performing the DMI check in arch code and making it
available with a header in include/linux/platform_data/x86/.

To this end, add early_platform_quirks() to arch/x86/kernel/quirks.c
to perform the DMI check and invoke it from setup_arch(). Switch over
all existing Apple DMI checks, thereby fixing two deficiencies:

* They are now #defined to false on non-x86 arches and can thus be
optimized away if they're located in cross-arch code.

* Some of them only match "Apple Inc." but not "Apple Computer, Inc.",
which is used by BIOSes released between January 2006 (when the first
x86 Macs started shipping) and January 2007 (when the company name
changed upon introduction of the iPhone).

Cc: Lv Zheng <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Matt Fleming <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Andreas Noever <[email protected]>
Cc: Michael Jamet <[email protected]>
Cc: Yehezkel Bernat <[email protected]>
Cc: Mika Westerberg <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Suggested-by: Rafael J. Wysocki <[email protected]>
Suggested-by: Darren Hart <[email protected]>
Signed-off-by: Lukas Wunner <[email protected]>
---
Changes v3 -> v4:
- Extend consolidation of Apple DMI checks to the entire tree
instead of just the ACPI core. (Rafael, Andy, Darren)

arch/x86/include/asm/setup.h | 1 +
arch/x86/kernel/early-quirks.c | 4 ++--
arch/x86/kernel/quirks.c | 10 +++++++++
arch/x86/kernel/setup.c | 2 ++
drivers/acpi/osi.c | 37 +++++++--------------------------
drivers/acpi/pci_root.c | 4 ++--
drivers/acpi/sbs.c | 25 ++--------------------
drivers/firmware/efi/apple-properties.c | 5 ++---
drivers/pci/quirks.c | 5 +++--
drivers/thunderbolt/icm.c | 13 ++++--------
drivers/thunderbolt/tb.c | 4 ++--
include/linux/platform_data/x86/apple.h | 13 ++++++++++++
12 files changed, 51 insertions(+), 72 deletions(-)
create mode 100644 include/linux/platform_data/x86/apple.h

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index e4585a393965..a65cf544686a 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -39,6 +39,7 @@ static inline void vsmp_init(void) { }
#endif

void setup_bios_corruption_check(void);
+void early_platform_quirks(void);

extern unsigned long saved_video_mode;

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index d907c3d8633f..a70a65ae798a 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -12,10 +12,10 @@
#include <linux/pci.h>
#include <linux/acpi.h>
#include <linux/delay.h>
-#include <linux/dmi.h>
#include <linux/pci_ids.h>
#include <linux/bcma/bcma.h>
#include <linux/bcma/bcma_regs.h>
+#include <linux/platform_data/x86/apple.h>
#include <drm/i915_drm.h>
#include <asm/pci-direct.h>
#include <asm/dma.h>
@@ -593,7 +593,7 @@ static void __init apple_airport_reset(int bus, int slot, int func)
u64 addr;
int i;

- if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
+ if (!x86_apple_machine)
return;

/* Card may have been put into PCI_D3hot by grub quirk */
diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 0bee04d41bed..eaa591cfd98b 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -1,6 +1,7 @@
/*
* This file contains work-arounds for x86 and x86_64 platform bugs.
*/
+#include <linux/dmi.h>
#include <linux/pci.h>
#include <linux/irq.h>

@@ -656,3 +657,12 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, quirk_intel_brickland_xeon_
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2083, quirk_intel_purley_xeon_ras_cap);
#endif
#endif
+
+bool x86_apple_machine;
+EXPORT_SYMBOL(x86_apple_machine);
+
+void __init early_platform_quirks(void)
+{
+ x86_apple_machine = dmi_match(DMI_SYS_VENDOR, "Apple Inc.") ||
+ dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc.");
+}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3486d0498800..77b3c3af7cba 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1206,6 +1206,8 @@ void __init setup_arch(char **cmdline_p)

io_delay_init();

+ early_platform_quirks();
+
/*
* Parse the ACPI tables for possible boot-time SMP configuration.
*/
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index 723bee58bbcf..19cdd8a783a9 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -27,6 +27,7 @@
#include <linux/kernel.h>
#include <linux/acpi.h>
#include <linux/dmi.h>
+#include <linux/platform_data/x86/apple.h>

#include "internal.h"

@@ -257,12 +258,11 @@ bool acpi_osi_is_win8(void)
}
EXPORT_SYMBOL(acpi_osi_is_win8);

-static void __init acpi_osi_dmi_darwin(bool enable,
- const struct dmi_system_id *d)
+static void __init acpi_osi_dmi_darwin(void)
{
- pr_notice("DMI detected to setup _OSI(\"Darwin\"): %s\n", d->ident);
+ pr_notice("DMI detected to setup _OSI(\"Darwin\"): Apple hardware\n");
osi_config.darwin_dmi = 1;
- __acpi_osi_setup_darwin(enable);
+ __acpi_osi_setup_darwin(true);
}

static void __init acpi_osi_dmi_linux(bool enable,
@@ -273,13 +273,6 @@ static void __init acpi_osi_dmi_linux(bool enable,
__acpi_osi_setup_linux(enable);
}

-static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d)
-{
- acpi_osi_dmi_darwin(true, d);
-
- return 0;
-}
-
static int __init dmi_enable_osi_linux(const struct dmi_system_id *d)
{
acpi_osi_dmi_linux(true, d);
@@ -481,30 +474,16 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
DMI_MATCH(DMI_PRODUCT_NAME, "1015PX"),
},
},
-
- /*
- * Enable _OSI("Darwin") for all apple platforms.
- */
- {
- .callback = dmi_enable_osi_darwin,
- .ident = "Apple hardware",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
- },
- },
- {
- .callback = dmi_enable_osi_darwin,
- .ident = "Apple hardware",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, Inc."),
- },
- },
{}
};

static __init void acpi_osi_dmi_blacklisted(void)
{
dmi_check_system(acpi_osi_dmi_table);
+
+ /* Enable _OSI("Darwin") for Apple platforms. */
+ if (x86_apple_machine)
+ acpi_osi_dmi_darwin();
}

int __init early_acpi_osi_init(void)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 9eec3095e6c3..6fc204a52493 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -33,6 +33,7 @@
#include <linux/acpi.h>
#include <linux/slab.h>
#include <linux/dmi.h>
+#include <linux/platform_data/x86/apple.h>
#include <acpi/apei.h> /* for acpi_hest_init() */

#include "internal.h"
@@ -431,8 +432,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
* been called successfully. We know the feature set supported by the
* platform, so avoid calling _OSC at all
*/
-
- if (dmi_match(DMI_SYS_VENDOR, "Apple Inc.")) {
+ if (x86_apple_machine) {
root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
decode_osc_control(root, "OS assumes control of",
root->osc_control_set);
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index ad0b13ad4bbb..a18463799ad7 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -31,7 +31,7 @@
#include <linux/jiffies.h>
#include <linux/delay.h>
#include <linux/power_supply.h>
-#include <linux/dmi.h>
+#include <linux/platform_data/x86/apple.h>

#include "sbshc.h"
#include "battery.h"
@@ -58,8 +58,6 @@ static unsigned int cache_time = 1000;
module_param(cache_time, uint, 0644);
MODULE_PARM_DESC(cache_time, "cache time in milliseconds");

-static bool sbs_manager_broken;
-
#define MAX_SBS_BAT 4
#define ACPI_SBS_BLOCK_MAX 32

@@ -632,31 +630,12 @@ static void acpi_sbs_callback(void *context)
}
}

-static int disable_sbs_manager(const struct dmi_system_id *d)
-{
- sbs_manager_broken = true;
- return 0;
-}
-
-static struct dmi_system_id acpi_sbs_dmi_table[] = {
- {
- .callback = disable_sbs_manager,
- .ident = "Apple",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc.")
- },
- },
- { },
-};
-
static int acpi_sbs_add(struct acpi_device *device)
{
struct acpi_sbs *sbs;
int result = 0;
int id;

- dmi_check_system(acpi_sbs_dmi_table);
-
sbs = kzalloc(sizeof(struct acpi_sbs), GFP_KERNEL);
if (!sbs) {
result = -ENOMEM;
@@ -677,7 +656,7 @@ static int acpi_sbs_add(struct acpi_device *device)

result = 0;

- if (!sbs_manager_broken) {
+ if (!x86_apple_machine) {
result = acpi_manager_get_info(sbs);
if (!result) {
sbs->manager_present = 1;
diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c
index c473f4c5ca34..9f6bcf173b0e 100644
--- a/drivers/firmware/efi/apple-properties.c
+++ b/drivers/firmware/efi/apple-properties.c
@@ -18,8 +18,8 @@
#define pr_fmt(fmt) "apple-properties: " fmt

#include <linux/bootmem.h>
-#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/platform_data/x86/apple.h>
#include <linux/property.h>
#include <linux/slab.h>
#include <linux/ucs2_string.h>
@@ -191,8 +191,7 @@ static int __init map_properties(void)
u64 pa_data;
int ret;

- if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc.") &&
- !dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc."))
+ if (!x86_apple_machine)
return 0;

pa_data = boot_params.hdr.setup_data;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6967c6b4cf6b..ba7b7c64379e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -25,6 +25,7 @@
#include <linux/sched.h>
#include <linux/ktime.h>
#include <linux/mm.h>
+#include <linux/platform_data/x86/apple.h>
#include <asm/dma.h> /* isa_dma_bridge_buggy */
#include "pci.h"

@@ -3447,7 +3448,7 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
{
acpi_handle bridge, SXIO, SXFP, SXLV;

- if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
+ if (!x86_apple_machine)
return;
if (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM)
return;
@@ -3492,7 +3493,7 @@ static void quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
struct pci_dev *sibling = NULL;
struct pci_dev *nhi = NULL;

- if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
+ if (!x86_apple_machine)
return;
if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
return;
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index bdaac1ff00a5..53250fc057e1 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -13,9 +13,9 @@
*/

#include <linux/delay.h>
-#include <linux/dmi.h>
#include <linux/mutex.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/apple.h>
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
@@ -102,11 +102,6 @@ static inline u64 get_route(u32 route_hi, u32 route_lo)
return (u64)route_hi << 32 | route_lo;
}

-static inline bool is_apple(void)
-{
- return dmi_match(DMI_BOARD_VENDOR, "Apple Inc.");
-}
-
static bool icm_match(const struct tb_cfg_request *req,
const struct ctl_pkg *pkg)
{
@@ -176,7 +171,7 @@ static int icm_request(struct tb *tb, const void *request, size_t request_size,

static bool icm_fr_is_supported(struct tb *tb)
{
- return !is_apple();
+ return !x86_apple_machine;
}

static inline int icm_fr_get_switch_index(u32 port)
@@ -517,7 +512,7 @@ static bool icm_ar_is_supported(struct tb *tb)
* Starting from Alpine Ridge we can use ICM on Apple machines
* as well. We just need to reset and re-enable it first.
*/
- if (!is_apple())
+ if (!x86_apple_machine)
return true;

/*
@@ -1011,7 +1006,7 @@ static int icm_start(struct tb *tb)
* don't provide images publicly either. To be on the safe side
* prevent root switch NVM upgrade on Macs for now.
*/
- tb->root_switch->no_nvm_upgrade = is_apple();
+ tb->root_switch->no_nvm_upgrade = x86_apple_machine;

ret = tb_switch_add(tb->root_switch);
if (ret)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 1b02ca0b6129..0b22ad9d68b4 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -7,7 +7,7 @@
#include <linux/slab.h>
#include <linux/errno.h>
#include <linux/delay.h>
-#include <linux/dmi.h>
+#include <linux/platform_data/x86/apple.h>

#include "tb.h"
#include "tb_regs.h"
@@ -453,7 +453,7 @@ struct tb *tb_probe(struct tb_nhi *nhi)
struct tb_cm *tcm;
struct tb *tb;

- if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
+ if (!x86_apple_machine)
return NULL;

tb = tb_domain_alloc(nhi, sizeof(*tcm));
diff --git a/include/linux/platform_data/x86/apple.h b/include/linux/platform_data/x86/apple.h
new file mode 100644
index 000000000000..079e816c3c21
--- /dev/null
+++ b/include/linux/platform_data/x86/apple.h
@@ -0,0 +1,13 @@
+#ifndef PLATFORM_DATA_X86_APPLE_H
+#define PLATFORM_DATA_X86_APPLE_H
+
+#ifdef CONFIG_X86
+/**
+ * x86_apple_machine - whether the machine is an x86 Apple Macintosh
+ */
+extern bool x86_apple_machine;
+#else
+#define x86_apple_machine false
+#endif
+
+#endif
--
2.11.0

2017-08-01 12:11:36

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 2/5] ACPI / property: Don't evaluate objects for devices w/o handle

Fabricated devices such as LNXPWRBN lack a handle, causing evaluation
of _CCA and _DSD to always fail with AE_BAD_PARAMETER. While that is
merely a (negligible) waste of processing power, evaluating a _DSM for
them (such as Apple's device properties _DSM which we're about to add)
results in an ugly error:

ACPI: \: failed to evaluate _DSM (0x1001)

Avoid by not evaluating _DSD and the upcoming _DSM for devices without
handle.

Cc: Andy Shevchenko <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
Signed-off-by: Lukas Wunner <[email protected]>
---
drivers/acpi/property.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a65c09cc223f..834e01bee015 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -338,6 +338,9 @@ void acpi_init_properties(struct acpi_device *adev)

INIT_LIST_HEAD(&adev->data.subnodes);

+ if (!adev->handle)
+ return;
+
/*
* Check if ACPI_DT_NAMESPACE_HID is present and inthat case we fill in
* Device Tree compatible properties for this device.
--
2.11.0

2017-08-01 12:12:05

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 3/5] ACPI / property: Support Apple _DSM properties

While the rest of the world has standardized on _DSD as the way to store
device properties in AML (introduced with ACPI 5.1 in 2014), Apple has
been using a custom _DSM to achieve the same for much longer (ever since
they switched from DeviceTree-based PowerPC to Intel in 2005, verified
with MacOS X 10.4.11).

The theory of operation on macOS is as follows: AppleACPIPlatform.kext
invokes mergeEFIproperties() and mergeDSMproperties() for each device to
merge properties conveyed by EFI drivers as well as properties stored in
AML into the I/O Kit registry from which they can be retrieved by
drivers. We've been supporting EFI properties since commit 58c5475aba67
("x86/efi: Retrieve and assign Apple device properties"). The present
commit adds support for _DSM properties, thereby completing our support
for Apple device properties. The _DSM properties are made available
under the primary fwnode, the EFI properties under the secondary fwnode.
So for devices which possess both property types, they can all be
elegantly accessed with the uniform API in <linux/property.h>.

Until recently we had no need to support _DSM properties, they contained
only uninteresting garbage. The situation has changed with MacBooks and
MacBook Pros introduced since 2015: Their keyboard is attached with SPI
instead of USB and the _CRS data which is necessary to initialize the
spi driver only contains valid information if OSPM responds "false" to
_OSI("Darwin"). If OSPM responds "true", _CRS is empty and the spi
driver fails to initialize. The rationale is very simple, Apple only
cares about macOS and Windows: On Windows, _CRS contains valid data,
whereas on macOS it is empty. Instead, macOS gleans the necessary data
from the _DSM properties.

Since Linux deliberately defaults to responding "true" to _OSI("Darwin"),
we need to emulate macOS' behaviour by initializing the spi driver with
data returned by the _DSM.

An out-of-tree driver for the SPI keyboard exists which currently binds
to the ACPI device, invokes the _DSM, parses the returned package and
instantiates an SPI device with the data gleaned from the _DSM:
https://github.com/cb22/macbook12-spi-driver/commit/9a416d699ef4
https://github.com/cb22/macbook12-spi-driver/commit/0c34936ed9a1

By adding support for Apple's _DSM properties in generic ACPI code, the
out-of-tree driver will be able to register as a regular SPI driver,
significantly reducing its amount of code and improving its chances to
be mainlined.

The SPI keyboard will not be the only user of this commit: E.g. on the
MacBook8,1, the UART-attached Bluetooth device likewise returns empty
_CRS data if OSPM returns "true" to _OSI("Darwin").

The _DSM returns a Package whose format unfortunately deviates slightly
from the _DSD spec: The properties are marshalled up in a single Package
as alternating key/value elements, unlike _DSD which stores them as a
Package of 2-element Packages. The present commit therefore converts
the Package to _DSD format and the ACPI core can then treat the data as
if Apple would follow the standard.

Well, except for one small annoyance: The properties returned by the
_DSM only ever have one of two types, Integer or Buffer. The former is
retrievable as usual with device_property_read_u64(), but the latter is
not part of the _DSD spec and it is not possible to retrieve Buffer
properties with the device_property_read_*() functions due to the type
checking performed in drivers/acpi/property.c. It is however possible
to retrieve them with acpi_dev_get_property(). Apple is using the
Buffer type somewhat sloppily to store null-terminated strings but also
integers. The real data type is not distinguishable by the ACPI core
and the onus is on the caller to use the contents of the Buffer in an
appropriate way.

In case Apple moves to _DSD in the future, this commit first checks for
_DSD and falls back to _DSM only if _DSD is not found.

Cc: Federico Lorenzi <[email protected]>
Tested-by: Ronald Tschalär <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Lukas Wunner <[email protected]>
---
drivers/acpi/Makefile | 1 +
drivers/acpi/internal.h | 6 ++
drivers/acpi/property.c | 3 +
drivers/acpi/x86/apple.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 151 insertions(+)
create mode 100644 drivers/acpi/x86/apple.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc62b1f..90265ab4437a 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -50,6 +50,7 @@ acpi-$(CONFIG_ACPI_REDUCED_HARDWARE_ONLY) += evged.o
acpi-y += sysfs.o
acpi-y += property.o
acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
+acpi-$(CONFIG_X86) += x86/apple.o
acpi-$(CONFIG_X86) += x86/utils.o
acpi-$(CONFIG_DEBUG_FS) += debugfs.o
acpi-$(CONFIG_ACPI_NUMA) += numa.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 58dd7ab3c653..ee066d74f5bf 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -233,6 +233,12 @@ static inline void suspend_nvs_restore(void) {}
void acpi_init_properties(struct acpi_device *adev);
void acpi_free_properties(struct acpi_device *adev);

+#ifdef CONFIG_X86
+void acpi_extract_apple_properties(struct acpi_device *adev);
+#else
+static inline void acpi_extract_apple_properties(struct acpi_device *adev) {}
+#endif
+
/*--------------------------------------------------------------------------
Watchdog
-------------------------------------------------------------------------- */
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 834e01bee015..aea98713a017 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -375,6 +375,9 @@ void acpi_init_properties(struct acpi_device *adev)
if (acpi_of && !adev->flags.of_compatible_ok)
acpi_handle_info(adev->handle,
ACPI_DT_NAMESPACE_HID " requires 'compatible' property\n");
+
+ if (!adev->data.pointer)
+ acpi_extract_apple_properties(adev);
}

static void acpi_destroy_nondev_subnodes(struct list_head *list)
diff --git a/drivers/acpi/x86/apple.c b/drivers/acpi/x86/apple.c
new file mode 100644
index 000000000000..51b4cf9f25da
--- /dev/null
+++ b/drivers/acpi/x86/apple.c
@@ -0,0 +1,141 @@
+/*
+ * apple.c - Apple ACPI quirks
+ * Copyright (C) 2017 Lukas Wunner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2) as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitmap.h>
+#include <linux/platform_data/x86/apple.h>
+#include <linux/uuid.h>
+
+/* Apple _DSM device properties GUID */
+static const guid_t apple_prp_guid =
+ GUID_INIT(0xa0b5b7c6, 0x1318, 0x441c,
+ 0xb0, 0xc9, 0xfe, 0x69, 0x5e, 0xaf, 0x94, 0x9b);
+
+/**
+ * acpi_extract_apple_properties - retrieve and convert Apple _DSM properties
+ * @adev: ACPI device for which to retrieve the properties
+ *
+ * Invoke Apple's custom _DSM once to check the protocol version and once more
+ * to retrieve the properties. They are marshalled up in a single package as
+ * alternating key/value elements, unlike _DSD which stores them as a package
+ * of 2-element packages. Convert to _DSD format and make them available under
+ * the primary fwnode.
+ */
+void acpi_extract_apple_properties(struct acpi_device *adev)
+{
+ unsigned int i, j = 0, newsize = 0, numprops, numvalid;
+ union acpi_object *props, *newprops;
+ unsigned long *valid = NULL;
+ void *free_space;
+
+ if (!x86_apple_machine)
+ return;
+
+ props = acpi_evaluate_dsm_typed(adev->handle, &apple_prp_guid, 1, 0,
+ NULL, ACPI_TYPE_BUFFER);
+ if (!props)
+ return;
+
+ if (!props->buffer.length)
+ goto out_free;
+
+ if (props->buffer.pointer[0] != 3) {
+ acpi_handle_info(adev->handle, FW_INFO
+ "unsupported properties version %*ph\n",
+ props->buffer.length, props->buffer.pointer);
+ goto out_free;
+ }
+
+ ACPI_FREE(props);
+ props = acpi_evaluate_dsm_typed(adev->handle, &apple_prp_guid, 1, 1,
+ NULL, ACPI_TYPE_PACKAGE);
+ if (!props)
+ return;
+
+ numprops = props->package.count / 2;
+ if (!numprops)
+ goto out_free;
+
+ valid = kcalloc(BITS_TO_LONGS(numprops), sizeof(long), GFP_KERNEL);
+ if (!valid)
+ goto out_free;
+
+ /* newsize = key length + value length of each tuple */
+ for (i = 0; i < numprops; i++) {
+ union acpi_object *key = &props->package.elements[i * 2];
+ union acpi_object *val = &props->package.elements[i * 2 + 1];
+
+ if ( key->type != ACPI_TYPE_STRING ||
+ (val->type != ACPI_TYPE_INTEGER &&
+ val->type != ACPI_TYPE_BUFFER))
+ continue; /* skip invalid properties */
+
+ __set_bit(i, valid);
+ newsize += key->string.length + 1;
+ if ( val->type == ACPI_TYPE_BUFFER)
+ newsize += val->buffer.length;
+ }
+
+ numvalid = bitmap_weight(valid, numprops);
+ if (numprops > numvalid)
+ acpi_handle_info(adev->handle, FW_INFO
+ "skipped %u properties: wrong type\n",
+ numprops - numvalid);
+ if (numvalid == 0)
+ goto out_free;
+
+ /* newsize += top-level package + 3 objects for each key/value tuple */
+ newsize += (1 + 3 * numvalid) * sizeof(union acpi_object);
+ newprops = ACPI_ALLOCATE_ZEROED(newsize);
+ if (!newprops)
+ goto out_free;
+
+ /* layout: top-level package | packages | key/value tuples | strings */
+ newprops->type = ACPI_TYPE_PACKAGE;
+ newprops->package.count = numvalid;
+ newprops->package.elements = &newprops[1];
+ free_space = &newprops[1 + 3 * numvalid];
+
+ for_each_set_bit(i, valid, numprops) {
+ union acpi_object *key = &props->package.elements[i * 2];
+ union acpi_object *val = &props->package.elements[i * 2 + 1];
+ unsigned int k = 1 + numvalid + j * 2; /* index into newprops */
+ unsigned int v = k + 1;
+
+ newprops[1 + j].type = ACPI_TYPE_PACKAGE;
+ newprops[1 + j].package.count = 2;
+ newprops[1 + j].package.elements = &newprops[k];
+
+ newprops[k].type = ACPI_TYPE_STRING;
+ newprops[k].string.length = key->string.length;
+ newprops[k].string.pointer = free_space;
+ memcpy(free_space, key->string.pointer, key->string.length);
+ free_space += key->string.length + 1;
+
+ newprops[v].type = val->type;
+ if (val->type == ACPI_TYPE_INTEGER) {
+ newprops[v].integer.value = val->integer.value;
+ } else {
+ newprops[v].buffer.length = val->buffer.length;
+ newprops[v].buffer.pointer = free_space;
+ memcpy(free_space, val->buffer.pointer,
+ val->buffer.length);
+ free_space += val->buffer.length;
+ }
+ j++; /* count valid properties */
+ }
+ WARN_ON(free_space != (void *)newprops + newsize);
+
+ adev->data.properties = newprops;
+ adev->data.pointer = newprops;
+
+out_free:
+ ACPI_FREE(props);
+ kfree(valid);
+}
--
2.11.0

2017-08-01 12:12:27

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 4/5] ACPI / scan: Recognize Apple SPI and I2C slaves

SPI and I2C slaves are enumerated by their respective parents rather
than the ACPI core. They are recognized by presence of _CRS resources,
which however are missing on Macs. Check for presence of device
properties instead.

Cc: Federico Lorenzi <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Reported-and-tested-by: Ronald Tschalär <[email protected]>
Signed-off-by: Lukas Wunner <[email protected]>
---
drivers/acpi/scan.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 943536c9a2a8..71a067c412a1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -13,6 +13,7 @@
#include <linux/dmi.h>
#include <linux/nls.h>
#include <linux/dma-mapping.h>
+#include <linux/platform_data/x86/apple.h>

#include <asm/pgtable.h>

@@ -1452,6 +1453,12 @@ static bool acpi_is_spi_i2c_slave(struct acpi_device *device)
struct list_head resource_list;
bool is_spi_i2c_slave = false;

+ /* Macs use device properties in lieu of _CRS resources */
+ if (x86_apple_machine &&
+ (fwnode_property_present(&device->fwnode, "spiSclkPeriod") ||
+ fwnode_property_present(&device->fwnode, "i2cAddress")))
+ return true;
+
INIT_LIST_HEAD(&resource_list);
acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave,
&is_spi_i2c_slave);
--
2.11.0

2017-08-01 12:12:51

by Lukas Wunner

[permalink] [raw]
Subject: [PATCH v4 5/5] spi: Use Apple device properties in absence of ACPI resources

MacBooks and MacBook Pros introduced since 2015 return empty _CRS data
for SPI slaves, causing device initialization to fail. Most of the
information that would normally be conveyed via _CRS is available
through ACPI device properties instead, so take advantage of them.

The meaning and appropriate usage of the device properties was reverse
engineered by Ronald Tschalär and carried over from these commits
authored by him:

https://github.com/cb22/macbook12-spi-driver/commit/9a416d699ef4
https://github.com/cb22/macbook12-spi-driver/commit/0c34936ed9a1

According to Ronald, the device properties have the following meaning:

spiSclkPeriod /* period in ns */
spiWordSize /* in number of bits */
spiBitOrder /* 1 = MSB_FIRST, 0 = LSB_FIRST */
spiSPO /* clock polarity: 0 = low, 1 = high */
spiSPH /* clock phase: 0 = first, 1 = second */
spiCSDelay /* delay between cs and receive on reads in 10 us */
resetA2RUsec /* active-to-receive delay? */
resetRecUsec /* receive delay? */

Cc: Federico Lorenzi <[email protected]>
Reported-by: Leif Liddy <[email protected]>
Tested-by: Ronald Tschalär <[email protected]>
Acked-by: Mark Brown <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Lukas Wunner <[email protected]>
---
drivers/spi/spi.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4fcbb0aa71d3..7d920ea19957 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -40,6 +40,7 @@
#include <linux/ioport.h>
#include <linux/acpi.h>
#include <linux/highmem.h>
+#include <linux/platform_data/x86/apple.h>

#define CREATE_TRACE_POINTS
#include <trace/events/spi.h>
@@ -1693,6 +1694,35 @@ static void of_register_spi_devices(struct spi_controller *ctlr) { }
#endif

#ifdef CONFIG_ACPI
+static void acpi_spi_parse_apple_properties(struct spi_device *spi)
+{
+ struct acpi_device *dev = ACPI_COMPANION(&spi->dev);
+ const union acpi_object *obj;
+
+ if (!x86_apple_machine)
+ return;
+
+ if (!acpi_dev_get_property(dev, "spiSclkPeriod", ACPI_TYPE_BUFFER, &obj)
+ && obj->buffer.length >= 4)
+ spi->max_speed_hz = NSEC_PER_SEC / *(u32 *)obj->buffer.pointer;
+
+ if (!acpi_dev_get_property(dev, "spiWordSize", ACPI_TYPE_BUFFER, &obj)
+ && obj->buffer.length == 8)
+ spi->bits_per_word = *(u64 *)obj->buffer.pointer;
+
+ if (!acpi_dev_get_property(dev, "spiBitOrder", ACPI_TYPE_BUFFER, &obj)
+ && obj->buffer.length == 8 && !*(u64 *)obj->buffer.pointer)
+ spi->mode |= SPI_LSB_FIRST;
+
+ if (!acpi_dev_get_property(dev, "spiSPO", ACPI_TYPE_BUFFER, &obj)
+ && obj->buffer.length == 8 && *(u64 *)obj->buffer.pointer)
+ spi->mode |= SPI_CPOL;
+
+ if (!acpi_dev_get_property(dev, "spiSPH", ACPI_TYPE_BUFFER, &obj)
+ && obj->buffer.length == 8 && *(u64 *)obj->buffer.pointer)
+ spi->mode |= SPI_CPHA;
+}
+
static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
{
struct spi_device *spi = data;
@@ -1766,6 +1796,8 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
acpi_spi_add_resource, spi);
acpi_dev_free_resource_list(&resource_list);

+ acpi_spi_parse_apple_properties(spi);
+
if (ret < 0 || !spi->max_speed_hz) {
spi_dev_put(spi);
return AE_OK;
--
2.11.0

2017-08-01 12:37:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] treewide: Consolidate Apple DMI checks

On Tue, 2017-08-01 at 14:10 +0200, Lukas Wunner wrote:
> We're about to amend ACPI bus scan with DMI checks whether we're
> running
> on a Mac to support Apple device properties in AML.  The DMI checks
> are
> performed for every single device, adding overhead for everything x86
> that isn't Apple, which is the majority.  Rafael and Andy therefore
> request to perform the DMI match only once and cache the result.
>
> Outside of ACPI various other Apple DMI checks exist and it seems
> reasonable to use the cached value there as well.  Rafael, Andy and
> Darren suggest performing the DMI check in arch code and making it
> available with a header in include/linux/platform_data/x86/.
>
> To this end, add early_platform_quirks() to arch/x86/kernel/quirks.c
> to perform the DMI check and invoke it from setup_arch().  Switch over
> all existing Apple DMI checks, thereby fixing two deficiencies:
>
> * They are now #defined to false on non-x86 arches and can thus be
>   optimized away if they're located in cross-arch code.
>
> * Some of them only match "Apple Inc." but not "Apple Computer, Inc.",
>   which is used by BIOSes released between January 2006 (when the
> first
>   x86 Macs started shipping) and January 2007 (when the company name
>   changed upon introduction of the iPhone).


I like the idea, though I can repeat what I commented on your Github
page.

We might need to distinguish 2006 vs 2007 Apple hardware. Thus, my
proposal was to use unsigned int (as bitwise flags) instead of bool and
provide two definitions for the hardware. Set those bits accordingly.
In case of most of the checks it will be the same as in this patch, but
leaves a flexibility of a choice.

>
> Cc: Lv Zheng <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Matt Fleming <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Andreas Noever <[email protected]>
> Cc: Michael Jamet <[email protected]>
> Cc: Yehezkel Bernat <[email protected]>
> Cc: Mika Westerberg <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Suggested-by: Rafael J. Wysocki <[email protected]>
> Suggested-by: Darren Hart <[email protected]>
> Signed-off-by: Lukas Wunner <[email protected]>
> ---
> Changes v3 -> v4:
> - Extend consolidation of Apple DMI checks to the entire tree
>   instead of just the ACPI core. (Rafael, Andy, Darren)
>
>  arch/x86/include/asm/setup.h            |  1 +
>  arch/x86/kernel/early-quirks.c          |  4 ++--
>  arch/x86/kernel/quirks.c                | 10 +++++++++
>  arch/x86/kernel/setup.c                 |  2 ++
>  drivers/acpi/osi.c                      | 37 +++++++-----------------
> ---------
>  drivers/acpi/pci_root.c                 |  4 ++--
>  drivers/acpi/sbs.c                      | 25 ++--------------------
>  drivers/firmware/efi/apple-properties.c |  5 ++---
>  drivers/pci/quirks.c                    |  5 +++--
>  drivers/thunderbolt/icm.c               | 13 ++++--------
>  drivers/thunderbolt/tb.c                |  4 ++--
>  include/linux/platform_data/x86/apple.h | 13 ++++++++++++
>  12 files changed, 51 insertions(+), 72 deletions(-)
>  create mode 100644 include/linux/platform_data/x86/apple.h
>
> diff --git a/arch/x86/include/asm/setup.h
> b/arch/x86/include/asm/setup.h
> index e4585a393965..a65cf544686a 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -39,6 +39,7 @@ static inline void vsmp_init(void) { }
>  #endif
>  
>  void setup_bios_corruption_check(void);
> +void early_platform_quirks(void);
>  
>  extern unsigned long saved_video_mode;
>  
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-
> quirks.c
> index d907c3d8633f..a70a65ae798a 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -12,10 +12,10 @@
>  #include <linux/pci.h>
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
> -#include <linux/dmi.h>
>  #include <linux/pci_ids.h>
>  #include <linux/bcma/bcma.h>
>  #include <linux/bcma/bcma_regs.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <drm/i915_drm.h>
>  #include <asm/pci-direct.h>
>  #include <asm/dma.h>
> @@ -593,7 +593,7 @@ static void __init apple_airport_reset(int bus,
> int slot, int func)
>   u64 addr;
>   int i;
>  
> - if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
> + if (!x86_apple_machine)
>   return;
>  
>   /* Card may have been put into PCI_D3hot by grub quirk */
> diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
> index 0bee04d41bed..eaa591cfd98b 100644
> --- a/arch/x86/kernel/quirks.c
> +++ b/arch/x86/kernel/quirks.c
> @@ -1,6 +1,7 @@
>  /*
>   * This file contains work-arounds for x86 and x86_64 platform bugs.
>   */
> +#include <linux/dmi.h>
>  #include <linux/pci.h>
>  #include <linux/irq.h>
>  
> @@ -656,3 +657,12 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,
> 0x6fc0, quirk_intel_brickland_xeon_
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2083,
> quirk_intel_purley_xeon_ras_cap);
>  #endif
>  #endif
> +
> +bool x86_apple_machine;
> +EXPORT_SYMBOL(x86_apple_machine);
> +
> +void __init early_platform_quirks(void)
> +{
> + x86_apple_machine = dmi_match(DMI_SYS_VENDOR, "Apple Inc.")
> ||
> +     dmi_match(DMI_SYS_VENDOR, "Apple
> Computer, Inc.");
> +}
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 3486d0498800..77b3c3af7cba 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1206,6 +1206,8 @@ void __init setup_arch(char **cmdline_p)
>  
>   io_delay_init();
>  
> + early_platform_quirks();
> +
>   /*
>    * Parse the ACPI tables for possible boot-time SMP
> configuration.
>    */
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index 723bee58bbcf..19cdd8a783a9 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -27,6 +27,7 @@
>  #include <linux/kernel.h>
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
> +#include <linux/platform_data/x86/apple.h>
>  
>  #include "internal.h"
>  
> @@ -257,12 +258,11 @@ bool acpi_osi_is_win8(void)
>  }
>  EXPORT_SYMBOL(acpi_osi_is_win8);
>  
> -static void __init acpi_osi_dmi_darwin(bool enable,
> -        const struct dmi_system_id *d)
> +static void __init acpi_osi_dmi_darwin(void)
>  {
> - pr_notice("DMI detected to setup _OSI(\"Darwin\"): %s\n", d-
> >ident);
> + pr_notice("DMI detected to setup _OSI(\"Darwin\"): Apple
> hardware\n");
>   osi_config.darwin_dmi = 1;
> - __acpi_osi_setup_darwin(enable);
> + __acpi_osi_setup_darwin(true);
>  }
>  
>  static void __init acpi_osi_dmi_linux(bool enable,
> @@ -273,13 +273,6 @@ static void __init acpi_osi_dmi_linux(bool
> enable,
>   __acpi_osi_setup_linux(enable);
>  }
>  
> -static int __init dmi_enable_osi_darwin(const struct dmi_system_id
> *d)
> -{
> - acpi_osi_dmi_darwin(true, d);
> -
> - return 0;
> -}
> -
>  static int __init dmi_enable_osi_linux(const struct dmi_system_id *d)
>  {
>   acpi_osi_dmi_linux(true, d);
> @@ -481,30 +474,16 @@ static struct dmi_system_id acpi_osi_dmi_table[]
> __initdata = {
>        DMI_MATCH(DMI_PRODUCT_NAME, "1015PX"),
>   },
>   },
> -
> - /*
> -  * Enable _OSI("Darwin") for all apple platforms.
> -  */
> - {
> - .callback = dmi_enable_osi_darwin,
> - .ident = "Apple hardware",
> - .matches = {
> -      DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> - },
> - },
> - {
> - .callback = dmi_enable_osi_darwin,
> - .ident = "Apple hardware",
> - .matches = {
> -      DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer,
> Inc."),
> - },
> - },
>   {}
>  };
>  
>  static __init void acpi_osi_dmi_blacklisted(void)
>  {
>   dmi_check_system(acpi_osi_dmi_table);
> +
> + /* Enable _OSI("Darwin") for Apple platforms. */
> + if (x86_apple_machine)
> + acpi_osi_dmi_darwin();
>  }
>  
>  int __init early_acpi_osi_init(void)
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 9eec3095e6c3..6fc204a52493 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -33,6 +33,7 @@
>  #include <linux/acpi.h>
>  #include <linux/slab.h>
>  #include <linux/dmi.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <acpi/apei.h> /* for acpi_hest_init() */
>  
>  #include "internal.h"
> @@ -431,8 +432,7 @@ static void negotiate_os_control(struct
> acpi_pci_root *root, int *no_aspm)
>    * been called successfully. We know the feature set
> supported by the
>    * platform, so avoid calling _OSC at all
>    */
> -
> - if (dmi_match(DMI_SYS_VENDOR, "Apple Inc.")) {
> + if (x86_apple_machine) {
>   root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
>   decode_osc_control(root, "OS assumes control of",
>      root->osc_control_set);
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index ad0b13ad4bbb..a18463799ad7 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -31,7 +31,7 @@
>  #include <linux/jiffies.h>
>  #include <linux/delay.h>
>  #include <linux/power_supply.h>
> -#include <linux/dmi.h>
> +#include <linux/platform_data/x86/apple.h>
>  
>  #include "sbshc.h"
>  #include "battery.h"
> @@ -58,8 +58,6 @@ static unsigned int cache_time = 1000;
>  module_param(cache_time, uint, 0644);
>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
>  
> -static bool sbs_manager_broken;
> -
>  #define MAX_SBS_BAT 4
>  #define ACPI_SBS_BLOCK_MAX 32
>  
> @@ -632,31 +630,12 @@ static void acpi_sbs_callback(void *context)
>   }
>  }
>  
> -static int disable_sbs_manager(const struct dmi_system_id *d)
> -{
> - sbs_manager_broken = true;
> - return 0;
> -}
> -
> -static struct dmi_system_id acpi_sbs_dmi_table[] = {
> - {
> - .callback = disable_sbs_manager,
> - .ident = "Apple",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc.")
> - },
> - },
> - { },
> -};
> -
>  static int acpi_sbs_add(struct acpi_device *device)
>  {
>   struct acpi_sbs *sbs;
>   int result = 0;
>   int id;
>  
> - dmi_check_system(acpi_sbs_dmi_table);
> -
>   sbs = kzalloc(sizeof(struct acpi_sbs), GFP_KERNEL);
>   if (!sbs) {
>   result = -ENOMEM;
> @@ -677,7 +656,7 @@ static int acpi_sbs_add(struct acpi_device
> *device)
>  
>   result = 0;
>  
> - if (!sbs_manager_broken) {
> + if (!x86_apple_machine) {
>   result = acpi_manager_get_info(sbs);
>   if (!result) {
>   sbs->manager_present = 1;
> diff --git a/drivers/firmware/efi/apple-properties.c
> b/drivers/firmware/efi/apple-properties.c
> index c473f4c5ca34..9f6bcf173b0e 100644
> --- a/drivers/firmware/efi/apple-properties.c
> +++ b/drivers/firmware/efi/apple-properties.c
> @@ -18,8 +18,8 @@
>  #define pr_fmt(fmt) "apple-properties: " fmt
>  
>  #include <linux/bootmem.h>
> -#include <linux/dmi.h>
>  #include <linux/efi.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  #include <linux/ucs2_string.h>
> @@ -191,8 +191,7 @@ static int __init map_properties(void)
>   u64 pa_data;
>   int ret;
>  
> - if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc.") &&
> -     !dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc."))
> + if (!x86_apple_machine)
>   return 0;
>  
>   pa_data = boot_params.hdr.setup_data;
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6967c6b4cf6b..ba7b7c64379e 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -25,6 +25,7 @@
>  #include <linux/sched.h>
>  #include <linux/ktime.h>
>  #include <linux/mm.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <asm/dma.h> /* isa_dma_bridge_buggy */
>  #include "pci.h"
>  
> @@ -3447,7 +3448,7 @@ static void
> quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
>  {
>   acpi_handle bridge, SXIO, SXFP, SXLV;
>  
> - if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
> + if (!x86_apple_machine)
>   return;
>   if (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM)
>   return;
> @@ -3492,7 +3493,7 @@ static void
> quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
>   struct pci_dev *sibling = NULL;
>   struct pci_dev *nhi = NULL;
>  
> - if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
> + if (!x86_apple_machine)
>   return;
>   if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
>   return;
> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index bdaac1ff00a5..53250fc057e1 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -13,9 +13,9 @@
>   */
>  
>  #include <linux/delay.h>
> -#include <linux/dmi.h>
>  #include <linux/mutex.h>
>  #include <linux/pci.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> @@ -102,11 +102,6 @@ static inline u64 get_route(u32 route_hi, u32
> route_lo)
>   return (u64)route_hi << 32 | route_lo;
>  }
>  
> -static inline bool is_apple(void)
> -{
> - return dmi_match(DMI_BOARD_VENDOR, "Apple Inc.");
> -}
> -
>  static bool icm_match(const struct tb_cfg_request *req,
>         const struct ctl_pkg *pkg)
>  {
> @@ -176,7 +171,7 @@ static int icm_request(struct tb *tb, const void
> *request, size_t request_size,
>  
>  static bool icm_fr_is_supported(struct tb *tb)
>  {
> - return !is_apple();
> + return !x86_apple_machine;
>  }
>  
>  static inline int icm_fr_get_switch_index(u32 port)
> @@ -517,7 +512,7 @@ static bool icm_ar_is_supported(struct tb *tb)
>    * Starting from Alpine Ridge we can use ICM on Apple
> machines
>    * as well. We just need to reset and re-enable it first.
>    */
> - if (!is_apple())
> + if (!x86_apple_machine)
>   return true;
>  
>   /*
> @@ -1011,7 +1006,7 @@ static int icm_start(struct tb *tb)
>    * don't provide images publicly either. To be on the safe
> side
>    * prevent root switch NVM upgrade on Macs for now.
>    */
> - tb->root_switch->no_nvm_upgrade = is_apple();
> + tb->root_switch->no_nvm_upgrade = x86_apple_machine;
>  
>   ret = tb_switch_add(tb->root_switch);
>   if (ret)
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 1b02ca0b6129..0b22ad9d68b4 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -7,7 +7,7 @@
>  #include <linux/slab.h>
>  #include <linux/errno.h>
>  #include <linux/delay.h>
> -#include <linux/dmi.h>
> +#include <linux/platform_data/x86/apple.h>
>  
>  #include "tb.h"
>  #include "tb_regs.h"
> @@ -453,7 +453,7 @@ struct tb *tb_probe(struct tb_nhi *nhi)
>   struct tb_cm *tcm;
>   struct tb *tb;
>  
> - if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
> + if (!x86_apple_machine)
>   return NULL;
>  
>   tb = tb_domain_alloc(nhi, sizeof(*tcm));
> diff --git a/include/linux/platform_data/x86/apple.h
> b/include/linux/platform_data/x86/apple.h
> new file mode 100644
> index 000000000000..079e816c3c21
> --- /dev/null
> +++ b/include/linux/platform_data/x86/apple.h
> @@ -0,0 +1,13 @@
> +#ifndef PLATFORM_DATA_X86_APPLE_H
> +#define PLATFORM_DATA_X86_APPLE_H
> +
> +#ifdef CONFIG_X86
> +/**
> + * x86_apple_machine - whether the machine is an x86 Apple Macintosh
> + */
> +extern bool x86_apple_machine;
> +#else
> +#define x86_apple_machine false
> +#endif
> +
> +#endif

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2017-08-01 12:42:53

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] treewide: Consolidate Apple DMI checks

On Tue, 2017-08-01 at 15:35 +0300, Andy Shevchenko wrote:
> On Tue, 2017-08-01 at 14:10 +0200, Lukas Wunner wrote:
> > We're about to amend ACPI bus scan with DMI checks whether we're
> > running
> > on a Mac to support Apple device properties in AML.  The DMI checks
> > are
> > performed for every single device, adding overhead for everything
> > x86
> > that isn't Apple, which is the majority.  Rafael and Andy therefore
> > request to perform the DMI match only once and cache the result.
> >
> > Outside of ACPI various other Apple DMI checks exist and it seems
> > reasonable to use the cached value there as well.  Rafael, Andy and
> > Darren suggest performing the DMI check in arch code and making it
> > available with a header in include/linux/platform_data/x86/.
> >
> > To this end, add early_platform_quirks() to arch/x86/kernel/quirks.c
> > to perform the DMI check and invoke it from setup_arch().  Switch
> > over
> > all existing Apple DMI checks, thereby fixing two deficiencies:
> >
> > * They are now #defined to false on non-x86 arches and can thus be
> >   optimized away if they're located in cross-arch code.
> >
> > * Some of them only match "Apple Inc." but not "Apple Computer,
> > Inc.",
> >   which is used by BIOSes released between January 2006 (when the
> > first
> >   x86 Macs started shipping) and January 2007 (when the company name
> >   changed upon introduction of the iPhone).
>
>
> I like the idea, though I can repeat what I commented on your Github
> page.
>
> We might need to distinguish 2006 vs 2007 Apple hardware. Thus, my
> proposal was to use unsigned int (as bitwise flags) instead of bool
> and
> provide two definitions for the hardware. Set those bits accordingly.
> In case of most of the checks it will be the same as in this patch,
> but
> leaves a flexibility of a choice.

Okay, and you answered there.

TWIMC
https://github.com/l1k/linux/commit/20f8b74b83ed45171583fe501182c93e5c6e
b4d9


--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2017-08-01 13:09:01

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] Apple SPI properties

On Tue, Aug 1, 2017 at 2:10 PM, Lukas Wunner <[email protected]> wrote:
> Retrieve device properties on Macs with an Apple-specific _DSM and
> use them in lieu of _CRS data upon SPI slave initialization
> in preparation of supporting the SPI keyboard on 2015+ MacBooks, v4.
>
> Please refer to the cover letter of v1 for further details:
> http://www.spinics.net/lists/linux-acpi/msg75537.html
>
> Changes v3 -> v4:
>
> Patch 1:
> - Extend consolidation of Apple DMI checks to the entire tree
> instead of just the ACPI core. (Rafael, Andy, Darren)
>
> Patches 2 - 5 are as before.

Since the majority of the material here has already been ACKed or
reviewed by the most interested parties and it looks OK to me too, I'm
going to take it as ACPI changes unless anyone in the CC has any
strong opinions against that.

Thanks,
Rafael

2017-08-01 13:11:26

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] treewide: Consolidate Apple DMI checks

On Tue, Aug 01, 2017 at 02:10:41PM +0200, Lukas Wunner wrote:
> We're about to amend ACPI bus scan with DMI checks whether we're running
> on a Mac to support Apple device properties in AML. The DMI checks are
> performed for every single device, adding overhead for everything x86
> that isn't Apple, which is the majority. Rafael and Andy therefore
> request to perform the DMI match only once and cache the result.
>
> Outside of ACPI various other Apple DMI checks exist and it seems
> reasonable to use the cached value there as well. Rafael, Andy and
> Darren suggest performing the DMI check in arch code and making it
> available with a header in include/linux/platform_data/x86/.
>
> To this end, add early_platform_quirks() to arch/x86/kernel/quirks.c
> to perform the DMI check and invoke it from setup_arch(). Switch over
> all existing Apple DMI checks, thereby fixing two deficiencies:
>
> * They are now #defined to false on non-x86 arches and can thus be
> optimized away if they're located in cross-arch code.
>
> * Some of them only match "Apple Inc." but not "Apple Computer, Inc.",
> which is used by BIOSes released between January 2006 (when the first
> x86 Macs started shipping) and January 2007 (when the company name
> changed upon introduction of the iPhone).
>
> Cc: Lv Zheng <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Matt Fleming <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Andreas Noever <[email protected]>
> Cc: Michael Jamet <[email protected]>
> Cc: Yehezkel Bernat <[email protected]>
> Cc: Mika Westerberg <[email protected]>

For the thunderbolt bits:

Acked-by: Mika Westerberg <[email protected]>

2017-08-01 13:15:24

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] ACPI / scan: Recognize Apple SPI and I2C slaves

On Tue, Aug 01, 2017 at 02:10:41PM +0200, Lukas Wunner wrote:
> SPI and I2C slaves are enumerated by their respective parents rather
> than the ACPI core. They are recognized by presence of _CRS resources,
> which however are missing on Macs. Check for presence of device
> properties instead.
>
> Cc: Federico Lorenzi <[email protected]>
> Cc: Mika Westerberg <[email protected]>

Acked-by: Mika Westerberg <[email protected]>

2017-08-09 22:50:57

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] Apple SPI properties

On Tuesday, August 1, 2017 3:08:58 PM CEST Rafael J. Wysocki wrote:
> On Tue, Aug 1, 2017 at 2:10 PM, Lukas Wunner <[email protected]> wrote:
> > Retrieve device properties on Macs with an Apple-specific _DSM and
> > use them in lieu of _CRS data upon SPI slave initialization
> > in preparation of supporting the SPI keyboard on 2015+ MacBooks, v4.
> >
> > Please refer to the cover letter of v1 for further details:
> > http://www.spinics.net/lists/linux-acpi/msg75537.html
> >
> > Changes v3 -> v4:
> >
> > Patch 1:
> > - Extend consolidation of Apple DMI checks to the entire tree
> > instead of just the ACPI core. (Rafael, Andy, Darren)
> >
> > Patches 2 - 5 are as before.
>
> Since the majority of the material here has already been ACKed or
> reviewed by the most interested parties and it looks OK to me too, I'm
> going to take it as ACPI changes unless anyone in the CC has any
> strong opinions against that.

No objections I know of, so applied now.

Thanks,
Rafael