2020-01-22 11:24:44

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 0/9] x86: Easy way of detecting MS Surface 3

While working on RTC regression, I noticed that we are using the same DMI check
over and over in the drivers for MS Surface 3 platform. This series dedicated
for making it easier in the same way how it's done for Apple machines.

Changelog v3:
- fixed typo in patch 5 (Jonathan)
- returned back to if {} else {} condition in ASoC driver (Mark)
- added Mark's Ack tag

Changelog v2:
- removed RTC patches for now (the fix will be independent to this series)
- added couple more clean ups to arch/x86/kernel/quirks.c
- redone DMI quirk to use driver_data instead of callback
- simplified check in soc-acpi-intel-cht-match.c to be oneliner
- added a new patch to cover rt5645 codec driver

Cc: Cezary Rojewski <[email protected]>
Cc: Pierre-Louis Bossart <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Jie Yang <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: [email protected]

Andy Shevchenko (9):
x86/platform: Rename x86/apple.h -> x86/machine.h
x86/quirks: Add missed include to satisfy static checker
x86/quirks: Introduce hpet_dev_print_force_hpet_address() helper
x86/quirks: Join string literals back
x86/quirks: Convert DMI matching to use a table
x86/quirks: Add a DMI quirk for Microsoft Surface 3
platform/x86: surface3_wmi: Switch DMI table match to a test of
variable
ASoC: rt5645: Switch DMI table match to a test of variable
ASoC: Intel: Switch DMI table match to a test of variable

arch/x86/kernel/quirks.c | 91 +++++++++++++------
drivers/platform/x86/surface3-wmi.c | 16 +---
include/linux/platform_data/x86/apple.h | 14 +--
include/linux/platform_data/x86/machine.h | 20 ++++
sound/soc/codecs/rt5645.c | 14 ++-
.../intel/common/soc-acpi-intel-cht-match.c | 28 +-----
6 files changed, 93 insertions(+), 90 deletions(-)
create mode 100644 include/linux/platform_data/x86/machine.h

--
2.24.1


2020-01-22 11:24:44

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 9/9] ASoC: Intel: Switch DMI table match to a test of variable

Since we have a common x86 quirk that provides an exported variable,
use it instead of local DMI table match.

Cc: Cezary Rojewski <[email protected]>
Cc: Pierre-Louis Bossart <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Jie Yang <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: [email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
Acked-by: Mark Brown <[email protected]>
---
.../intel/common/soc-acpi-intel-cht-match.c | 28 ++-----------------
1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
index d0fb43c2b9f6..833d2e130e6e 100644
--- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
@@ -5,31 +5,11 @@
* Copyright (c) 2017, Intel Corporation.
*/

-#include <linux/dmi.h>
+#include <linux/platform_data/x86/machine.h>
+
#include <sound/soc-acpi.h>
#include <sound/soc-acpi-intel-match.h>

-static unsigned long cht_machine_id;
-
-#define CHT_SURFACE_MACH 1
-
-static int cht_surface_quirk_cb(const struct dmi_system_id *id)
-{
- cht_machine_id = CHT_SURFACE_MACH;
- return 1;
-}
-
-static const struct dmi_system_id cht_table[] = {
- {
- .callback = cht_surface_quirk_cb,
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
- },
- },
- { }
-};
-
static struct snd_soc_acpi_mach cht_surface_mach = {
.id = "10EC5640",
.drv_name = "cht-bsw-rt5645",
@@ -43,9 +23,7 @@ static struct snd_soc_acpi_mach *cht_quirk(void *arg)
{
struct snd_soc_acpi_mach *mach = arg;

- dmi_check_system(cht_table);
-
- if (cht_machine_id == CHT_SURFACE_MACH)
+ if (x86_microsoft_surface_3_machine)
return &cht_surface_mach;
else
return mach;
--
2.24.1

2020-01-22 11:24:46

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 5/9] x86/quirks: Convert DMI matching to use a table

In order to extend the DMI based quirks, convert them to a table.

Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 6c122f35508a..3867f81baae7 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -658,8 +658,37 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2083, quirk_intel_purley_xeon_ras
bool x86_apple_machine;
EXPORT_SYMBOL(x86_apple_machine);

+static const struct dmi_system_id x86_machine_table[] __initconst = {
+ {
+ .ident = "x86 Apple Macintosh",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ },
+ .driver_data = &x86_apple_machine,
+ },
+ {
+ .ident = "x86 Apple Macintosh",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, Inc."),
+ },
+ .driver_data = &x86_apple_machine,
+ },
+ {}
+};
+
+static void __init early_platform_detect_quirk(void)
+{
+ const struct dmi_system_id *id;
+
+ id = dmi_first_match(x86_machine_table);
+ if (!id)
+ return;
+
+ printk(KERN_DEBUG "Detected %s platform\n", id->ident);
+ *((bool *)id->driver_data) = true;
+}
+
void __init early_platform_quirks(void)
{
- x86_apple_machine = dmi_match(DMI_SYS_VENDOR, "Apple Inc.") ||
- dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc.");
+ early_platform_detect_quirk();
}
--
2.24.1

2020-01-22 11:24:52

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 7/9] platform/x86: surface3_wmi: Switch DMI table match to a test of variable

Since we have a common x86 quirk that provides an exported variable,
use it instead of local DMI table match.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/platform/x86/surface3-wmi.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/surface3-wmi.c b/drivers/platform/x86/surface3-wmi.c
index 130b6f52a600..5eeedc4ddb8a 100644
--- a/drivers/platform/x86/surface3-wmi.c
+++ b/drivers/platform/x86/surface3-wmi.c
@@ -11,9 +11,9 @@
#include <linux/slab.h>

#include <linux/acpi.h>
-#include <linux/dmi.h>
#include <linux/input.h>
#include <linux/mutex.h>
+#include <linux/platform_data/x86/machine.h>
#include <linux/platform_device.h>
#include <linux/spi/spi.h>

@@ -29,18 +29,6 @@ MODULE_LICENSE("GPL");

MODULE_ALIAS("wmi:" SURFACE3_LID_GUID);

-static const struct dmi_system_id surface3_dmi_table[] = {
-#if defined(CONFIG_X86)
- {
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
- },
- },
-#endif
- { }
-};
-
struct surface3_wmi {
struct acpi_device *touchscreen_adev;
struct acpi_device *pnp0c0d_adev;
@@ -201,7 +189,7 @@ static int __init s3_wmi_probe(struct platform_device *pdev)
{
int error;

- if (!dmi_check_system(surface3_dmi_table))
+ if (!x86_microsoft_surface_3_machine)
return -ENODEV;

memset(&s3_wmi, 0, sizeof(s3_wmi));
--
2.24.1

2020-01-22 11:25:49

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 3/9] x86/quirks: Introduce hpet_dev_print_force_hpet_address() helper

Introduce hpet_dev_print_force_hpet_address() helper to unify printing
forced HPET address. No functional change intended.

Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 5b96654aacc0..ce8fc9bec43b 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -68,6 +68,11 @@ static enum {
ATI_FORCE_HPET_RESUME,
} force_hpet_resume_type;

+static void hpet_dev_print_force_hpet_address(struct device *dev)
+{
+ dev_printk(KERN_DEBUG, dev, "Force enabled HPET at 0x%lx\n", force_hpet_address);
+}
+
static void __iomem *rcba_base;

static void ich_force_hpet_resume(void)
@@ -125,8 +130,7 @@ static void ich_force_enable_hpet(struct pci_dev *dev)
/* HPET is enabled in HPTC. Just not reported by BIOS */
val = val & 0x3;
force_hpet_address = 0xFED00000 | (val << 12);
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
- "0x%lx\n", force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
iounmap(rcba_base);
return;
}
@@ -149,8 +153,7 @@ static void ich_force_enable_hpet(struct pci_dev *dev)
"Failed to force enable HPET\n");
} else {
force_hpet_resume_type = ICH_FORCE_HPET_RESUME;
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
- "0x%lx\n", force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
}
}

@@ -223,8 +226,7 @@ static void old_ich_force_enable_hpet(struct pci_dev *dev)
if (val & 0x4) {
val &= 0x3;
force_hpet_address = 0xFED00000 | (val << 12);
- dev_printk(KERN_DEBUG, &dev->dev, "HPET at 0x%lx\n",
- force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
return;
}

@@ -244,8 +246,7 @@ static void old_ich_force_enable_hpet(struct pci_dev *dev)
/* HPET is enabled in HPTC. Just not reported by BIOS */
val &= 0x3;
force_hpet_address = 0xFED00000 | (val << 12);
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
- "0x%lx\n", force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
cached_dev = dev;
force_hpet_resume_type = OLD_ICH_FORCE_HPET_RESUME;
return;
@@ -316,8 +317,7 @@ static void vt8237_force_enable_hpet(struct pci_dev *dev)
*/
if (val & 0x80) {
force_hpet_address = (val & ~0x3ff);
- dev_printk(KERN_DEBUG, &dev->dev, "HPET at 0x%lx\n",
- force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
return;
}

@@ -331,8 +331,7 @@ static void vt8237_force_enable_hpet(struct pci_dev *dev)
pci_read_config_dword(dev, 0x68, &val);
if (val & 0x80) {
force_hpet_address = (val & ~0x3ff);
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
- "0x%lx\n", force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
cached_dev = dev;
force_hpet_resume_type = VT8237_FORCE_HPET_RESUME;
return;
@@ -412,8 +411,7 @@ static void ati_force_enable_hpet(struct pci_dev *dev)

force_hpet_address = val;
force_hpet_resume_type = ATI_FORCE_HPET_RESUME;
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at 0x%lx\n",
- force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
cached_dev = dev;
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP400_SMBUS,
@@ -444,8 +442,7 @@ static void nvidia_force_enable_hpet(struct pci_dev *dev)
pci_read_config_dword(dev, 0x44, &val);
force_hpet_address = val & 0xfffffffe;
force_hpet_resume_type = NVIDIA_FORCE_HPET_RESUME;
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at 0x%lx\n",
- force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
cached_dev = dev;
}

@@ -509,8 +506,7 @@ static void e6xx_force_enable_hpet(struct pci_dev *dev)

force_hpet_address = 0xFED00000;
force_hpet_resume_type = NONE_FORCE_HPET_RESUME;
- dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
- "0x%lx\n", force_hpet_address);
+ hpet_dev_print_force_hpet_address(&dev->dev);
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E6XX_CU,
e6xx_force_enable_hpet);
--
2.24.1

2020-01-22 11:25:57

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 2/9] x86/quirks: Add missed include to satisfy static checker

Static checker is not happy with

.../kernel/quirks.c:666:6: warning: symbol 'x86_apple_machine' was not declared. Should it be static?

This is due to missed inclusion. Add it to satisfy the static checker.

Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 1daf8f2aa21f..5b96654aacc0 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -4,6 +4,7 @@
*/
#include <linux/dmi.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/machine.h>
#include <linux/irq.h>

#include <asm/hpet.h>
--
2.24.1

2020-01-22 11:25:57

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 4/9] x86/quirks: Join string literals back

There is no need to split string literals. Moreover, it would be simpler
to grep for an actual code line, when debugging, by using almost any
part of the string literal in question.

No functional change intended.

Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index ce8fc9bec43b..6c122f35508a 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -36,8 +36,7 @@ static void quirk_intel_irqbalance(struct pci_dev *dev)
pci_bus_read_config_word(dev->bus, PCI_DEVFN(8, 0), 0x4c, &word);

if (!(word & (1 << 13))) {
- dev_info(&dev->dev, "Intel E7520/7320/7525 detected; "
- "disabling irq balancing and affinity\n");
+ dev_info(&dev->dev, "Intel E7520/7320/7525 detected; disabling irq balancing and affinity\n");
noirqdebug_setup("");
#ifdef CONFIG_PROC_FS
no_irq_affinity = 1;
@@ -110,16 +109,14 @@ static void ich_force_enable_hpet(struct pci_dev *dev)
pci_read_config_dword(dev, 0xF0, &rcba);
rcba &= 0xFFFFC000;
if (rcba == 0) {
- dev_printk(KERN_DEBUG, &dev->dev, "RCBA disabled; "
- "cannot force enable HPET\n");
+ dev_printk(KERN_DEBUG, &dev->dev, "RCBA disabled; cannot force enable HPET\n");
return;
}

/* use bits 31:14, 16 kB aligned */
rcba_base = ioremap_nocache(rcba, 0x4000);
if (rcba_base == NULL) {
- dev_printk(KERN_DEBUG, &dev->dev, "ioremap failed; "
- "cannot force enable HPET\n");
+ dev_printk(KERN_DEBUG, &dev->dev, "ioremap failed; cannot force enable HPET\n");
return;
}

@@ -149,8 +146,7 @@ static void ich_force_enable_hpet(struct pci_dev *dev)
if (err) {
force_hpet_address = 0;
iounmap(rcba_base);
- dev_printk(KERN_DEBUG, &dev->dev,
- "Failed to force enable HPET\n");
+ dev_printk(KERN_DEBUG, &dev->dev, "Failed to force enable HPET\n");
} else {
force_hpet_resume_type = ICH_FORCE_HPET_RESUME;
hpet_dev_print_force_hpet_address(&dev->dev);
@@ -182,8 +178,7 @@ static struct pci_dev *cached_dev;

static void hpet_print_force_info(void)
{
- printk(KERN_INFO "HPET not enabled in BIOS. "
- "You might try hpet=force boot option\n");
+ printk(KERN_INFO "HPET not enabled in BIOS. You might try hpet=force boot option\n");
}

static void old_ich_force_hpet_resume(void)
--
2.24.1

2020-01-22 11:26:28

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 1/9] x86/platform: Rename x86/apple.h -> x86/machine.h

Rename linux/platform_data/x86/apple.h to linux/platform_data/x86/machine.h
in order to add new quirks later on. For sake of being less intrusive,
leave former file that includes a latter one.

While here, add include to linux/types.h due to bool type in use.

Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/platform_data/x86/apple.h | 14 +-------------
include/linux/platform_data/x86/machine.h | 15 +++++++++++++++
2 files changed, 16 insertions(+), 13 deletions(-)
create mode 100644 include/linux/platform_data/x86/machine.h

diff --git a/include/linux/platform_data/x86/apple.h b/include/linux/platform_data/x86/apple.h
index 079e816c3c21..1fd0af6ffea9 100644
--- a/include/linux/platform_data/x86/apple.h
+++ b/include/linux/platform_data/x86/apple.h
@@ -1,13 +1 @@
-#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
+#include <linux/platform_data/x86/machine.h>
diff --git a/include/linux/platform_data/x86/machine.h b/include/linux/platform_data/x86/machine.h
new file mode 100644
index 000000000000..b1e7a560a046
--- /dev/null
+++ b/include/linux/platform_data/x86/machine.h
@@ -0,0 +1,15 @@
+#ifndef PLATFORM_DATA_X86_MACHINE_H
+#define PLATFORM_DATA_X86_MACHINE_H
+
+#include <linux/types.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 /* PLATFORM_DATA_X86_MACHINE_H */
--
2.24.1

2020-02-14 14:26:21

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] x86: Easy way of detecting MS Surface 3

On Wed, Jan 22, 2020 at 01:22:57PM +0200, Andy Shevchenko wrote:
> While working on RTC regression, I noticed that we are using the same DMI check
> over and over in the drivers for MS Surface 3 platform. This series dedicated
> for making it easier in the same way how it's done for Apple machines.


Any comments on this?

> Changelog v3:
> - fixed typo in patch 5 (Jonathan)
> - returned back to if {} else {} condition in ASoC driver (Mark)
> - added Mark's Ack tag
>
> Changelog v2:
> - removed RTC patches for now (the fix will be independent to this series)
> - added couple more clean ups to arch/x86/kernel/quirks.c
> - redone DMI quirk to use driver_data instead of callback
> - simplified check in soc-acpi-intel-cht-match.c to be oneliner
> - added a new patch to cover rt5645 codec driver
>
> Cc: Cezary Rojewski <[email protected]>
> Cc: Pierre-Louis Bossart <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> Cc: Jie Yang <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: [email protected]
>
> Andy Shevchenko (9):
> x86/platform: Rename x86/apple.h -> x86/machine.h
> x86/quirks: Add missed include to satisfy static checker
> x86/quirks: Introduce hpet_dev_print_force_hpet_address() helper
> x86/quirks: Join string literals back
> x86/quirks: Convert DMI matching to use a table
> x86/quirks: Add a DMI quirk for Microsoft Surface 3
> platform/x86: surface3_wmi: Switch DMI table match to a test of
> variable
> ASoC: rt5645: Switch DMI table match to a test of variable
> ASoC: Intel: Switch DMI table match to a test of variable
>
> arch/x86/kernel/quirks.c | 91 +++++++++++++------
> drivers/platform/x86/surface3-wmi.c | 16 +---
> include/linux/platform_data/x86/apple.h | 14 +--
> include/linux/platform_data/x86/machine.h | 20 ++++
> sound/soc/codecs/rt5645.c | 14 ++-
> .../intel/common/soc-acpi-intel-cht-match.c | 28 +-----
> 6 files changed, 93 insertions(+), 90 deletions(-)
> create mode 100644 include/linux/platform_data/x86/machine.h
>
> --
> 2.24.1
>

--
With Best Regards,
Andy Shevchenko


2020-03-16 15:29:48

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] x86/quirks: Convert DMI matching to use a table

Andy Shevchenko <[email protected]> writes:

> +static const struct dmi_system_id x86_machine_table[] __initconst = {
> + {
> + .ident = "x86 Apple Macintosh",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + },
> + .driver_data = &x86_apple_machine,
> + },
> + {
> + .ident = "x86 Apple Macintosh",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, Inc."),
> + },
> + .driver_data = &x86_apple_machine,
> + },
> + {}
> +};
> +
> +static void __init early_platform_detect_quirk(void)
> +{
> + const struct dmi_system_id *id;
> +
> + id = dmi_first_match(x86_machine_table);
> + if (!id)
> + return;
> +
> + printk(KERN_DEBUG "Detected %s platform\n", id->ident);
> + *((bool *)id->driver_data) = true;

I'd suggest that x86_apple_machine and the ones that you add further
down this patchset, be made functions instead. That way you could at
first hide the global bool(s) and then replace this with something a
little more type safe.

Regards,
--
Alex

2020-03-16 15:33:43

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] x86: Easy way of detecting MS Surface 3

Andy Shevchenko <[email protected]> writes:

> While working on RTC regression, I noticed that we are using the same DMI check
> over and over in the drivers for MS Surface 3 platform. This series dedicated
> for making it easier in the same way how it's done for Apple machines.

[...]

> x86/quirks: Introduce hpet_dev_print_force_hpet_address() helper
> x86/quirks: Join string literals back

These two don't seem to be related to the Surface 3 cause of the rest of
the patchset, or am I missing something?

Regards,
--
Alex

2020-03-16 15:53:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] x86: Easy way of detecting MS Surface 3

On Mon, Mar 16, 2020 at 05:33:02PM +0200, Alexander Shishkin wrote:
> Andy Shevchenko <[email protected]> writes:
>
> > While working on RTC regression, I noticed that we are using the same DMI check
> > over and over in the drivers for MS Surface 3 platform. This series dedicated
> > for making it easier in the same way how it's done for Apple machines.
>
> [...]
>
> > x86/quirks: Introduce hpet_dev_print_force_hpet_address() helper
> > x86/quirks: Join string literals back
>
> These two don't seem to be related to the Surface 3 cause of the rest of
> the patchset, or am I missing something?

No, they are not. I think it's suitable to have them in the bunch nevertheless.

--
With Best Regards,
Andy Shevchenko


2020-03-16 15:54:50

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] x86/quirks: Convert DMI matching to use a table

On Mon, Mar 16, 2020 at 05:27:30PM +0200, Alexander Shishkin wrote:
> Andy Shevchenko <[email protected]> writes:
>
> > +static const struct dmi_system_id x86_machine_table[] __initconst = {
> > + {
> > + .ident = "x86 Apple Macintosh",
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> > + },
> > + .driver_data = &x86_apple_machine,
> > + },
> > + {
> > + .ident = "x86 Apple Macintosh",
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, Inc."),
> > + },
> > + .driver_data = &x86_apple_machine,
> > + },
> > + {}
> > +};
> > +
> > +static void __init early_platform_detect_quirk(void)
> > +{
> > + const struct dmi_system_id *id;
> > +
> > + id = dmi_first_match(x86_machine_table);
> > + if (!id)
> > + return;
> > +
> > + printk(KERN_DEBUG "Detected %s platform\n", id->ident);
> > + *((bool *)id->driver_data) = true;
>
> I'd suggest that x86_apple_machine and the ones that you add further
> down this patchset, be made functions instead. That way you could at
> first hide the global bool(s) and then replace this with something a
> little more type safe.

I'm not sure we will get any benefit of the proposed change. Also it will be
more intrusive since we have dozen of modules that are using it in the form of
exported boolean.

--
With Best Regards,
Andy Shevchenko