linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
@ 2019-02-13 12:21 Erwan Velu
  2019-02-14  9:28 ` Erwan Velu
  0 siblings, 1 reply; 7+ messages in thread
From: Erwan Velu @ 2019-02-13 12:21 UTC (permalink / raw)
  Cc: rafael, Erwan Velu, Srinivas Pandruvada, Len Brown,
	Rafael J. Wysocki, Viresh Kumar, open list:INTEL PSTATE DRIVER,
	open list

The init code path has several exceptions where the module can decide not to load.
As CONFIG_X86_INTEL_PSTATE is generally set to Y, the return code is not reachable.
The initialization code is neither verbose of the reason why it did choose to prematurely exit.

This situation leads to a situation where its difficult for a user to determine,
on a given platform, why the driver didn't load properly.

This patch is about reporting to the user the reason/context of why the driver failed to load.
That is a precious hint when debugging a platform.

Signed-off-by: Erwan Velu <e.velu@criteo.com>
---
 drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dd66decf2087..e1ae309923bf 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2475,6 +2475,7 @@ static bool __init intel_pstate_no_acpi_pss(void)
 		kfree(pss);
 	}
 
+	pr_debug("ACPI _PSS not found\n");
 	return true;
 }
 
@@ -2484,10 +2485,15 @@ static bool __init intel_pstate_no_acpi_pcch(void)
 	acpi_handle handle;
 
 	status = acpi_get_handle(NULL, "\\_SB", &handle);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
+		pr_debug("ACPI PCCH not found\n");
 		return true;
+	}
 
-	return !acpi_has_method(handle, "PCCH");
+	status = acpi_has_method(handle, "PCCH");
+	if (!status)
+		pr_debug("ACPI PCCH not found\n");
+	return !status;
 }
 
 static bool __init intel_pstate_has_acpi_ppc(void)
@@ -2502,6 +2508,7 @@ static bool __init intel_pstate_has_acpi_ppc(void)
 		if (acpi_has_method(pr->handle, "_PPC"))
 			return true;
 	}
+	pr_debug("ACPI _PPC not found\n");
 	return false;
 }
 
@@ -2539,8 +2546,10 @@ static bool __init intel_pstate_platform_pwr_mgmt_exists(void)
 	id = x86_match_cpu(intel_pstate_cpu_oob_ids);
 	if (id) {
 		rdmsrl(MSR_MISC_PWR_MGMT, misc_pwr);
-		if ( misc_pwr & (1 << 8))
+		if (misc_pwr & (1 << 8)) {
+			pr_debug("MSR_MISC_PWR_MGMT enabled\n");
 			return true;
+		}
 	}
 
 	idx = acpi_match_platform_list(plat_info);
@@ -2606,22 +2615,28 @@ static int __init intel_pstate_init(void)
 		}
 	} else {
 		id = x86_match_cpu(intel_pstate_cpu_ids);
-		if (!id)
+		if (!id) {
+			pr_info("CPU ID is not in the list of supported devices\n");
 			return -ENODEV;
+		}
 
 		copy_cpu_funcs((struct pstate_funcs *)id->driver_data);
 	}
 
-	if (intel_pstate_msrs_not_valid())
+	if (intel_pstate_msrs_not_valid()) {
+		pr_warn("Cannot enable driver as per invalid MSRs\n");
 		return -ENODEV;
+	}
 
 hwp_cpu_matched:
 	/*
 	 * The Intel pstate driver will be ignored if the platform
 	 * firmware has its own power management modes.
 	 */
-	if (intel_pstate_platform_pwr_mgmt_exists())
+	if (intel_pstate_platform_pwr_mgmt_exists()) {
+		pr_info("Platform already taking care of power management\n");
 		return -ENODEV;
+	}
 
 	if (!hwp_active && hwp_only)
 		return -ENOTSUPP;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-13 12:21 [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit Erwan Velu
@ 2019-02-14  9:28 ` Erwan Velu
  2019-02-14 10:22   ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Erwan Velu @ 2019-02-14  9:28 UTC (permalink / raw)
  To: rafael
  Cc: Erwan Velu, Srinivas Pandruvada, Len Brown, Rafael J. Wysocki,
	Viresh Kumar, open list:INTEL PSTATE DRIVER, open list

Hey rafael,

Does the V6 looks good to you ?

Thanks,

Erwan,

Le 13/02/2019 à 13:21, Erwan Velu a écrit :
> The init code path has several exceptions where the module can decide not to load.
> As CONFIG_X86_INTEL_PSTATE is generally set to Y, the return code is not reachable.
> The initialization code is neither verbose of the reason why it did choose to prematurely exit.
>
> This situation leads to a situation where its difficult for a user to determine,
> on a given platform, why the driver didn't load properly.
>
> This patch is about reporting to the user the reason/context of why the driver failed to load.
> That is a precious hint when debugging a platform.
>
> Signed-off-by: Erwan Velu <e.velu@criteo.com>
> ---
>   drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dd66decf2087..e1ae309923bf 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2475,6 +2475,7 @@ static bool __init intel_pstate_no_acpi_pss(void)
>   		kfree(pss);
>   	}
>   
> +	pr_debug("ACPI _PSS not found\n");
>   	return true;
>   }
>   
> @@ -2484,10 +2485,15 @@ static bool __init intel_pstate_no_acpi_pcch(void)
>   	acpi_handle handle;
>   
>   	status = acpi_get_handle(NULL, "\\_SB", &handle);
> -	if (ACPI_FAILURE(status))
> +	if (ACPI_FAILURE(status)) {
> +		pr_debug("ACPI PCCH not found\n");
>   		return true;
> +	}
>   
> -	return !acpi_has_method(handle, "PCCH");
> +	status = acpi_has_method(handle, "PCCH");
> +	if (!status)
> +		pr_debug("ACPI PCCH not found\n");
> +	return !status;
>   }
>   
>   static bool __init intel_pstate_has_acpi_ppc(void)
> @@ -2502,6 +2508,7 @@ static bool __init intel_pstate_has_acpi_ppc(void)
>   		if (acpi_has_method(pr->handle, "_PPC"))
>   			return true;
>   	}
> +	pr_debug("ACPI _PPC not found\n");
>   	return false;
>   }
>   
> @@ -2539,8 +2546,10 @@ static bool __init intel_pstate_platform_pwr_mgmt_exists(void)
>   	id = x86_match_cpu(intel_pstate_cpu_oob_ids);
>   	if (id) {
>   		rdmsrl(MSR_MISC_PWR_MGMT, misc_pwr);
> -		if ( misc_pwr & (1 << 8))
> +		if (misc_pwr & (1 << 8)) {
> +			pr_debug("MSR_MISC_PWR_MGMT enabled\n");
>   			return true;
> +		}
>   	}
>   
>   	idx = acpi_match_platform_list(plat_info);
> @@ -2606,22 +2615,28 @@ static int __init intel_pstate_init(void)
>   		}
>   	} else {
>   		id = x86_match_cpu(intel_pstate_cpu_ids);
> -		if (!id)
> +		if (!id) {
> +			pr_info("CPU ID is not in the list of supported devices\n");
>   			return -ENODEV;
> +		}
>   
>   		copy_cpu_funcs((struct pstate_funcs *)id->driver_data);
>   	}
>   
> -	if (intel_pstate_msrs_not_valid())
> +	if (intel_pstate_msrs_not_valid()) {
> +		pr_warn("Cannot enable driver as per invalid MSRs\n");
>   		return -ENODEV;
> +	}
>   
>   hwp_cpu_matched:
>   	/*
>   	 * The Intel pstate driver will be ignored if the platform
>   	 * firmware has its own power management modes.
>   	 */
> -	if (intel_pstate_platform_pwr_mgmt_exists())
> +	if (intel_pstate_platform_pwr_mgmt_exists()) {
> +		pr_info("Platform already taking care of power management\n");
>   		return -ENODEV;
> +	}
>   
>   	if (!hwp_active && hwp_only)
>   		return -ENOTSUPP;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-14  9:28 ` Erwan Velu
@ 2019-02-14 10:22   ` Rafael J. Wysocki
  2019-02-14 15:18     ` Erwan Velu
  2019-02-14 16:42     ` Erwan Velu
  0 siblings, 2 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-02-14 10:22 UTC (permalink / raw)
  To: Erwan Velu
  Cc: rafael, Erwan Velu, Srinivas Pandruvada, Len Brown,
	Rafael J. Wysocki, Viresh Kumar, open list:INTEL PSTATE DRIVER,
	open list

On Thu, Feb 14, 2019 at 10:28 AM Erwan Velu <e.velu@criteo.com> wrote:
>
> Hey rafael,
>
> Does the V6 looks good to you ?

I've queued it up with some manual changes.  No need to update.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-14 10:22   ` Rafael J. Wysocki
@ 2019-02-14 15:18     ` Erwan Velu
  2019-02-14 17:30       ` Rafael J. Wysocki
  2019-02-14 16:42     ` Erwan Velu
  1 sibling, 1 reply; 7+ messages in thread
From: Erwan Velu @ 2019-02-14 15:18 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Erwan Velu, Srinivas Pandruvada, Len Brown, Rafael J. Wysocki,
	Viresh Kumar, open list:INTEL PSTATE DRIVER, open list


Le 14/02/2019 à 11:22, Rafael J. Wysocki a écrit :
> On Thu, Feb 14, 2019 at 10:28 AM Erwan Velu <e.velu@criteo.com> wrote:
>> Hey rafael,
>>
>> Does the V6 looks good to you ?
> I've queued it up with some manual changes.  No need to update.

Ok, cool. What kind of changes ?

I'd love to understand what I missed.

Thx,

Erwan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-14 10:22   ` Rafael J. Wysocki
  2019-02-14 15:18     ` Erwan Velu
@ 2019-02-14 16:42     ` Erwan Velu
  2019-02-14 17:31       ` Rafael J. Wysocki
  1 sibling, 1 reply; 7+ messages in thread
From: Erwan Velu @ 2019-02-14 16:42 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Erwan Velu, Srinivas Pandruvada, Len Brown, Rafael J. Wysocki,
	Viresh Kumar, open list:INTEL PSTATE DRIVER, open list


Le 14/02/2019 à 11:22, Rafael J. Wysocki a écrit :
> I've queued it up with some manual changes.  No need to update.

As this could help troubleshooting platforms and as some commits like 
the PSS one on Proliant servers were backported to stable, I would 
surely make sense to backport it to the other stable.

I know this is only a trivial print patch but as my initial issue 
started with some of the 4.14 serie, it could be nice to have it too.

Erwan,


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-14 15:18     ` Erwan Velu
@ 2019-02-14 17:30       ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-02-14 17:30 UTC (permalink / raw)
  To: Erwan Velu
  Cc: Rafael J. Wysocki, Erwan Velu, Srinivas Pandruvada, Len Brown,
	Rafael J. Wysocki, Viresh Kumar, open list:INTEL PSTATE DRIVER,
	open list

On Thu, Feb 14, 2019 at 4:18 PM Erwan Velu <e.velu@criteo.com> wrote:
>
>
> Le 14/02/2019 à 11:22, Rafael J. Wysocki a écrit :
> > On Thu, Feb 14, 2019 at 10:28 AM Erwan Velu <e.velu@criteo.com> wrote:
> >> Hey rafael,
> >>
> >> Does the V6 looks good to you ?
> > I've queued it up with some manual changes.  No need to update.
>
> Ok, cool. What kind of changes ?

Nothing fundamental:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=076b862c7e4409d2dcacfda19f7eaf8d07ab9200

> I'd love to understand what I missed.

It's more about my preferences.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit
  2019-02-14 16:42     ` Erwan Velu
@ 2019-02-14 17:31       ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-02-14 17:31 UTC (permalink / raw)
  To: Erwan Velu
  Cc: Rafael J. Wysocki, Erwan Velu, Srinivas Pandruvada, Len Brown,
	Rafael J. Wysocki, Viresh Kumar, open list:INTEL PSTATE DRIVER,
	open list

On Thu, Feb 14, 2019 at 5:42 PM Erwan Velu <e.velu@criteo.com> wrote:
>
>
> Le 14/02/2019 à 11:22, Rafael J. Wysocki a écrit :
> > I've queued it up with some manual changes.  No need to update.
>
> As this could help troubleshooting platforms and as some commits like
> the PSS one on Proliant servers were backported to stable, I would
> surely make sense to backport it to the other stable.
>
> I know this is only a trivial print patch but as my initial issue
> started with some of the 4.14 serie, it could be nice to have it too.

You can always request -stable to pick up a patch once it's got to the mainline.

I'm not going to mark it as -stable material, though, as it is not a
fix strictly speaking.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-14 17:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-13 12:21 [PATCH v6] cpufreq: intel_pstate: Reporting reasons why driver prematurely exit Erwan Velu
2019-02-14  9:28 ` Erwan Velu
2019-02-14 10:22   ` Rafael J. Wysocki
2019-02-14 15:18     ` Erwan Velu
2019-02-14 17:30       ` Rafael J. Wysocki
2019-02-14 16:42     ` Erwan Velu
2019-02-14 17:31       ` Rafael J. Wysocki

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).