2019-05-08 21:24:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: linux-next: Tree for May 8 (drivers/platform/x86/intel_pmc_core_plat_drv.c)

On Wed, May 8, 2019 at 11:45 PM Randy Dunlap <[email protected]> wrote:
>
> On 5/8/19 12:34 AM, Stephen Rothwell wrote:
> > Hi all,
> >
> > Changes since 20190507:
> >
> > The ubifs tree gained a conflict against Linus' tree.
> >
>
> on i386 or x86_64:

Thank you for report. Can you provide what is the config option for
this module? I suppose it's built-in.
Rajat, I will drop this from the repo, because I don't see it would
have a chance to be tested in time.

>
>
> CC drivers/platform/x86/intel_pmc_core_plat_drv.o
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: data definition has no type or storage class [enabled by default]
> MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids);
> ^
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: error: type defaults to ‘int’ in declaration of ‘MODULE_DEVICE_TABLE’ [-Werror=implicit-int]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: parameter names (without types) in function declaration [enabled by default]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: data definition has no type or storage class [enabled by default]
> module_init(pmc_core_platform_init);
> ^
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: parameter names (without types) in function declaration [enabled by default]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: data definition has no type or storage class [enabled by default]
> module_exit(pmc_core_platform_exit);
> ^
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: error: type defaults to ‘int’ in declaration of ‘module_exit’ [-Werror=implicit-int]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: parameter names (without types) in function declaration [enabled by default]
> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:42:19: warning: ‘pmc_core_platform_init’ defined but not used [-Wunused-function]
> static int __init pmc_core_platform_init(void)
> ^
>
> and
> WARNING: modpost: missing MODULE_LICENSE() in drivers/platform/x86/intel_pmc_core_plat_drv.o
>
> --
> ~Randy



--
With Best Regards,
Andy Shevchenko


2019-05-08 21:57:29

by Randy Dunlap

[permalink] [raw]
Subject: Re: linux-next: Tree for May 8 (drivers/platform/x86/intel_pmc_core_plat_drv.c)

On 5/8/19 2:21 PM, Andy Shevchenko wrote:
> On Wed, May 8, 2019 at 11:45 PM Randy Dunlap <[email protected]> wrote:
>>
>> On 5/8/19 12:34 AM, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Changes since 20190507:
>>>
>>> The ubifs tree gained a conflict against Linus' tree.
>>>
>>
>> on i386 or x86_64:
>
> Thank you for report. Can you provide what is the config option for
> this module? I suppose it's built-in.

Yes, CONFIG_INTEL_PMC_CORE=y

One i386 randconfig and one x86_64 randconfig are attached.

> Rajat, I will drop this from the repo, because I don't see it would
> have a chance to be tested in time.
>
>>
>>
>> CC drivers/platform/x86/intel_pmc_core_plat_drv.o
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: data definition has no type or storage class [enabled by default]
>> MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids);
>> ^
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: error: type defaults to ‘int’ in declaration of ‘MODULE_DEVICE_TABLE’ [-Werror=implicit-int]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: parameter names (without types) in function declaration [enabled by default]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: data definition has no type or storage class [enabled by default]
>> module_init(pmc_core_platform_init);
>> ^
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: parameter names (without types) in function declaration [enabled by default]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: data definition has no type or storage class [enabled by default]
>> module_exit(pmc_core_platform_exit);
>> ^
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: error: type defaults to ‘int’ in declaration of ‘module_exit’ [-Werror=implicit-int]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: parameter names (without types) in function declaration [enabled by default]
>> ../drivers/platform/x86/intel_pmc_core_plat_drv.c:42:19: warning: ‘pmc_core_platform_init’ defined but not used [-Wunused-function]
>> static int __init pmc_core_platform_init(void)
>> ^
>>
>> and
>> WARNING: modpost: missing MODULE_LICENSE() in drivers/platform/x86/intel_pmc_core_plat_drv.o

For this warning, CONFIG_INTEL_PMC_CORE=m


--
~Randy


Attachments:
config-r5187 (94.85 kB)
config-r5182 (106.09 kB)
Download all attachments

2019-05-08 23:39:47

by Rajat Jain

[permalink] [raw]
Subject: Re: linux-next: Tree for May 8 (drivers/platform/x86/intel_pmc_core_plat_drv.c)

From: Andy Shevchenko <[email protected]>
Date: Wed, May 8, 2019 at 2:22 PM
To: Randy Dunlap
Cc: Stephen Rothwell, Linux Next Mailing List, Linux Kernel Mailing
List, Rajat Jain, Platform Driver, Rajneesh Bhardwaj, Vishwanath
Somayaji

> On Wed, May 8, 2019 at 11:45 PM Randy Dunlap <[email protected]> wrote:
> >
> > On 5/8/19 12:34 AM, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > Changes since 20190507:
> > >
> > > The ubifs tree gained a conflict against Linus' tree.
> > >
> >
> > on i386 or x86_64:
>
> Thank you for report. Can you provide what is the config option for
> this module? I suppose it's built-in.
> Rajat, I will drop this from the repo, because I don't see it would
> have a chance to be tested in time.


OK, NP. Just to be sure I understand,

1) Please let me know if I should send in a fix (it would be
#include/linux/module.h and also add MODULE_LICENSE() I believe)?
2) Would this be lined up for next version though?

Thanks,
Rajat


>
>
> >
> >
> > CC drivers/platform/x86/intel_pmc_core_plat_drv.o
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: data definition has no type or storage class [enabled by default]
> > MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids);
> > ^
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: error: type defaults to ‘int’ in declaration of ‘MODULE_DEVICE_TABLE’ [-Werror=implicit-int]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:40:1: warning: parameter names (without types) in function declaration [enabled by default]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: data definition has no type or storage class [enabled by default]
> > module_init(pmc_core_platform_init);
> > ^
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: error: type defaults to ‘int’ in declaration of ‘module_init’ [-Werror=implicit-int]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:59:1: warning: parameter names (without types) in function declaration [enabled by default]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: data definition has no type or storage class [enabled by default]
> > module_exit(pmc_core_platform_exit);
> > ^
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: error: type defaults to ‘int’ in declaration of ‘module_exit’ [-Werror=implicit-int]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:60:1: warning: parameter names (without types) in function declaration [enabled by default]
> > ../drivers/platform/x86/intel_pmc_core_plat_drv.c:42:19: warning: ‘pmc_core_platform_init’ defined but not used [-Wunused-function]
> > static int __init pmc_core_platform_init(void)
> > ^
> >
> > and
> > WARNING: modpost: missing MODULE_LICENSE() in drivers/platform/x86/intel_pmc_core_plat_drv.o
> >
> > --
> > ~Randy
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

2019-06-11 22:41:07

by Andy Shevchenko

[permalink] [raw]
Subject: Re: linux-next: Tree for May 8 (drivers/platform/x86/intel_pmc_core_plat_drv.c)

On Thu, May 9, 2019 at 2:15 AM Rajat Jain <[email protected]> wrote:

> OK, NP. Just to be sure I understand,
>
> 1) Please let me know if I should send in a fix (it would be
> #include/linux/module.h and also add MODULE_LICENSE() I believe)?
> 2) Would this be lined up for next version though?

Resend a complete series based on the latest stuff we have in our
for-next branch.

--
With Best Regards,
Andy Shevchenko

2019-06-28 03:35:16

by Rajat Jain

[permalink] [raw]
Subject: [PATCH v7] platform/x86: intel_pmc_core: Attach using APCI HID "INT33A1"

Most modern platforms already have the ACPI device "INT33A1" that could
be used to attach to the driver. Switch the driver to using that and
thus make the intel_pmc_core.c a pure platform_driver.

Some of the legacy platforms though, may still not have this ACPI device
in their ACPI tables. Thus for such platforms, move the code to manually
instantiate a platform_device into a new file of its own. This would
instantiate the intel_pmc_core platform device and thus attach to
the driver, if the ACPI device for the same ("INT33A1") is not present
in a system where it should be. This was discussed here:
https://www.mail-archive.com/[email protected]/msg1966991.html

Signed-off-by: Rajat Jain <[email protected]>
---
v7: Include module.h and add MODULE_LICENSE()
v6: was here: https://patchwork.kernel.org/patch/10906387/
(The patchset was accepted, but this particular patch was later
dropped becasue it created some warnings in certain kernel
configurations: https://www.spinics.net/lists/linux-next/msg47611.html)

drivers/platform/x86/Makefile | 2 +-
drivers/platform/x86/intel_pmc_core.c | 40 +++---------
.../platform/x86/intel_pmc_core_plat_drv.c | 62 +++++++++++++++++++
3 files changed, 71 insertions(+), 33 deletions(-)
create mode 100644 drivers/platform/x86/intel_pmc_core_plat_drv.c

diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 87b0069bd781..4ba707111c27 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -89,7 +89,7 @@ obj-$(CONFIG_INTEL_BXTWC_PMIC_TMU) += intel_bxtwc_tmu.o
obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \
intel_telemetry_pltdrv.o \
intel_telemetry_debugfs.o
-obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o
+obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o intel_pmc_core_plat_drv.o
obj-$(CONFIG_PMC_ATOM) += pmc_atom.o
obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o
obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index 1d902230ba61..f20d08ad39ea 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -1023,47 +1023,23 @@ static const struct dev_pm_ops pmc_core_pm_ops = {
SET_LATE_SYSTEM_SLEEP_PM_OPS(pmc_core_suspend, pmc_core_resume)
};

+static const struct acpi_device_id pmc_core_acpi_ids[] = {
+ {"INT33A1", 0}, /* _HID for Intel Power Engine, _CID PNP0D80*/
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, pmc_core_acpi_ids);
+
static struct platform_driver pmc_core_driver = {
.driver = {
.name = "intel_pmc_core",
+ .acpi_match_table = ACPI_PTR(pmc_core_acpi_ids),
.pm = &pmc_core_pm_ops,
},
.probe = pmc_core_probe,
.remove = pmc_core_remove,
};

-static struct platform_device pmc_core_device = {
- .name = "intel_pmc_core",
-};
-
-static int __init pmc_core_init(void)
-{
- int ret;
-
- if (!x86_match_cpu(intel_pmc_core_ids))
- return -ENODEV;
-
- ret = platform_driver_register(&pmc_core_driver);
- if (ret)
- return ret;
-
- ret = platform_device_register(&pmc_core_device);
- if (ret) {
- platform_driver_unregister(&pmc_core_driver);
- return ret;
- }
-
- return 0;
-}
-
-static void __exit pmc_core_exit(void)
-{
- platform_device_unregister(&pmc_core_device);
- platform_driver_unregister(&pmc_core_driver);
-}
-
-module_init(pmc_core_init)
-module_exit(pmc_core_exit)
+module_platform_driver(pmc_core_driver);

MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Intel PMC Core Driver");
diff --git a/drivers/platform/x86/intel_pmc_core_plat_drv.c b/drivers/platform/x86/intel_pmc_core_plat_drv.c
new file mode 100644
index 000000000000..a8754a6db1b8
--- /dev/null
+++ b/drivers/platform/x86/intel_pmc_core_plat_drv.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Intel PMC Core platform init
+ * Copyright (c) 2019, Google Inc.
+ * Author - Rajat Jain
+ *
+ * This code instantiates platform devices for intel_pmc_core driver, only
+ * on supported platforms that may not have the ACPI devices in the ACPI tables.
+ * No new platforms should be added here, because we expect that new platforms
+ * should all have the ACPI device, which is the preferred way of enumeration.
+ */
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
+static struct platform_device pmc_core_device = {
+ .name = "intel_pmc_core",
+};
+
+/*
+ * intel_pmc_core_platform_ids is the list of platforms where we want to
+ * instantiate the platform_device if not already instantiated. This is
+ * different than intel_pmc_core_ids in intel_pmc_core.c which is the
+ * list of platforms that the driver supports for pmc_core device. The
+ * other list may grow, but this list should not.
+ */
+static const struct x86_cpu_id intel_pmc_core_platform_ids[] = {
+ INTEL_CPU_FAM6(SKYLAKE_MOBILE, pmc_core_device),
+ INTEL_CPU_FAM6(SKYLAKE_DESKTOP, pmc_core_device),
+ INTEL_CPU_FAM6(KABYLAKE_MOBILE, pmc_core_device),
+ INTEL_CPU_FAM6(KABYLAKE_DESKTOP, pmc_core_device),
+ INTEL_CPU_FAM6(CANNONLAKE_MOBILE, pmc_core_device),
+ INTEL_CPU_FAM6(ICELAKE_MOBILE, pmc_core_device),
+ {}
+};
+MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids);
+
+static int __init pmc_core_platform_init(void)
+{
+ /* Skip creating the platform device if ACPI already has a device */
+ if (acpi_dev_present("INT33A1", NULL, -1))
+ return -ENODEV;
+
+ if (!x86_match_cpu(intel_pmc_core_platform_ids))
+ return -ENODEV;
+
+ return platform_device_register(&pmc_core_device);
+}
+
+static void __exit pmc_core_platform_exit(void)
+{
+ platform_device_unregister(&pmc_core_device);
+}
+
+module_init(pmc_core_platform_init);
+module_exit(pmc_core_platform_exit);
+MODULE_LICENSE("GPL v2");
--
2.22.0.410.gd8fdbe21b5-goog

2019-06-28 03:39:29

by Rajat Jain

[permalink] [raw]
Subject: Re: linux-next: Tree for May 8 (drivers/platform/x86/intel_pmc_core_plat_drv.c)

Hi Andy,

On Tue, Jun 11, 2019 at 12:06 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, May 9, 2019 at 2:15 AM Rajat Jain <[email protected]> wrote:
>
> > OK, NP. Just to be sure I understand,
> >
> > 1) Please let me know if I should send in a fix (it would be
> > #include/linux/module.h and also add MODULE_LICENSE() I believe)?
> > 2) Would this be lined up for next version though?
>
> Resend a complete series based on the latest stuff we have in our
> for-next branch.

My apologies for the delay in resending. I just sent a v7 of the patch
that was dropped:
https://lkml.org/lkml/2019/6/27/1264

Only 2 changes from the v6:

#include <linux/module.h>

and

MODULE_LICENSE()

Thanks & Best Regards,

Rajat

>
> --
> With Best Regards,
> Andy Shevchenko

2019-08-23 15:23:42

by Rajat Jain

[permalink] [raw]
Subject: Re: [PATCH v7] platform/x86: intel_pmc_core: Attach using APCI HID "INT33A1"

On Thu, Jun 27, 2019 at 8:34 PM Rajat Jain <[email protected]> wrote:
>
> Most modern platforms already have the ACPI device "INT33A1" that could
> be used to attach to the driver. Switch the driver to using that and
> thus make the intel_pmc_core.c a pure platform_driver.
>
> Some of the legacy platforms though, may still not have this ACPI device
> in their ACPI tables. Thus for such platforms, move the code to manually
> instantiate a platform_device into a new file of its own. This would
> instantiate the intel_pmc_core platform device and thus attach to
> the driver, if the ACPI device for the same ("INT33A1") is not present
> in a system where it should be. This was discussed here:
> https://www.mail-archive.com/[email protected]/msg1966991.html


HI Andy, Rajneesh, this seems to have fallen through the cracks. Any
comments here on this patch?

Thanks,

Rajat


>
> Signed-off-by: Rajat Jain <[email protected]>
> ---
> v7: Include module.h and add MODULE_LICENSE()
> v6: was here: https://patchwork.kernel.org/patch/10906387/
> (The patchset was accepted, but this particular patch was later
> dropped becasue it created some warnings in certain kernel
> configurations: https://www.spinics.net/lists/linux-next/msg47611.html)
>
> drivers/platform/x86/Makefile | 2 +-
> drivers/platform/x86/intel_pmc_core.c | 40 +++---------
> .../platform/x86/intel_pmc_core_plat_drv.c | 62 +++++++++++++++++++
> 3 files changed, 71 insertions(+), 33 deletions(-)
> create mode 100644 drivers/platform/x86/intel_pmc_core_plat_drv.c
>
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 87b0069bd781..4ba707111c27 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -89,7 +89,7 @@ obj-$(CONFIG_INTEL_BXTWC_PMIC_TMU) += intel_bxtwc_tmu.o
> obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \
> intel_telemetry_pltdrv.o \
> intel_telemetry_debugfs.o
> -obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o
> +obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o intel_pmc_core_plat_drv.o
> obj-$(CONFIG_PMC_ATOM) += pmc_atom.o
> obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o
> obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index 1d902230ba61..f20d08ad39ea 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -1023,47 +1023,23 @@ static const struct dev_pm_ops pmc_core_pm_ops = {
> SET_LATE_SYSTEM_SLEEP_PM_OPS(pmc_core_suspend, pmc_core_resume)
> };
>
> +static const struct acpi_device_id pmc_core_acpi_ids[] = {
> + {"INT33A1", 0}, /* _HID for Intel Power Engine, _CID PNP0D80*/
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pmc_core_acpi_ids);
> +
> static struct platform_driver pmc_core_driver = {
> .driver = {
> .name = "intel_pmc_core",
> + .acpi_match_table = ACPI_PTR(pmc_core_acpi_ids),
> .pm = &pmc_core_pm_ops,
> },
> .probe = pmc_core_probe,
> .remove = pmc_core_remove,
> };
>
> -static struct platform_device pmc_core_device = {
> - .name = "intel_pmc_core",
> -};
> -
> -static int __init pmc_core_init(void)
> -{
> - int ret;
> -
> - if (!x86_match_cpu(intel_pmc_core_ids))
> - return -ENODEV;
> -
> - ret = platform_driver_register(&pmc_core_driver);
> - if (ret)
> - return ret;
> -
> - ret = platform_device_register(&pmc_core_device);
> - if (ret) {
> - platform_driver_unregister(&pmc_core_driver);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static void __exit pmc_core_exit(void)
> -{
> - platform_device_unregister(&pmc_core_device);
> - platform_driver_unregister(&pmc_core_driver);
> -}
> -
> -module_init(pmc_core_init)
> -module_exit(pmc_core_exit)
> +module_platform_driver(pmc_core_driver);
>
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("Intel PMC Core Driver");
> diff --git a/drivers/platform/x86/intel_pmc_core_plat_drv.c b/drivers/platform/x86/intel_pmc_core_plat_drv.c
> new file mode 100644
> index 000000000000..a8754a6db1b8
> --- /dev/null
> +++ b/drivers/platform/x86/intel_pmc_core_plat_drv.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Intel PMC Core platform init
> + * Copyright (c) 2019, Google Inc.
> + * Author - Rajat Jain
> + *
> + * This code instantiates platform devices for intel_pmc_core driver, only
> + * on supported platforms that may not have the ACPI devices in the ACPI tables.
> + * No new platforms should be added here, because we expect that new platforms
> + * should all have the ACPI device, which is the preferred way of enumeration.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +
> +static struct platform_device pmc_core_device = {
> + .name = "intel_pmc_core",
> +};
> +
> +/*
> + * intel_pmc_core_platform_ids is the list of platforms where we want to
> + * instantiate the platform_device if not already instantiated. This is
> + * different than intel_pmc_core_ids in intel_pmc_core.c which is the
> + * list of platforms that the driver supports for pmc_core device. The
> + * other list may grow, but this list should not.
> + */
> +static const struct x86_cpu_id intel_pmc_core_platform_ids[] = {
> + INTEL_CPU_FAM6(SKYLAKE_MOBILE, pmc_core_device),
> + INTEL_CPU_FAM6(SKYLAKE_DESKTOP, pmc_core_device),
> + INTEL_CPU_FAM6(KABYLAKE_MOBILE, pmc_core_device),
> + INTEL_CPU_FAM6(KABYLAKE_DESKTOP, pmc_core_device),
> + INTEL_CPU_FAM6(CANNONLAKE_MOBILE, pmc_core_device),
> + INTEL_CPU_FAM6(ICELAKE_MOBILE, pmc_core_device),
> + {}
> +};
> +MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids);
> +
> +static int __init pmc_core_platform_init(void)
> +{
> + /* Skip creating the platform device if ACPI already has a device */
> + if (acpi_dev_present("INT33A1", NULL, -1))
> + return -ENODEV;
> +
> + if (!x86_match_cpu(intel_pmc_core_platform_ids))
> + return -ENODEV;
> +
> + return platform_device_register(&pmc_core_device);
> +}
> +
> +static void __exit pmc_core_platform_exit(void)
> +{
> + platform_device_unregister(&pmc_core_device);
> +}
> +
> +module_init(pmc_core_platform_init);
> +module_exit(pmc_core_platform_exit);
> +MODULE_LICENSE("GPL v2");
> --
> 2.22.0.410.gd8fdbe21b5-goog
>

2019-09-09 03:37:38

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v7] platform/x86: intel_pmc_core: Attach using APCI HID "INT33A1"

On Fri, Aug 23, 2019 at 1:44 AM Rajat Jain <[email protected]> wrote:
>
> On Thu, Jun 27, 2019 at 8:34 PM Rajat Jain <[email protected]> wrote:
> >
> > Most modern platforms already have the ACPI device "INT33A1" that could
> > be used to attach to the driver. Switch the driver to using that and
> > thus make the intel_pmc_core.c a pure platform_driver.
> >
> > Some of the legacy platforms though, may still not have this ACPI device
> > in their ACPI tables. Thus for such platforms, move the code to manually
> > instantiate a platform_device into a new file of its own. This would
> > instantiate the intel_pmc_core platform device and thus attach to
> > the driver, if the ACPI device for the same ("INT33A1") is not present
> > in a system where it should be. This was discussed here:
> > https://www.mail-archive.com/[email protected]/msg1966991.html
>
>
> HI Andy, Rajneesh, this seems to have fallen through the cracks. Any
> comments here on this patch?

% git tag --list v5.3* --contains b02f6a2ef0a1
v5.3-rc1

--
With Best Regards,
Andy Shevchenko

2019-09-09 13:34:13

by Rajat Jain

[permalink] [raw]
Subject: Re: [PATCH v7] platform/x86: intel_pmc_core: Attach using APCI HID "INT33A1"

On Sat, Sep 7, 2019 at 10:42 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Aug 23, 2019 at 1:44 AM Rajat Jain <[email protected]> wrote:
> >
> > On Thu, Jun 27, 2019 at 8:34 PM Rajat Jain <[email protected]> wrote:
> > >
> > > Most modern platforms already have the ACPI device "INT33A1" that could
> > > be used to attach to the driver. Switch the driver to using that and
> > > thus make the intel_pmc_core.c a pure platform_driver.
> > >
> > > Some of the legacy platforms though, may still not have this ACPI device
> > > in their ACPI tables. Thus for such platforms, move the code to manually
> > > instantiate a platform_device into a new file of its own. This would
> > > instantiate the intel_pmc_core platform device and thus attach to
> > > the driver, if the ACPI device for the same ("INT33A1") is not present
> > > in a system where it should be. This was discussed here:
> > > https://www.mail-archive.com/[email protected]/msg1966991.html
> >
> >
> > HI Andy, Rajneesh, this seems to have fallen through the cracks. Any
> > comments here on this patch?
>
> % git tag --list v5.3* --contains b02f6a2ef0a1
> v5.3-rc1

Oops, my apologies, I failed to notice it.

Thanks,

Rajat
>
> --
> With Best Regards,
> Andy Shevchenko