* [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs
@ 2020-02-15 16:09 Alexander Koskovich
2020-02-15 11:58 ` Thomas Gleixner
2020-02-17 7:59 ` Zhang Rui
0 siblings, 2 replies; 3+ messages in thread
From: Alexander Koskovich @ 2020-02-15 16:09 UTC (permalink / raw)
To: arjan, jacob.jun.pan
Cc: zvnexus, Alexander Koskovich, Zhang Rui, Daniel Lezcano,
Amit Kucheria, Greg Kroah-Hartman, Luc Van Oostenryck,
Petr Mladek, Thomas Gleixner, linux-pm, linux-kernel
Resolves dmesg error "intel_powerclamp: CPU does not support MWAIT".
The error that is outputted in dmesg prior to this patch
is innacurate, AMD Ryzen CPUs do support MWAIT. We could
also add the AMD vendor to the MWAIT check, but even though
AMD CPUs do support MWAIT, they fail the C-state package
check so it's better just to bail out in the beginning.
Signed-off-by: Alexander Koskovich <zvnexus@outlook.com>
---
drivers/thermal/intel/intel_powerclamp.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c
index 53216dcbe173..3c5b25bfa596 100644
--- a/drivers/thermal/intel/intel_powerclamp.c
+++ b/drivers/thermal/intel/intel_powerclamp.c
@@ -650,6 +650,11 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
.set_cur_state = powerclamp_set_cur_state,
};
+static const struct x86_cpu_id amd_cpu[] = {
+ { X86_VENDOR_AMD },
+ {},
+};
+
static const struct x86_cpu_id __initconst intel_powerclamp_ids[] = {
{ X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_MWAIT },
{}
@@ -659,6 +664,11 @@ MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
static int __init powerclamp_probe(void)
{
+ if (x86_match_cpu(amd_cpu)) {
+ pr_info("Intel PowerClamp does not support AMD CPUs\n");
+ return -ENODEV;
+ }
+
if (!x86_match_cpu(intel_powerclamp_ids)) {
pr_err("CPU does not support MWAIT\n");
return -ENODEV;
--
2.25.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs
2020-02-15 16:09 [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs Alexander Koskovich
@ 2020-02-15 11:58 ` Thomas Gleixner
2020-02-17 7:59 ` Zhang Rui
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Gleixner @ 2020-02-15 11:58 UTC (permalink / raw)
To: Alexander Koskovich, arjan, jacob.jun.pan
Cc: zvnexus, Alexander Koskovich, Zhang Rui, Daniel Lezcano,
Amit Kucheria, Greg Kroah-Hartman, Luc Van Oostenryck,
Petr Mladek, linux-pm, linux-kernel
Alexander Koskovich <zvnexus@gmail.com> writes:
> Resolves dmesg error "intel_powerclamp: CPU does not support MWAIT".
>
> The error that is outputted in dmesg prior to this patch
> is innacurate, AMD Ryzen CPUs do support MWAIT. We could
> also add the AMD vendor to the MWAIT check, but even though
> AMD CPUs do support MWAIT, they fail the C-state package
> check so it's better just to bail out in the beginning.
>
> Signed-off-by: Alexander Koskovich <zvnexus@outlook.com>
> ---
> drivers/thermal/intel/intel_powerclamp.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c
> index 53216dcbe173..3c5b25bfa596 100644
> --- a/drivers/thermal/intel/intel_powerclamp.c
> +++ b/drivers/thermal/intel/intel_powerclamp.c
> @@ -650,6 +650,11 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
> .set_cur_state = powerclamp_set_cur_state,
> };
>
> +static const struct x86_cpu_id amd_cpu[] = {
> + { X86_VENDOR_AMD },
> + {},
> +};
> +
> static const struct x86_cpu_id __initconst intel_powerclamp_ids[] = {
> { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_MWAIT },
> {}
> @@ -659,6 +664,11 @@ MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
> static int __init powerclamp_probe(void)
> {
>
> + if (x86_match_cpu(amd_cpu)) {
> + pr_info("Intel PowerClamp does not support AMD CPUs\n");
> + return -ENODEV;
This is still running into the same problem on all other non Intel
vendors, e.g. HYGON, VIA ....
> + }
> +
> if (!x86_match_cpu(intel_powerclamp_ids)) {
> pr_err("CPU does not support MWAIT\n");
> return -ENODEV;
The right thing to do is to remove this silly pr_err(). It's not an
error when a CPU does not support MWAIT. It's a fact and even older
Intel CPUs do not have MWAIT.
We do not print "Machine does not have $FEATURE" in device drivers and
whatever code which depends on runtime detection either. dmesg would be
full of this.
Thanks,
tglx
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs
2020-02-15 16:09 [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs Alexander Koskovich
2020-02-15 11:58 ` Thomas Gleixner
@ 2020-02-17 7:59 ` Zhang Rui
1 sibling, 0 replies; 3+ messages in thread
From: Zhang Rui @ 2020-02-17 7:59 UTC (permalink / raw)
To: Alexander Koskovich, arjan, jacob.jun.pan
Cc: Alexander Koskovich, Daniel Lezcano, Amit Kucheria,
Greg Kroah-Hartman, Luc Van Oostenryck, Petr Mladek,
Thomas Gleixner, linux-pm, linux-kernel
On Sat, 2020-02-15 at 11:09 -0500, Alexander Koskovich wrote:
> Resolves dmesg error "intel_powerclamp: CPU does not support MWAIT".
>
> The error that is outputted in dmesg prior to this patch
> is innacurate, AMD Ryzen CPUs do support MWAIT. We could
> also add the AMD vendor to the MWAIT check, but even though
> AMD CPUs do support MWAIT, they fail the C-state package
> check so it's better just to bail out in the beginning.
>
> Signed-off-by: Alexander Koskovich <zvnexus@outlook.com>
I think you're resending the same patch.
As intel_powerclamp_ids already checks for Intel CPUs, so we don't need
the check for AMD CPU, but instead, just remove the following line
"pr_err("CPU does not support MWAIT\n");"
when intel_powerclamp driver fails to probe.
thanks,
rui
> ---
> drivers/thermal/intel/intel_powerclamp.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/thermal/intel/intel_powerclamp.c
> b/drivers/thermal/intel/intel_powerclamp.c
> index 53216dcbe173..3c5b25bfa596 100644
> --- a/drivers/thermal/intel/intel_powerclamp.c
> +++ b/drivers/thermal/intel/intel_powerclamp.c
> @@ -650,6 +650,11 @@ static struct thermal_cooling_device_ops
> powerclamp_cooling_ops = {
> .set_cur_state = powerclamp_set_cur_state,
> };
>
> +static const struct x86_cpu_id amd_cpu[] = {
> + { X86_VENDOR_AMD },
> + {},
> +};
> +
> static const struct x86_cpu_id __initconst intel_powerclamp_ids[] =
> {
> { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY,
> X86_FEATURE_MWAIT },
> {}
> @@ -659,6 +664,11 @@ MODULE_DEVICE_TABLE(x86cpu,
> intel_powerclamp_ids);
> static int __init powerclamp_probe(void)
> {
>
> + if (x86_match_cpu(amd_cpu)) {
> + pr_info("Intel PowerClamp does not support AMD
> CPUs\n");
> + return -ENODEV;
> + }
> +
> if (!x86_match_cpu(intel_powerclamp_ids)) {
> pr_err("CPU does not support MWAIT\n");
> return -ENODEV;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-17 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-15 16:09 [PATCH] thermal/intel_powerclamp: Don't report an error for AMD CPUs Alexander Koskovich
2020-02-15 11:58 ` Thomas Gleixner
2020-02-17 7:59 ` Zhang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).