[PATCH 0/3] ASoC/pdx86/input: Introduce and use soc_intel_is_*() helpers
Hi All,
We have been open-coding x86_match_cpu() checks for enabling some SoC specific behavior in various places.
The sound/soc/intel drivers used to also open-code this but this was cleaned up a while ago introducing a number of soc_intel_is_*() helpers.
This series moves the definition of these helpers to a more public place and uses it in a couple of more places outside the sound tree.
Mark, I know we are a bit late in the cycle, but if you can pick up patch 1/3 (assuming on one objects) for 5.16, then the rest can be applied after 5.16-rc1 is out.
Regards,
Hans
Hans de Goede (3): ASoC: Intel: Move soc_intel_is_foo() helpers to a generic header platform/x86: intel_int0002_vgpio: Use the new soc_intel_is_byt/cht helpers Input: axp20x-pek - Use new soc_intel_is_cht() helper
drivers/input/misc/axp20x-pek.c | 26 ++------- drivers/platform/x86/intel/int0002_vgpio.c | 14 +---- include/linux/platform_data/x86/soc.h | 65 ++++++++++++++++++++++ sound/soc/intel/common/soc-intel-quirks.h | 51 +---------------- 4 files changed, 75 insertions(+), 81 deletions(-) create mode 100644 include/linux/platform_data/x86/soc.h
The soc_intel_is_foo() helpers from sound/soc/intel/common/soc-intel-quirks.h are useful outside of the sound subsystem too.
Move these to include/linux/platform_data/x86/soc.h, so that other code can use them too.
Suggested-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Hans de Goede hdegoede@redhat.com --- include/linux/platform_data/x86/soc.h | 65 +++++++++++++++++++++++ sound/soc/intel/common/soc-intel-quirks.h | 51 ++---------------- 2 files changed, 68 insertions(+), 48 deletions(-) create mode 100644 include/linux/platform_data/x86/soc.h
diff --git a/include/linux/platform_data/x86/soc.h b/include/linux/platform_data/x86/soc.h new file mode 100644 index 000000000000..8d710834e87a --- /dev/null +++ b/include/linux/platform_data/x86/soc.h @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * soc.h - helpers for Intel SoC model detection + * + * Copyright (c) 2019, Intel Corporation. + */ + +#ifndef __PLATFORM_DATA_X86_SOC_H +#define __PLATFORM_DATA_X86_SOC_H + +#if IS_ENABLED(CONFIG_X86) + +#include <asm/cpu_device_id.h> +#include <asm/intel-family.h> + +#define SOC_INTEL_IS_CPU(soc, type) \ +static inline bool soc_intel_is_##soc(void) \ +{ \ + static const struct x86_cpu_id soc##_cpu_ids[] = { \ + X86_MATCH_INTEL_FAM6_MODEL(type, NULL), \ + {} \ + }; \ + const struct x86_cpu_id *id; \ + \ + id = x86_match_cpu(soc##_cpu_ids); \ + if (id) \ + return true; \ + return false; \ +} + +SOC_INTEL_IS_CPU(byt, ATOM_SILVERMONT); +SOC_INTEL_IS_CPU(cht, ATOM_AIRMONT); +SOC_INTEL_IS_CPU(apl, ATOM_GOLDMONT); +SOC_INTEL_IS_CPU(glk, ATOM_GOLDMONT_PLUS); +SOC_INTEL_IS_CPU(cml, KABYLAKE_L); + +#else /* IS_ENABLED(CONFIG_X86) */ + +static inline bool soc_intel_is_byt(void) +{ + return false; +} + +static inline bool soc_intel_is_cht(void) +{ + return false; +} + +static inline bool soc_intel_is_apl(void) +{ + return false; +} + +static inline bool soc_intel_is_glk(void) +{ + return false; +} + +static inline bool soc_intel_is_cml(void) +{ + return false; +} +#endif /* IS_ENABLED(CONFIG_X86) */ + +#endif /* __PLATFORM_DATA_X86_SOC_H */ diff --git a/sound/soc/intel/common/soc-intel-quirks.h b/sound/soc/intel/common/soc-intel-quirks.h index a93987ab7f4d..de4e550c5b34 100644 --- a/sound/soc/intel/common/soc-intel-quirks.h +++ b/sound/soc/intel/common/soc-intel-quirks.h @@ -9,34 +9,13 @@ #ifndef _SND_SOC_INTEL_QUIRKS_H #define _SND_SOC_INTEL_QUIRKS_H
+#include <linux/platform_data/x86/soc.h> + #if IS_ENABLED(CONFIG_X86)
#include <linux/dmi.h> -#include <asm/cpu_device_id.h> -#include <asm/intel-family.h> #include <asm/iosf_mbi.h>
-#define SOC_INTEL_IS_CPU(soc, type) \ -static inline bool soc_intel_is_##soc(void) \ -{ \ - static const struct x86_cpu_id soc##_cpu_ids[] = { \ - X86_MATCH_INTEL_FAM6_MODEL(type, NULL), \ - {} \ - }; \ - const struct x86_cpu_id *id; \ - \ - id = x86_match_cpu(soc##_cpu_ids); \ - if (id) \ - return true; \ - return false; \ -} - -SOC_INTEL_IS_CPU(byt, ATOM_SILVERMONT); -SOC_INTEL_IS_CPU(cht, ATOM_AIRMONT); -SOC_INTEL_IS_CPU(apl, ATOM_GOLDMONT); -SOC_INTEL_IS_CPU(glk, ATOM_GOLDMONT_PLUS); -SOC_INTEL_IS_CPU(cml, KABYLAKE_L); - static inline bool soc_intel_is_byt_cr(struct platform_device *pdev) { /* @@ -114,30 +93,6 @@ static inline bool soc_intel_is_byt_cr(struct platform_device *pdev) return false; }
-static inline bool soc_intel_is_byt(void) -{ - return false; -} - -static inline bool soc_intel_is_cht(void) -{ - return false; -} - -static inline bool soc_intel_is_apl(void) -{ - return false; -} - -static inline bool soc_intel_is_glk(void) -{ - return false; -} - -static inline bool soc_intel_is_cml(void) -{ - return false; -} #endif
- #endif /* _SND_SOC_INTEL_QUIRKS_H */ +#endif /* _SND_SOC_INTEL_QUIRKS_H */
On Mon, Oct 18, 2021 at 5:33 PM Hans de Goede hdegoede@redhat.com wrote:
The soc_intel_is_foo() helpers from sound/soc/intel/common/soc-intel-quirks.h are useful outside of the sound subsystem too.
Move these to include/linux/platform_data/x86/soc.h, so that other code can use them too.
A nit-pick below which may be ignored.
Suggested-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Hans de Goede hdegoede@redhat.com
include/linux/platform_data/x86/soc.h | 65 +++++++++++++++++++++++ sound/soc/intel/common/soc-intel-quirks.h | 51 ++---------------- 2 files changed, 68 insertions(+), 48 deletions(-) create mode 100644 include/linux/platform_data/x86/soc.h
diff --git a/include/linux/platform_data/x86/soc.h b/include/linux/platform_data/x86/soc.h new file mode 100644 index 000000000000..8d710834e87a --- /dev/null +++ b/include/linux/platform_data/x86/soc.h @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/*
- soc.h - helpers for Intel SoC model detection
No need of having the filename in the file itself, it even might add a churn in the future in case of renaming.
- Copyright (c) 2019, Intel Corporation.
- */
+#ifndef __PLATFORM_DATA_X86_SOC_H +#define __PLATFORM_DATA_X86_SOC_H
+#if IS_ENABLED(CONFIG_X86)
+#include <asm/cpu_device_id.h> +#include <asm/intel-family.h>
+#define SOC_INTEL_IS_CPU(soc, type) \ +static inline bool soc_intel_is_##soc(void) \ +{ \
static const struct x86_cpu_id soc##_cpu_ids[] = { \X86_MATCH_INTEL_FAM6_MODEL(type, NULL), \{} \}; \const struct x86_cpu_id *id; \\id = x86_match_cpu(soc##_cpu_ids); \if (id) \return true; \return false; \+}
+SOC_INTEL_IS_CPU(byt, ATOM_SILVERMONT); +SOC_INTEL_IS_CPU(cht, ATOM_AIRMONT); +SOC_INTEL_IS_CPU(apl, ATOM_GOLDMONT); +SOC_INTEL_IS_CPU(glk, ATOM_GOLDMONT_PLUS); +SOC_INTEL_IS_CPU(cml, KABYLAKE_L);
+#else /* IS_ENABLED(CONFIG_X86) */
+static inline bool soc_intel_is_byt(void) +{
return false;+}
+static inline bool soc_intel_is_cht(void) +{
return false;+}
+static inline bool soc_intel_is_apl(void) +{
return false;+}
+static inline bool soc_intel_is_glk(void) +{
return false;+}
+static inline bool soc_intel_is_cml(void) +{
return false;+} +#endif /* IS_ENABLED(CONFIG_X86) */
+#endif /* __PLATFORM_DATA_X86_SOC_H */ diff --git a/sound/soc/intel/common/soc-intel-quirks.h b/sound/soc/intel/common/soc-intel-quirks.h index a93987ab7f4d..de4e550c5b34 100644 --- a/sound/soc/intel/common/soc-intel-quirks.h +++ b/sound/soc/intel/common/soc-intel-quirks.h @@ -9,34 +9,13 @@ #ifndef _SND_SOC_INTEL_QUIRKS_H #define _SND_SOC_INTEL_QUIRKS_H
+#include <linux/platform_data/x86/soc.h>
#if IS_ENABLED(CONFIG_X86)
#include <linux/dmi.h> -#include <asm/cpu_device_id.h> -#include <asm/intel-family.h> #include <asm/iosf_mbi.h>
-#define SOC_INTEL_IS_CPU(soc, type) \ -static inline bool soc_intel_is_##soc(void) \ -{ \
static const struct x86_cpu_id soc##_cpu_ids[] = { \X86_MATCH_INTEL_FAM6_MODEL(type, NULL), \{} \}; \const struct x86_cpu_id *id; \\id = x86_match_cpu(soc##_cpu_ids); \if (id) \return true; \return false; \-}
-SOC_INTEL_IS_CPU(byt, ATOM_SILVERMONT); -SOC_INTEL_IS_CPU(cht, ATOM_AIRMONT); -SOC_INTEL_IS_CPU(apl, ATOM_GOLDMONT); -SOC_INTEL_IS_CPU(glk, ATOM_GOLDMONT_PLUS); -SOC_INTEL_IS_CPU(cml, KABYLAKE_L);
static inline bool soc_intel_is_byt_cr(struct platform_device *pdev) { /* @@ -114,30 +93,6 @@ static inline bool soc_intel_is_byt_cr(struct platform_device *pdev) return false; }
-static inline bool soc_intel_is_byt(void) -{
return false;-}
-static inline bool soc_intel_is_cht(void) -{
return false;-}
-static inline bool soc_intel_is_apl(void) -{
return false;-}
-static inline bool soc_intel_is_glk(void) -{
return false;-}
-static inline bool soc_intel_is_cml(void) -{
return false;-} #endif
- #endif /* _SND_SOC_INTEL_QUIRKS_H */
+#endif /* _SND_SOC_INTEL_QUIRKS_H */
2.31.1
On Mon, Oct 18, 2021 at 04:33:22PM +0200, Hans de Goede wrote:
The soc_intel_is_foo() helpers from sound/soc/intel/common/soc-intel-quirks.h are useful outside of the sound subsystem too.
Acked-by: Mark Brown broonie@kernel.org
Hi Mark,
On 10/18/21 17:35, Mark Brown wrote:
On Mon, Oct 18, 2021 at 04:33:22PM +0200, Hans de Goede wrote:
The soc_intel_is_foo() helpers from sound/soc/intel/common/soc-intel-quirks.h are useful outside of the sound subsystem too.
Acked-by: Mark Brown broonie@kernel.org
Does this mean that you are ok with me merging patch 1 + 2 through the drivers/platform/x86 tree ?
Regards,
Hans
Use the new soc_intel_is_byt/cht helpers to clean things up a bit.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/intel/int0002_vgpio.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/int0002_vgpio.c b/drivers/platform/x86/intel/int0002_vgpio.c index 569342aa8926..617dbf98980e 100644 --- a/drivers/platform/x86/intel/int0002_vgpio.c +++ b/drivers/platform/x86/intel/int0002_vgpio.c @@ -34,13 +34,11 @@ #include <linux/io.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/platform_data/x86/soc.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/suspend.h>
-#include <asm/cpu_device_id.h> -#include <asm/intel-family.h> - #define DRV_NAME "INT0002 Virtual GPIO"
/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */ @@ -151,12 +149,6 @@ static struct irq_chip int0002_irqchip = { .irq_set_wake = int0002_irq_set_wake, };
-static const struct x86_cpu_id int0002_cpu_ids[] = { - X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT, NULL), - X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT, NULL), - {} -}; - static void int0002_init_irq_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask, unsigned int ngpios) @@ -167,15 +159,13 @@ static void int0002_init_irq_valid_mask(struct gpio_chip *chip, static int int0002_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; - const struct x86_cpu_id *cpu_id; struct int0002_data *int0002; struct gpio_irq_chip *girq; struct gpio_chip *chip; int irq, ret;
/* Menlow has a different INT0002 device? <sigh> */ - cpu_id = x86_match_cpu(int0002_cpu_ids); - if (!cpu_id) + if (!soc_intel_is_byt() && !soc_intel_is_cht()) return -ENODEV;
irq = platform_get_irq(pdev, 0);
On Mon, Oct 18, 2021 at 5:33 PM Hans de Goede hdegoede@redhat.com wrote:
Couple of nit-picks below (may be ignored).
Use the new soc_intel_is_byt/cht helpers to clean things up a bit.
soc_intel_is_byt()/soc_intel_is_cht() (or anything alike to show that these are functions / macros).
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/intel/int0002_vgpio.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/int0002_vgpio.c b/drivers/platform/x86/intel/int0002_vgpio.c index 569342aa8926..617dbf98980e 100644 --- a/drivers/platform/x86/intel/int0002_vgpio.c +++ b/drivers/platform/x86/intel/int0002_vgpio.c @@ -34,13 +34,11 @@ #include <linux/io.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/platform_data/x86/soc.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/suspend.h>
-#include <asm/cpu_device_id.h> -#include <asm/intel-family.h>
#define DRV_NAME "INT0002 Virtual GPIO"
/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */ @@ -151,12 +149,6 @@ static struct irq_chip int0002_irqchip = { .irq_set_wake = int0002_irq_set_wake, };
-static const struct x86_cpu_id int0002_cpu_ids[] = {
X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT, NULL),X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT, NULL),{}-};
static void int0002_init_irq_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask, unsigned int ngpios) @@ -167,15 +159,13 @@ static void int0002_init_irq_valid_mask(struct gpio_chip *chip, static int int0002_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev;
const struct x86_cpu_id *cpu_id; struct int0002_data *int0002; struct gpio_irq_chip *girq; struct gpio_chip *chip; int irq, ret; /* Menlow has a different INT0002 device? <sigh> */cpu_id = x86_match_cpu(int0002_cpu_ids);if (!cpu_id)
if (!soc_intel_is_byt() && !soc_intel_is_cht())
if (!(soc_intel_is_byt() || soc_intel_is_cht()))
?
return -ENODEV; irq = platform_get_irq(pdev, 0);-- 2.31.1
On Mon, Oct 18, 2021 at 6:03 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Mon, Oct 18, 2021 at 5:33 PM Hans de Goede hdegoede@redhat.com wrote:
...
if (!soc_intel_is_byt() && !soc_intel_is_cht())if (!(soc_intel_is_byt() || soc_intel_is_cht()))
?
Self-nak on this. && is slightly better in case we got the first argument false. Proposed variant will always evaluate both.
Use the new soc_intel_is_cht() helper to find out if we are running on a CHT device rather then checking the ACPI _HRV field.
This is more reliable (some CHT devices have been found where the _HRV for the PMIC is 2 rather then 3) and leads to a nice cleanup.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/input/misc/axp20x-pek.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c index 9c6386b2af33..e09b1fae42e1 100644 --- a/drivers/input/misc/axp20x-pek.c +++ b/drivers/input/misc/axp20x-pek.c @@ -22,6 +22,7 @@ #include <linux/kernel.h> #include <linux/mfd/axp20x.h> #include <linux/module.h> +#include <linux/platform_data/x86/soc.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/slab.h> @@ -255,41 +256,24 @@ static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek, return 0; }
-#ifdef CONFIG_ACPI -static bool axp20x_pek_should_register_input(struct axp20x_pek *axp20x_pek, - struct platform_device *pdev) +static bool axp20x_pek_should_register_input(struct axp20x_pek *axp20x_pek) { - unsigned long long hrv = 0; - acpi_status status; - if (IS_ENABLED(CONFIG_INPUT_SOC_BUTTON_ARRAY) && axp20x_pek->axp20x->variant == AXP288_ID) { - status = acpi_evaluate_integer(ACPI_HANDLE(pdev->dev.parent), - "_HRV", NULL, &hrv); - if (ACPI_FAILURE(status)) - dev_err(&pdev->dev, "Failed to get PMIC hardware revision\n"); - /* * On Cherry Trail platforms (hrv == 3), do not register the * input device if there is an "INTCFD9" or "ACPI0011" gpio * button ACPI device, as that handles the power button too, * and otherwise we end up reporting all presses twice. */ - if (hrv == 3 && (acpi_dev_present("INTCFD9", NULL, -1) || + if (soc_intel_is_cht() && + (acpi_dev_present("INTCFD9", NULL, -1) || acpi_dev_present("ACPI0011", NULL, -1))) return false; - }
return true; } -#else -static bool axp20x_pek_should_register_input(struct axp20x_pek *axp20x_pek, - struct platform_device *pdev) -{ - return true; -} -#endif
static int axp20x_pek_probe(struct platform_device *pdev) { @@ -321,7 +305,7 @@ static int axp20x_pek_probe(struct platform_device *pdev) axp20x_pek->irq_dbf = regmap_irq_get_virq( axp20x_pek->axp20x->regmap_irqc, axp20x_pek->irq_dbf);
- if (axp20x_pek_should_register_input(axp20x_pek, pdev)) { + if (axp20x_pek_should_register_input(axp20x_pek)) { error = axp20x_pek_probe_input_device(axp20x_pek, pdev); if (error) return error;
On Mon, Oct 18, 2021 at 04:33:24PM +0200, Hans de Goede wrote:
Use the new soc_intel_is_cht() helper to find out if we are running on a CHT device rather then checking the ACPI _HRV field.
This is more reliable (some CHT devices have been found where the _HRV for the PMIC is 2 rather then 3) and leads to a nice cleanup.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Dmitry Torokhov dmitry.torokhov@gmail.com
Please feel free to merge with the rest of the patches.
Thanks.
On Mon, Oct 18, 2021 at 5:33 PM Hans de Goede hdegoede@redhat.com wrote:
Hi All,
We have been open-coding x86_match_cpu() checks for enabling some SoC specific behavior in various places.
The sound/soc/intel drivers used to also open-code this but this was cleaned up a while ago introducing a number of soc_intel_is_*() helpers.
This series moves the definition of these helpers to a more public place and uses it in a couple of more places outside the sound tree.
Mark, I know we are a bit late in the cycle, but if you can pick up patch 1/3 (assuming on one objects) for 5.16, then the rest can be
I suppose s/on one/no-one/ :-)
applied after 5.16-rc1 is out.
What I like about this series is dropping ugly ifdeffery here and there and consolidating it in one place. Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
P.S. Btw, since you are the maintainer of PDx86 it means either you or Mark (whoever gives an Ack to the other one) can take at least two patches that makes visible that the change is not just for a single user.
Hans de Goede (3): ASoC: Intel: Move soc_intel_is_foo() helpers to a generic header platform/x86: intel_int0002_vgpio: Use the new soc_intel_is_byt/cht helpers Input: axp20x-pek - Use new soc_intel_is_cht() helper
drivers/input/misc/axp20x-pek.c | 26 ++------- drivers/platform/x86/intel/int0002_vgpio.c | 14 +---- include/linux/platform_data/x86/soc.h | 65 ++++++++++++++++++++++ sound/soc/intel/common/soc-intel-quirks.h | 51 +---------------- 4 files changed, 75 insertions(+), 81 deletions(-) create mode 100644 include/linux/platform_data/x86/soc.h
-- 2.31.1
Hi all,
On 10/18/21 16:33, Hans de Goede wrote:
Hi All,
We have been open-coding x86_match_cpu() checks for enabling some SoC specific behavior in various places.
The sound/soc/intel drivers used to also open-code this but this was cleaned up a while ago introducing a number of soc_intel_is_*() helpers.
This series moves the definition of these helpers to a more public place and uses it in a couple of more places outside the sound tree.
Mark, I know we are a bit late in the cycle, but if you can pick up patch 1/3 (assuming on one objects) for 5.16, then the rest can be applied after 5.16-rc1 is out.
Regards,
Hans
Thank you all for the review and acks. I've pushed the entire series to: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.g... now, with Andy's Reviewed-by and the acks added and Andy's nitpicks addressed.
Once the buildbot has had a chance to play with this I'll push this out to platform-drivers-x86/for-next.
Regards,
Hans
Hans de Goede (3): ASoC: Intel: Move soc_intel_is_foo() helpers to a generic header platform/x86: intel_int0002_vgpio: Use the new soc_intel_is_byt/cht helpers Input: axp20x-pek - Use new soc_intel_is_cht() helper
drivers/input/misc/axp20x-pek.c | 26 ++------- drivers/platform/x86/intel/int0002_vgpio.c | 14 +---- include/linux/platform_data/x86/soc.h | 65 ++++++++++++++++++++++ sound/soc/intel/common/soc-intel-quirks.h | 51 +---------------- 4 files changed, 75 insertions(+), 81 deletions(-) create mode 100644 include/linux/platform_data/x86/soc.h
participants (4)
-
Andy Shevchenko -
Dmitry Torokhov -
Hans de Goede -
Mark Brown