Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
From: Randy Dunlap @ 2024-04-07 15:32 UTC (permalink / raw)
  To: xiongxin, mario.limonciello, Rafael Wysocki, hdegoede,
	linus.walleij
  Cc: Basavaraj.Natikar, Shyam-sundar.S-k, linux-pm, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio
In-Reply-To: <20230602073025.22884-1-mario.limonciello@amd.com>



On 4/7/24 5:49 AM, xiongxin wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> All uses in the kernel are currently already oriented around
> suspend/resume. As some other parts of the kernel may also use these
> messages in functions that could also be used outside of
> suspend/resume, only enable in suspend/resume path.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v3->v4:
>  * add back do/while as it wasn't pointless.  It fixes a warning.
> ---
>  include/linux/suspend.h | 8 +++++---
>  kernel/power/main.c     | 6 ++++++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 1a0426e6761c..74f406c53ac0 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
>  #ifdef CONFIG_PM_SLEEP_DEBUG
>  extern bool pm_print_times_enabled;
>  extern bool pm_debug_messages_on;
> +extern bool pm_debug_messages_should_print(void);
>  static inline int pm_dyn_debug_messages_on(void)
>  {
>  #ifdef CONFIG_DYNAMIC_DEBUG
> @@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
>  #endif
>  #define __pm_pr_dbg(fmt, ...)					\
>  	do {							\
> -		if (pm_debug_messages_on)			\
> +		if (pm_debug_messages_should_print())		\
>  			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
>  		else if (pm_dyn_debug_messages_on())		\
>  			pr_debug(fmt, ##__VA_ARGS__);	\
>  	} while (0)
>  #define __pm_deferred_pr_dbg(fmt, ...)				\
>  	do {							\
> -		if (pm_debug_messages_on)			\
> +		if (pm_debug_messages_should_print())		\
>  			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
>  	} while (0)
>  #else
> @@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
>  /**
>   * pm_pr_dbg - print pm sleep debug messages
>   *
> - * If pm_debug_messages_on is enabled, print message.
> + * If pm_debug_messages_on is enabled and the system is entering/leaving
> + *      suspend, print message.
>   * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
>   *	print message only from instances explicitly enabled on dynamic debug's
>   *	control.
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 3113ec2f1db4..daa535012e51 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
>  
>  bool pm_debug_messages_on __read_mostly;
>  
> +bool pm_debug_messages_should_print(void)
> +{
> +	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;
> 
>> hibernate processes also mostly use the pm_pr_dbg() function, which
>> results in hibernate processes only being able to output such logs
>> through dynamic debug, which is unfriendly to kernels without
>> CONFIG_DYNAMIC_DEBUG configuration.

This part of the patch doesn't look so good. ^^^^^^^^^^^^^^^^^^^^

> 
> +}
> +EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
> +
>  static ssize_t pm_debug_messages_show(struct kobject *kobj,
>  				      struct kobj_attribute *attr, char *buf)
>  {
> 

-- 
#Randy

^ permalink raw reply

* Re: [PATCH] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Thomas Gleixner @ 2024-04-07 13:49 UTC (permalink / raw)
  To: Anna-Maria Behnsen, linux-kernel
  Cc: Rafael J . Wysocki, Pavel Machek, Len Brown, Ulf Hansson,
	linux-pm, Frederic Weisbecker, x86, Anna-Maria Behnsen,
	Mario Limonciello, stable
In-Reply-To: <87jzlb8zov.ffs@tglx>

On Fri, Apr 05 2024 at 19:52, Thomas Gleixner wrote:
> On Fri, Apr 05 2024 at 10:34, Anna-Maria Behnsen wrote:
>> queued on those CPUs and should expire directly after resume, are
>> handled. Also timer list timers which are remotely queued to one of those
>> CPUs after resume will not result in a reporgramming IPI as the tick is
>
> s/reporgramming/reprogamming/

Haha. I can't spell either. reprogramming obviously.

^ permalink raw reply

* [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
From: xiongxin @ 2024-04-07 12:49 UTC (permalink / raw)
  To: mario.limonciello, Rafael Wysocki, hdegoede, linus.walleij
  Cc: Basavaraj.Natikar, Shyam-sundar.S-k, linux-pm, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio
In-Reply-To: <20230602073025.22884-1-mario.limonciello@amd.com>

From: Mario Limonciello <mario.limonciello@amd.com>

All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3->v4:
 * add back do/while as it wasn't pointless.  It fixes a warning.
---
 include/linux/suspend.h | 8 +++++---
 kernel/power/main.c     | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..74f406c53ac0 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
 static inline int pm_dyn_debug_messages_on(void)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
@@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
 #endif
 #define __pm_pr_dbg(fmt, ...)					\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 		else if (pm_dyn_debug_messages_on())		\
 			pr_debug(fmt, ##__VA_ARGS__);	\
 	} while (0)
 #define __pm_deferred_pr_dbg(fmt, ...)				\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 	} while (0)
 #else
@@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
 /**
  * pm_pr_dbg - print pm sleep debug messages
  *
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ *      suspend, print message.
  * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
  *	print message only from instances explicitly enabled on dynamic debug's
  *	control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
 
 bool pm_debug_messages_on __read_mostly;
 
+bool pm_debug_messages_should_print(void)
+{
+	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;

> hibernate processes also mostly use the pm_pr_dbg() function, which
> results in hibernate processes only being able to output such logs
> through dynamic debug, which is unfriendly to kernels without
> CONFIG_DYNAMIC_DEBUG configuration.

+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
 static ssize_t pm_debug_messages_show(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *buf)
 {

-- 
2.34.1

^ permalink raw reply related

* [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
From: xiongxin @ 2024-04-07 12:39 UTC (permalink / raw)
  To: xiongxin, Rafael Wysocki, hdegoede, linus.walleij
  Cc: Mario Limonciello, linux-acpi, linux-kernel, platform-driver-x86,
	linux-gpio, linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k
In-Reply-To: <20230602073025.22884-1-mario.limonciello@amd.com>

From: Mario Limonciello <mario.limonciello@amd.com>

All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3->v4:
 * add back do/while as it wasn't pointless.  It fixes a warning.
---
 include/linux/suspend.h | 8 +++++---
 kernel/power/main.c     | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..74f406c53ac0 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
 static inline int pm_dyn_debug_messages_on(void)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
@@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
 #endif
 #define __pm_pr_dbg(fmt, ...)					\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 		else if (pm_dyn_debug_messages_on())		\
 			pr_debug(fmt, ##__VA_ARGS__);	\
 	} while (0)
 #define __pm_deferred_pr_dbg(fmt, ...)				\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 	} while (0)
 #else
@@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
 /**
  * pm_pr_dbg - print pm sleep debug messages
  *
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ *      suspend, print message.
  * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
  *	print message only from instances explicitly enabled on dynamic debug's
  *	control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
 
 bool pm_debug_messages_on __read_mostly;
 
+bool pm_debug_messages_should_print(void)
+{
+	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;

> hibernate processes also mostly use the pm_pr_dbg() function, which
> results in hibernate processes only being able to output such logs
> through dynamic debug, which is unfriendly to kernels without
> CONFIG_DYNAMIC_DEBUG configuration.

+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
 static ssize_t pm_debug_messages_show(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *buf)
 {

-- 
2.34.1

^ permalink raw reply related

* [Bug 218689] AMD_Pstate_EPP Ryzen 7000 issues. Freezing and static sound
From: bugzilla-daemon @ 2024-04-07 11:21 UTC (permalink / raw)
  To: linux-pm
In-Reply-To: <bug-218689-137361@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=218689

--- Comment #1 from Artem S. Tashkinov (aros@gmx.com) ---
This is extremely unlikely to be caused by the CPU driver.

Please try resetting your BIOS settings and not using any XMP memory profiles
first.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [RFC PATCH 3/3] cpufreq: Add Rust based cpufreq-dt driver
From: Benno Lossin @ 2024-04-07 10:17 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, Miguel Ojeda, Miguel Ojeda,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl
  Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon,
	rust-for-linux, Manos Pitsidianakis, Erik Schilling,
	Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <4ff5f30b-f2b8-4625-b3cd-ac08e4ffb068@proton.me>

On 07.04.24 11:54, Benno Lossin wrote:
> On 05.04.24 13:09, Viresh Kumar wrote:
>> +// Finds exact supply name from the OF node.
>> +fn find_supply_name_exact(np: *mut bindings::device_node, name: &str) -> Option<CString> {
>> +    let sname = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
>> +
>> +    // SAFETY: The OF node is guaranteed by the C code to be valid.
>> +    let pp = unsafe { bindings::of_find_property(np, sname.as_ptr() as *mut _, ptr::null_mut()) };
> 
> Drivers should avoid calling `unsafe` code as much as possible. They
> also should not be calling `bindings` code directly. Please write (or
> find) abstractions for these `unsafe` calls.

Having re-read the cover letter, I see that you are already aware of
this. If you need any help with creating the abstractions, feel free to
reach out!

-- 
Cheers,
Benno


^ permalink raw reply

* Re: [RFC PATCH 3/3] cpufreq: Add Rust based cpufreq-dt driver
From: Benno Lossin @ 2024-04-07  9:54 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, Miguel Ojeda, Miguel Ojeda,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl
  Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon,
	rust-for-linux, Manos Pitsidianakis, Erik Schilling,
	Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <1792467a772b7a8355c6d0cb0cbacfbffff08afd.1712314032.git.viresh.kumar@linaro.org>

On 05.04.24 13:09, Viresh Kumar wrote:
> +// Finds exact supply name from the OF node.
> +fn find_supply_name_exact(np: *mut bindings::device_node, name: &str) -> Option<CString> {
> +    let sname = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
> +
> +    // SAFETY: The OF node is guaranteed by the C code to be valid.
> +    let pp = unsafe { bindings::of_find_property(np, sname.as_ptr() as *mut _, ptr::null_mut()) };

Drivers should avoid calling `unsafe` code as much as possible. They
also should not be calling `bindings` code directly. Please write (or
find) abstractions for these `unsafe` calls.

-- 
Cheers,
Benno

> +    if pp.is_null() {
> +        None
> +    } else {
> +        CString::try_from_fmt(fmt!("{}", name)).ok()
> +    }
> +}


^ permalink raw reply

* Re: [RFC PATCH 1/3] rust: Add bindings for OPP framework
From: Benno Lossin @ 2024-04-07  9:54 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, Miguel Ojeda, Miguel Ojeda,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl
  Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon,
	rust-for-linux, Manos Pitsidianakis, Erik Schilling,
	Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <06bb914eae00671a69b393bf86bb01ddec86c16f.1712314032.git.viresh.kumar@linaro.org>

Hi,

I took a quick look and left some comments from the Rust side of view.

On 05.04.24 13:09, Viresh Kumar wrote:
> +/// Equivalent to `struct dev_pm_opp_config` in the C Code.
> +pub struct Config<T: ConfigOps> {
> +    token: Option<i32>,
> +    clk_names: Option<Pin<Vec<CString>>>,

Why are you using `Pin<Vec<_>>`? The vector may reallocate the backing
storage at any point in time.

> +    prop_name: Option<Pin<CString>>,
> +    regulator_names: Option<Pin<Vec<CString>>>,
> +    genpd_names: Option<Pin<Vec<CString>>>,
> +    supported_hw: Option<Pin<Vec<u32>>>,
> +    required_devs: Option<Pin<Vec<Device>>>,
> +    _data: PhantomData<T>,
> +}

[...]

> +    /// Sets the configuration with the OPP core.
> +    pub fn set(&mut self, dev: &Device) -> Result<()> {
> +        // Already configured.
> +        if self.token.is_some() {

Why does the config hold onto this token? Would it make sense to consume
the config and return a `Handle` or `Token` abstraction? Then you don't
need to check if the config has been "used" before.

> +            return Err(EBUSY);
> +        }
> +
> +        let (_clk_list, clk_names) = match &self.clk_names {
> +            Some(x) => {
> +                let list = to_c_str_array(x)?;
> +                let ptr = list.as_ptr();
> +                (Some(list), ptr)
> +            }
> +            None => (None, ptr::null()),
> +        };

[...]

> +/// Operating performance point (OPP).
> +///
> +/// # Invariants
> +///
> +/// `ptr` is valid, non-null, and has a non-zero reference count. One of the references is owned by
> +/// `self`, and will be decremented when `self` is dropped.
> +#[repr(transparent)]
> +pub struct OPP(*mut bindings::dev_pm_opp);

I think you should use the `ARef` pattern instead:

     #[repr(transparent)]
     pub struct OPP(Opaque<bindings::dev_pm_opp>);

     unsafe impl AlwaysRefCounted for OPP {
         // ...
     }

Then you can use `ARef<OPP>` everywhere you use `OPP` currently.

-- 
Cheers,
Benno


^ permalink raw reply

* Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to read registers
From: Zhang, Rui @ 2024-04-07  8:39 UTC (permalink / raw)
  To: linux@roeck-us.net, ricardo.neri-calderon@linux.intel.com
  Cc: Neri, Ricardo, linux-hwmon@vger.kernel.org, lukasz.luba@arm.com,
	daniel.lezcano@linaro.org, jdelvare@suse.com,
	srinivas.pandruvada@linux.intel.com, linux-kernel@vger.kernel.org,
	Wysocki, Rafael J, linux-pm@vger.kernel.org
In-Reply-To: <f4d18a63-c348-4882-897b-dc636feb149b@roeck-us.net>

On Sat, 2024-04-06 at 06:17 -0700, Guenter Roeck wrote:
> On Fri, Apr 05, 2024 at 06:04:16PM -0700, Ricardo Neri wrote:
> > The Intel Software Development manual defines states the
> > temperature
> > digital readout as the bits [22:16] of the
> > IA32_[PACKAGE]_THERM_STATUS
> > registers. In recent processor, however, the range is [23:16]. Use
> > a
> > model-specific bitmask to extract the temperature readout
> > correctly.
> > 
> > Instead of re-implementing model checks, extract the correct
> > bitmask
> > using the intel_tcc library. Add an 'imply' weak reverse dependency
> > on
> > CONFIG_INTEL_TCC. This captures the dependency and lets user to
> > unselect
> > them if they are so inclined. In such case, the bitmask used for
> > the
> > digital readout is [22:16] as specified in the Intel Software
> > Developer's
> > manual.
> > 
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Lukasz Luba <lukasz.luba@arm.com>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: linux-hwmon@vger.kernel.org
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: stable@vger.kernel.org # v6.7+
> > ---
> >  drivers/hwmon/Kconfig    | 1 +
> >  drivers/hwmon/coretemp.c | 6 +++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 83945397b6eb..11d72b3009bf 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -847,6 +847,7 @@ config SENSORS_I5500
> >  config SENSORS_CORETEMP
> >         tristate "Intel Core/Core2/Atom temperature sensor"
> >         depends on X86
> > +       imply INTEL_TCC
> 
> I do not think it is appropriate to make a hardware monitoring driver
> depend on the thermal subsystem.
> 
> NAK in the current form.
> 
Hi, Guenter,

Thanks for reviewing.

We've seen a couple of hwmon drivers depends on or imply THERMAL.
That's why we think this is an applicable solution.
Using the intel_tcc APIs can effectively reduce the future maintenance
burden because we don't need to duplicate the model list in multiple
places.

or do you have any suggestions?

thanks,
rui

^ permalink raw reply

* Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to read registers
From: Zhang, Rui @ 2024-04-07  8:24 UTC (permalink / raw)
  To: linux@roeck-us.net, ricardo.neri-calderon@linux.intel.com,
	Wysocki, Rafael J, jdelvare@suse.com
  Cc: srinivas.pandruvada@linux.intel.com, lukasz.luba@arm.com,
	linux-pm@vger.kernel.org, linux-hwmon@vger.kernel.org,
	daniel.lezcano@linaro.org, linux-kernel@vger.kernel.org,
	Neri, Ricardo
In-Reply-To: <20240406010416.4821-4-ricardo.neri-calderon@linux.intel.com>

On Fri, 2024-04-05 at 18:04 -0700, Ricardo Neri wrote:
> The Intel Software Development manual defines states the temperature

I failed to parse this, is the above "states" redundant?

[...]

> digital readout as the bits [22:16] of the
> IA32_[PACKAGE]_THERM_STATUS
> registers. In recent processor, however, the range is [23:16]. Use a
> model-specific bitmask to extract the temperature readout correctly.
> 
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index 616bd1a5b864..5632e1b1dfb1 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -17,6 +17,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/err.h>
> +#include <linux/intel_tcc.h>
>  #include <linux/mutex.h>
>  #include <linux/list.h>
>  #include <linux/platform_device.h>
> @@ -404,6 +405,8 @@ static ssize_t show_temp(struct device *dev,
>         tjmax = get_tjmax(tdata, dev);
>         /* Check whether the time interval has elapsed */
>         if (time_after(jiffies, tdata->last_updated + HZ)) {
> +               u32 mask =
> intel_tcc_get_temp_mask(is_pkg_temp_data(tdata));
> +
>                 rdmsr_on_cpu(tdata->cpu, tdata->status_reg, &eax,
> &edx);
>                 /*
>                  * Ignore the valid bit. In all observed cases the
> register
> @@ -411,7 +414,7 @@ static ssize_t show_temp(struct device *dev,
>                  * Return it instead of reporting an error which
> doesn't
>                  * really help at all.
>                  */
> -               tdata->temp = tjmax - ((eax >> 16) & 0x7f) * 1000;
> +               tdata->temp = tjmax - ((eax >> 16) & mask) * 1000;
>                 tdata->last_updated = jiffies;
>         }
> 
Besides this one, we can also convert to use intel_tcc_get_tjmax() in
get_tjmax().

thanks,
rui


^ permalink raw reply

* Re: [PATCH 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers
From: Zhang, Rui @ 2024-04-07  8:13 UTC (permalink / raw)
  To: linux@roeck-us.net, ricardo.neri-calderon@linux.intel.com,
	Wysocki, Rafael J, jdelvare@suse.com
  Cc: srinivas.pandruvada@linux.intel.com, lukasz.luba@arm.com,
	linux-pm@vger.kernel.org, linux-hwmon@vger.kernel.org,
	daniel.lezcano@linaro.org, linux-kernel@vger.kernel.org,
	Neri, Ricardo
In-Reply-To: <20240406010416.4821-2-ricardo.neri-calderon@linux.intel.com>

> +
> +#define TCC_FAM6_MODEL_TEMP_MASKS

Future non FAM6 processors can still use this macro, right?
So I'd prefer to remove FAM6_MODEL in the macro name.

[...]
> 
> +
> +/**
> + * get_tcc_offset_mask() - Returns the model-specific bitmask for
> TCC offset
> + *
> + * Get the model-specific bitmask to extract TCC_OFFSET from the
> MSR_TEMPERATURE_
> + * TARGET register. If the mask is 0, it means the processor does
> not support TCC offset.
> + *
> + * Return: The model-specific bitmask for TCC offset.
> + */
> +u32 get_tcc_offset_mask(void)
> +{
> +       return intel_tcc_temp_masks.tcc_offset;
> +}
> +EXPORT_SYMBOL_NS(get_tcc_offset_mask, INTEL_TCC);

the name is not consistent with the other intel_tcc APIs.

how about intel_tcc_get_offset_mask()?

[...]

> diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
> index 8ff8eabb4a98..e281cf06aeab 100644
> --- a/include/linux/intel_tcc.h
> +++ b/include/linux/intel_tcc.h
> @@ -14,5 +14,13 @@ int intel_tcc_get_tjmax(int cpu);
>  int intel_tcc_get_offset(int cpu);
>  int intel_tcc_set_offset(int cpu, int offset);
>  int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
> +#ifdef CONFIG_INTEL_TCC
> +u32 get_tcc_offset_mask(void);
> +u32 intel_tcc_get_temp_mask(bool pkg);
> +#else
> +static inline u32 get_tcc_offset_mask(void) { return 0; }
> +/* Use the architectural bitmask of the temperature readout. No
> model checks. */
> +static inline u32 intel_tcc_get_temp_mask(bool pkg) { return 0x7f; }
> +#endif

for intel_tcc_get_temp_mask()
   1. with CONFIG_INTEL_TCC
      a) for a platform in the model list, return the hardcoded value
      b) for a platform not in the model list, return 0xff
   2. without CONFIG_INTEL_TCC, return 0x7f

This is a bit confusing. IMO, at least we should leave a comment about
this difference.

thanks,
rui

^ permalink raw reply

* [Bug 218689] AMD_Pstate_EPP Ryzen 7000 issues. Freezing and static sound
From: bugzilla-daemon @ 2024-04-07  0:32 UTC (permalink / raw)
  To: linux-pm
In-Reply-To: <bug-218689-137361@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=218689

Christopher Bii (kbii.chris@gmail.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|AMD_Pstate_EPP Ryzen 7000   |AMD_Pstate_EPP Ryzen 7000
                   |issues. Freezing and        |issues. Freezing and static
                   |buzzing                     |sound

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* [Bug 218689] New: AMD_Pstate_EPP Ryzen 7000 issues. Freezing and buzzing
From: bugzilla-daemon @ 2024-04-06 20:49 UTC (permalink / raw)
  To: linux-pm

https://bugzilla.kernel.org/show_bug.cgi?id=218689

            Bug ID: 218689
           Summary: AMD_Pstate_EPP Ryzen 7000 issues. Freezing and buzzing
           Product: Power Management
           Version: 2.5
          Hardware: AMD
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: cpufreq
          Assignee: linux-pm@vger.kernel.org
          Reporter: kbii.chris@gmail.com
        Regression: No

Hardware:

AMD Ryzen 9 7900x
Gigabyte b650 aero g
Corsair Dominator Platinum 2x16 6000MHz

Kernel tested on:

6.8.2 Arch
6.8.1 Arch
6.6.9 Debian

Issue:

There has been a static noise coming from my cpu as well as frequent freezes.
This is nearly constant and makes the computer impossible to use for long due
to the sound.

Remedy:

I have found the issue to disappear completely while using the passive driver
provided by amd_pstate. The scaling driver must be set to schedutil. This
completely mitigates the issue.
kernel flags: amd_pstate=passive cpufreq.default_governor=schedutil

Suspicions:

I suspect the power management implementation used by amd_pstate_epp to be
causing this issue. There is a async param found inside
/sys/devices/system/cpu/power that is always set to false whenever
amd_pstate_epp is used. My intuition tells me this must be what holds up the
system at times while changing c_states or something.

Extra:

I have seen alot of talk surrounding the fTPM and issues around it but I cannot
related these two observations together, hence why I am opening a new issue. If
this is not the right way to go about it mybad, this is my first bug report.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* [PATCH] power: supply: cros_usbpd: Don't show messages for no charging ports found
From: Mario Limonciello @ 2024-04-06 19:17 UTC (permalink / raw)
  To: Benson Leung, Guenter Roeck
  Cc: Sebastian Reichel, open list:CHROMEOS EC SUBDRIVERS,
	open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS, open list,
	Dustin L . Howett, Mario Limonciello

From: Mario Limonciello <mario.limonciello@amd.com>

Framework 13 and 16 don't use cros_usbpd but do use cros_ec. The following
sequence of messages is totally unnecessary.

[    5.545352] cros-usbpd-charger cros-usbpd-charger.4.auto: No USB PD
charging ports found
[    5.546722] cros-usbpd-charger cros-usbpd-charger.4.auto: Unexpected
number of charge port count
[    5.546730] cros-usbpd-charger cros-usbpd-charger.4.auto: Failing
probe (err:0xffffffb9)
[    5.546735] cros-usbpd-charger cros-usbpd-charger.4.auto: probe with
driver cros-usbpd-charger failed with error -71

Link: https://lore.kernel.org/chrome-platform/20240403004713.130365-1-dustin@howett.net/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/power/supply/cros_usbpd-charger.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index b6c96376776a..4e69da2cce7b 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -570,7 +570,8 @@ static int cros_usbpd_charger_probe(struct platform_device *pd)
 		 * This can happen on a system that doesn't support USB PD.
 		 * Log a message, but no need to warn.
 		 */
-		dev_info(dev, "No USB PD charging ports found\n");
+		dev_dbg(dev, "No USB PD charging ports found\n");
+		return -ENODEV;
 	}
 
 	charger->num_charger_ports = cros_usbpd_charger_get_num_ports(charger);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v4 7/8] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
From: Okanovic, Haris @ 2024-04-06 18:42 UTC (permalink / raw)
  To: ankur.a.arora@oracle.com
  Cc: joao.m.martins@oracle.com, kvm@vger.kernel.org,
	dianders@chromium.org, linux-arm-kernel@lists.infradead.org,
	pmladek@suse.com, wanpengli@tencent.com,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	catalin.marinas@arm.com, mingo@redhat.com, pbonzini@redhat.com,
	tglx@linutronix.de, daniel.lezcano@linaro.org,
	mihai.carabas@oracle.com, arnd@arndb.de, will@kernel.org,
	hpa@zytor.com, peterz@infradead.org, mic@digikod.net,
	vkuznets@redhat.com, bp@alien8.de, npiggin@gmail.com,
	linux-pm@vger.kernel.org, rafael@kernel.org,
	juerg.haefliger@canonical.com, x86@kernel.org,
	rick.p.edgecombe@intel.com
In-Reply-To: <87r0fjtn9y.fsf@oracle.com>

On Fri, 2024-04-05 at 16:14 -0700, Ankur Arora wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Okanovic, Haris <harisokn@amazon.com> writes:
> 
> > On Thu, 2024-02-15 at 09:41 +0200, Mihai Carabas wrote:
> > > cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> > > smp_cond_load_relaxed which basically does a "wfe".
> > > 
> > > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> > > ---
> > >  drivers/cpuidle/poll_state.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> > > index 9b6d90a72601..1e45be906e72 100644
> > > --- a/drivers/cpuidle/poll_state.c
> > > +++ b/drivers/cpuidle/poll_state.c
> > > @@ -13,6 +13,7 @@
> > >  static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > >                             struct cpuidle_driver *drv, int index)
> > >  {
> > > +    unsigned long ret;
> > >      u64 time_start;
> > > 
> > >      time_start = local_clock_noinstr();
> > > @@ -26,12 +27,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > > 
> > >              limit = cpuidle_poll_time(drv, dev);
> > > 
> > > -            while (!need_resched()) {
> > > -                    cpu_relax();
> > > -                    if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> > > -                            continue;
> > > -
> > > +            for (;;) {
> > >                      loop_count = 0;
> > > +
> > > +                    ret = smp_cond_load_relaxed(&current_thread_info()->flags,
> > > +                                                VAL & _TIF_NEED_RESCHED ||
> > > +                                                loop_count++ >= POLL_IDLE_RELAX_COUNT);
> > 
> > Is it necessary to repeat this 200 times with a wfe poll?
> 
> The POLL_IDLE_RELAX_COUNT is there because on x86 each cpu_relax()
> iteration is much shorter.
> 
> With WFE, it makes less sense.
> 
> > Does kvm not implement a timeout period?
> 
> Not yet, but it does become more useful after a WFE haltpoll is
> available on ARM64.

Note that kvm conditionally traps WFE and WFI based on number of host
CPU tasks. VMs will sometimes see hardware behavior - potentially
polling for a long time before entering WFI.

https://elixir.bootlin.com/linux/latest/source/arch/arm64/kvm/arm.c#L459

> 
> Haltpoll does have a timeout, which you should be able to tune via
> /sys/module/haltpoll/parameters/ but that, of course, won't help here.
> 
> > Could you make it configurable? This patch improves certain workloads
> > on AWS Graviton instances as well, but blocks up to 6ms in 200 * 30us
> > increments before going to wfi, which is a bit excessive.
> 
> Yeah, this looks like a problem. We could solve it by making it an
> architectural parameter. Though I worry about ARM platforms with
> much smaller default timeouts.
> The other possibility is using WFET in the primitive, but then we
> have that dependency and that's a bigger change.

See arm64's delay() for inspiration:

https://elixir.bootlin.com/linux/v6.9-rc2/source/arch/arm64/lib/delay.c#L26

> 
> Will address this in the next version.
> 
> Thanks for pointing this out.
> 
> --
> ankur


^ permalink raw reply

* Re: [PATCH v2 0/3] Add support for the IPQ5321 SoC
From: Kathiravan Thirumoorthy @ 2024-04-06 14:48 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Ilia Lin, Rafael J. Wysocki, Viresh Kumar
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-pm,
	Krzysztof Kozlowski, Mukesh Ojha
In-Reply-To: <20240325-ipq5321-sku-support-v2-0-f30ce244732f@quicinc.com>



On 3/25/2024 9:19 PM, Kathiravan Thirumoorthy wrote:
> IPQ5321 SoC belong to IPQ5332 family. Add the SoC ID and the cpufreq
> support. Maximum cpufreq for IPQ5321 is 1.1GHZ, which is determined
> based on the eFuse.
> 
> Viresh is okay to merge the cpufreq change via qcom tree[1] and provided
> his Ack.
> 
> [1]
> https://lore.kernel.org/linux-arm-msm/20240306053200.6iwrviltwt3pnfnt@vireshk-i7/


Gentle Reminder...

> 
> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> ---
> Changes in v2:
> 	- rebase on next-20240325
> 	- pick up the tags
> 	- Link to v1:
> 	  https://lore.kernel.org/linux-arm-msm/20240228-ipq5321-sku-support-v1-0-14e4d4715f4b@quicinc.com/
> 
> ---
> Kathiravan Thirumoorthy (3):
>        dt-bindings: arm: qcom,ids: Add SoC ID for IPQ5321
>        soc: qcom: socinfo: Add SoC ID for IPQ5321
>        cpufreq: qcom-nvmem: add support for IPQ5321
> 
>   drivers/cpufreq/qcom-cpufreq-nvmem.c | 1 +
>   drivers/soc/qcom/socinfo.c           | 1 +
>   include/dt-bindings/arm/qcom,ids.h   | 1 +
>   3 files changed, 3 insertions(+)
> ---
> base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
> change-id: 20240228-ipq5321-sku-support-bd07056d5e01
> 
> Best regards,

^ permalink raw reply

* [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper
From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel
  Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare,
	Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev,
	Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules
In-Reply-To: <cover.1712410202.git.lukas@wunner.de>

Deduplicate ->read() callbacks of bin_attributes which are backed by a
simple buffer in memory:

Use the newly introduced sysfs_bin_attr_simple_read() helper instead,
either by referencing it directly or by declaring such bin_attributes
with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO().

Aside from a reduction of LoC, this shaves off a few bytes from vmlinux
(304 bytes on an x86_64 allyesconfig).

No functional change intended.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 arch/powerpc/platforms/powernv/opal.c              | 10 +--------
 drivers/acpi/bgrt.c                                |  9 +-------
 drivers/firmware/dmi_scan.c                        | 12 ++--------
 drivers/firmware/efi/rci2-table.c                  | 10 +--------
 drivers/gpu/drm/i915/gvt/firmware.c                | 26 +++++-----------------
 .../intel/int340x_thermal/int3400_thermal.c        |  9 +-------
 init/initramfs.c                                   | 10 +--------
 kernel/module/sysfs.c                              | 13 +----------
 8 files changed, 14 insertions(+), 85 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 45dd77e..5d0f35b 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void)
 	return 0;
 }
 
-static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
-				struct bin_attribute *bin_attr, char *buf,
-				loff_t off, size_t count)
-{
-	return memory_read_from_buffer(buf, count, &off, bin_attr->private,
-				       bin_attr->size);
-}
-
 static int opal_add_one_export(struct kobject *parent, const char *export_name,
 			       struct device_node *np, const char *prop_name)
 {
@@ -826,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name,
 	sysfs_bin_attr_init(attr);
 	attr->attr.name = name;
 	attr->attr.mode = 0400;
-	attr->read = export_attr_read;
+	attr->read = sysfs_bin_attr_simple_read;
 	attr->private = __va(vals[0]);
 	attr->size = vals[1];
 
diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c
index e4fb9e2..d1d9c92 100644
--- a/drivers/acpi/bgrt.c
+++ b/drivers/acpi/bgrt.c
@@ -29,14 +29,7 @@
 BGRT_SHOW(xoffset, image_offset_x);
 BGRT_SHOW(yoffset, image_offset_y);
 
-static ssize_t image_read(struct file *file, struct kobject *kobj,
-	       struct bin_attribute *attr, char *buf, loff_t off, size_t count)
-{
-	memcpy(buf, attr->private + off, count);
-	return count;
-}
-
-static BIN_ATTR_RO(image, 0);	/* size gets filled in later */
+static BIN_ATTR_SIMPLE_RO(image);
 
 static struct attribute *bgrt_attributes[] = {
 	&bgrt_attr_version.attr,
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 015c95a..3d0f773 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -746,16 +746,8 @@ static void __init dmi_scan_machine(void)
 	pr_info("DMI not present or invalid.\n");
 }
 
-static ssize_t raw_table_read(struct file *file, struct kobject *kobj,
-			      struct bin_attribute *attr, char *buf,
-			      loff_t pos, size_t count)
-{
-	memcpy(buf, attr->private + pos, count);
-	return count;
-}
-
-static BIN_ATTR(smbios_entry_point, S_IRUSR, raw_table_read, NULL, 0);
-static BIN_ATTR(DMI, S_IRUSR, raw_table_read, NULL, 0);
+static BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point);
+static BIN_ATTR_SIMPLE_ADMIN_RO(DMI);
 
 static int __init dmi_init(void)
 {
diff --git a/drivers/firmware/efi/rci2-table.c b/drivers/firmware/efi/rci2-table.c
index de1a9a1..4fd45d6 100644
--- a/drivers/firmware/efi/rci2-table.c
+++ b/drivers/firmware/efi/rci2-table.c
@@ -40,15 +40,7 @@ struct rci2_table_global_hdr {
 static u32 rci2_table_len;
 unsigned long rci2_table_phys __ro_after_init = EFI_INVALID_TABLE_ADDR;
 
-static ssize_t raw_table_read(struct file *file, struct kobject *kobj,
-			      struct bin_attribute *attr, char *buf,
-			      loff_t pos, size_t count)
-{
-	memcpy(buf, attr->private + pos, count);
-	return count;
-}
-
-static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0);
+static BIN_ATTR_SIMPLE_ADMIN_RO(rci2);
 
 static u16 checksum(void)
 {
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
index 4dd52ac..5e66a26 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -50,21 +50,7 @@ struct gvt_firmware_header {
 
 #define dev_to_drm_minor(d) dev_get_drvdata((d))
 
-static ssize_t
-gvt_firmware_read(struct file *filp, struct kobject *kobj,
-	     struct bin_attribute *attr, char *buf,
-	     loff_t offset, size_t count)
-{
-	memcpy(buf, attr->private + offset, count);
-	return count;
-}
-
-static struct bin_attribute firmware_attr = {
-	.attr = {.name = "gvt_firmware", .mode = (S_IRUSR)},
-	.read = gvt_firmware_read,
-	.write = NULL,
-	.mmap = NULL,
-};
+static BIN_ATTR_SIMPLE_ADMIN_RO(gvt_firmware);
 
 static int expose_firmware_sysfs(struct intel_gvt *gvt)
 {
@@ -107,10 +93,10 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
 	crc32_start = offsetof(struct gvt_firmware_header, version);
 	h->crc32 = crc32_le(0, firmware + crc32_start, size - crc32_start);
 
-	firmware_attr.size = size;
-	firmware_attr.private = firmware;
+	bin_attr_gvt_firmware.size = size;
+	bin_attr_gvt_firmware.private = firmware;
 
-	ret = device_create_bin_file(&pdev->dev, &firmware_attr);
+	ret = device_create_bin_file(&pdev->dev, &bin_attr_gvt_firmware);
 	if (ret) {
 		vfree(firmware);
 		return ret;
@@ -122,8 +108,8 @@ static void clean_firmware_sysfs(struct intel_gvt *gvt)
 {
 	struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
 
-	device_remove_bin_file(&pdev->dev, &firmware_attr);
-	vfree(firmware_attr.private);
+	device_remove_bin_file(&pdev->dev, &bin_attr_gvt_firmware);
+	vfree(bin_attr_gvt_firmware.private);
 }
 
 /**
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 427d370..6d4b51a 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -73,14 +73,7 @@ struct odvp_attr {
 	struct device_attribute attr;
 };
 
-static ssize_t data_vault_read(struct file *file, struct kobject *kobj,
-	     struct bin_attribute *attr, char *buf, loff_t off, size_t count)
-{
-	memcpy(buf, attr->private + off, count);
-	return count;
-}
-
-static BIN_ATTR_RO(data_vault, 0);
+static BIN_ATTR_SIMPLE_RO(data_vault);
 
 static struct bin_attribute *data_attributes[] = {
 	&bin_attr_data_vault,
diff --git a/init/initramfs.c b/init/initramfs.c
index da79760..5193fae 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -575,15 +575,7 @@ static int __init initramfs_async_setup(char *str)
 #include <linux/initrd.h>
 #include <linux/kexec.h>
 
-static ssize_t raw_read(struct file *file, struct kobject *kobj,
-			struct bin_attribute *attr, char *buf,
-			loff_t pos, size_t count)
-{
-	memcpy(buf, attr->private + pos, count);
-	return count;
-}
-
-static BIN_ATTR(initrd, 0440, raw_read, NULL, 0);
+static BIN_ATTR(initrd, 0440, sysfs_bin_attr_simple_read, NULL, 0);
 
 void __init reserve_initrd_mem(void)
 {
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index d964167..26efe13 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -146,17 +146,6 @@ struct module_notes_attrs {
 	struct bin_attribute attrs[] __counted_by(notes);
 };
 
-static ssize_t module_notes_read(struct file *filp, struct kobject *kobj,
-				 struct bin_attribute *bin_attr,
-				 char *buf, loff_t pos, size_t count)
-{
-	/*
-	 * The caller checked the pos and count against our size.
-	 */
-	memcpy(buf, bin_attr->private + pos, count);
-	return count;
-}
-
 static void free_notes_attrs(struct module_notes_attrs *notes_attrs,
 			     unsigned int i)
 {
@@ -205,7 +194,7 @@ static void add_notes_attrs(struct module *mod, const struct load_info *info)
 			nattr->attr.mode = 0444;
 			nattr->size = info->sechdrs[i].sh_size;
 			nattr->private = (void *)info->sechdrs[i].sh_addr;
-			nattr->read = module_notes_read;
+			nattr->read = sysfs_bin_attr_simple_read;
 			++nattr;
 		}
 		++loaded;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper
From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel
  Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare,
	Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev,
	Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules
In-Reply-To: <cover.1712410202.git.lukas@wunner.de>

When drivers expose a bin_attribute in sysfs which is backed by a buffer
in memory, a common pattern is to set the @private and @size members in
struct bin_attribute to the buffer's location and size.

The ->read() callback then merely consists of a single memcpy() call.
It's not even necessary to perform bounds checks as these are already
handled by sysfs_kf_bin_read().

However each driver is so far providing its own ->read() implementation.
The pattern is sufficiently frequent to merit a public helper, so add
sysfs_bin_attr_simple_read() as well as BIN_ATTR_SIMPLE_RO() and
BIN_ATTR_SIMPLE_ADMIN_RO() macros to ease declaration of such
bin_attributes and reduce LoC and .text section size.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 fs/sysfs/file.c       | 27 +++++++++++++++++++++++++++
 include/linux/sysfs.h | 15 +++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 6b7652f..289b57d 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -783,3 +783,30 @@ int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
 	return len;
 }
 EXPORT_SYMBOL_GPL(sysfs_emit_at);
+
+/**
+ *	sysfs_bin_attr_simple_read - read callback to simply copy from memory.
+ *	@file:	attribute file which is being read.
+ *	@kobj:	object to which the attribute belongs.
+ *	@attr:	attribute descriptor.
+ *	@buf:	destination buffer.
+ *	@off:	offset in bytes from which to read.
+ *	@count:	maximum number of bytes to read.
+ *
+ * Simple ->read() callback for bin_attributes backed by a buffer in memory.
+ * The @private and @size members in struct bin_attribute must be set to the
+ * buffer's location and size before the bin_attribute is created in sysfs.
+ *
+ * Bounds check for @off and @count is done in sysfs_kf_bin_read().
+ * Negative value check for @off is done in vfs_setpos() and default_llseek().
+ *
+ * Returns number of bytes written to @buf.
+ */
+ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
+				   struct bin_attribute *attr, char *buf,
+				   loff_t off, size_t count)
+{
+	memcpy(buf, attr->private + off, count);
+	return count;
+}
+EXPORT_SYMBOL_GPL(sysfs_bin_attr_simple_read);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 326341c..a7d725f 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -371,6 +371,17 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR_ADMIN_RO(_name, _size)
 #define BIN_ATTR_ADMIN_RW(_name, _size)					\
 struct bin_attribute bin_attr_##_name = __BIN_ATTR_ADMIN_RW(_name, _size)
 
+#define __BIN_ATTR_SIMPLE_RO(_name, _mode) {				\
+	.attr	= { .name = __stringify(_name), .mode = _mode },	\
+	.read	= sysfs_bin_attr_simple_read,				\
+}
+
+#define BIN_ATTR_SIMPLE_RO(_name)					\
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0444)
+
+#define BIN_ATTR_SIMPLE_ADMIN_RO(_name)					\
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0400)
+
 struct sysfs_ops {
 	ssize_t	(*show)(struct kobject *, struct attribute *, char *);
 	ssize_t	(*store)(struct kobject *, struct attribute *, const char *, size_t);
@@ -478,6 +489,10 @@ int sysfs_group_change_owner(struct kobject *kobj,
 __printf(3, 4)
 int sysfs_emit_at(char *buf, int at, const char *fmt, ...);
 
+ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
+				   struct bin_attribute *attr, char *buf,
+				   loff_t off, size_t count);
+
 #else /* CONFIG_SYSFS */
 
 static inline int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
-- 
2.43.0


^ permalink raw reply related

* [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks
From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel
  Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare,
	Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev,
	Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules

For my upcoming PCI device authentication v2 patches, I have the need
to expose a simple buffer in virtual memory as a bin_attribute.

It turns out we've duplicated the ->read() callback for such simple
buffers a fair number of times across the tree.

So instead of reinventing the wheel, I decided to introduce a common
helper and eliminate all duplications I could find.

I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read()
name. ;)

Lukas Wunner (2):
  sysfs: Add sysfs_bin_attr_simple_read() helper
  treewide: Use sysfs_bin_attr_simple_read() helper

 arch/powerpc/platforms/powernv/opal.c              | 10 +-------
 drivers/acpi/bgrt.c                                |  9 +-------
 drivers/firmware/dmi_scan.c                        | 12 ++--------
 drivers/firmware/efi/rci2-table.c                  | 10 +-------
 drivers/gpu/drm/i915/gvt/firmware.c                | 26 +++++----------------
 .../intel/int340x_thermal/int3400_thermal.c        |  9 +-------
 fs/sysfs/file.c                                    | 27 ++++++++++++++++++++++
 include/linux/sysfs.h                              | 15 ++++++++++++
 init/initramfs.c                                   | 10 +-------
 kernel/module/sysfs.c                              | 13 +----------
 10 files changed, 56 insertions(+), 85 deletions(-)

-- 
2.43.0


^ permalink raw reply

* Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to read registers
From: Guenter Roeck @ 2024-04-06 13:17 UTC (permalink / raw)
  To: Ricardo Neri
  Cc: Rafael J. Wysocki, Zhang Rui, Jean Delvare, Srinivas Pandruvada,
	Lukasz Luba, Daniel Lezcano, linux-pm, linux-hwmon, linux-kernel,
	Ricardo Neri
In-Reply-To: <20240406010416.4821-4-ricardo.neri-calderon@linux.intel.com>

On Fri, Apr 05, 2024 at 06:04:16PM -0700, Ricardo Neri wrote:
> The Intel Software Development manual defines states the temperature
> digital readout as the bits [22:16] of the IA32_[PACKAGE]_THERM_STATUS
> registers. In recent processor, however, the range is [23:16]. Use a
> model-specific bitmask to extract the temperature readout correctly.
> 
> Instead of re-implementing model checks, extract the correct bitmask
> using the intel_tcc library. Add an 'imply' weak reverse dependency on
> CONFIG_INTEL_TCC. This captures the dependency and lets user to unselect
> them if they are so inclined. In such case, the bitmask used for the
> digital readout is [22:16] as specified in the Intel Software Developer's
> manual.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lukasz Luba <lukasz.luba@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org # v6.7+
> ---
>  drivers/hwmon/Kconfig    | 1 +
>  drivers/hwmon/coretemp.c | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 83945397b6eb..11d72b3009bf 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -847,6 +847,7 @@ config SENSORS_I5500
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> +	imply INTEL_TCC

I do not think it is appropriate to make a hardware monitoring driver
depend on the thermal subsystem.

NAK in the current form.

Guenter

^ permalink raw reply

* Re: [PATCH 00/34] address all -Wunused-const warnings
From: patchwork-bot+netdevbpf @ 2024-04-06  5:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, arnd, mpe, christophe.leroy, dlemoal, jikos, gregkh,
	minyard, peterhuewe, jarkko, kristo, sboyd, abbotti, hsweeten,
	srinivas.pandruvada, lenb, rafael, john.allen, herbert, vkoul,
	ardb, andersson, mdf, liviu.dudau, benjamin.tissoires, andi.shyti,
	michael.hennerich, peda, lars, jic23, dmitry.torokhov,
	markuss.broks, alexandre.torgue, lee, kuba, Shyam-sundar.S-k,
	iyappan, yisen.zhuang, stf_xl, kvalo, sre, tony, broonie,
	alexandre.belloni, chenxiang66, martin.petersen, neil.armstrong,
	heiko, krzysztof.kozlowski, hvaibhav.linux, elder, jirislaby,
	ychuang3, deller, hch, robin.murphy, rostedt, mhiramat, akpm,
	keescook, trond.myklebust, anna, masahiroy, nathan, tiwai,
	linuxppc-dev, linux-ide, openipmi-developer, linux-integrity,
	linux-omap, linux-clk, linux-pm, linux-crypto, dmaengine,
	linux-efi, linux-arm-msm, linux-fpga, dri-devel, linux-input,
	linux-i2c, linux-iio, linux-stm32, linux-arm-kernel, netdev,
	linux-leds, linux-wireless, linux-rtc, linux-scsi, linux-spi,
	linux-amlogic, linux-rockchip, linux-samsung-soc, greybus-dev,
	linux-staging, linux-serial, linux-usb, linux-fbdev, iommu,
	linux-trace-kernel, kasan-dev, linux-hardening, linux-nfs,
	linux-kbuild, alsa-devel, linux-sound
In-Reply-To: <20240403080702.3509288-1-arnd@kernel.org>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  3 Apr 2024 10:06:18 +0200 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Compilers traditionally warn for unused 'static' variables, but not
> if they are constant. The reason here is a custom for C++ programmers
> to define named constants as 'static const' variables in header files
> instead of using macros or enums.
> 
> [...]

Here is the summary with links:
  - [05/34] 3c515: remove unused 'mtu' variable
    https://git.kernel.org/netdev/net-next/c/17b35355c2c6
  - [19/34] sunrpc: suppress warnings for unused procfs functions
    (no matching commit)
  - [26/34] isdn: kcapi: don't build unused procfs code
    https://git.kernel.org/netdev/net-next/c/91188544af06
  - [28/34] net: xgbe: remove extraneous #ifdef checks
    https://git.kernel.org/netdev/net-next/c/0ef416e045ad
  - [33/34] drivers: remove incorrect of_match_ptr/ACPI_PTR annotations
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users
From: Xilin Wu @ 2024-04-06  3:03 UTC (permalink / raw)
  To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
	Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
	Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
	Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
	Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
	linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
	Bartosz Golaszewski
In-Reply-To: <20240325131624.26023-1-brgl@bgdev.pl>


On 2024/3/25 21:16, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org>
>
> Note: I dropped most of the the review and test tags on purpose, the code
> changed significantly and warrants a new round of reviews and tests.
>
> ===
>
> Problem statement #1: Dynamic bus chicken-and-egg problem.
>
> Certain on-board PCI devices need to be powered up before they are can be
> detected but their PCI drivers won't get bound until the device is
> powered-up so enabling the relevant resources in the PCI device driver
> itself is impossible.
>
> Problem statement #2: Sharing inter-dependent resources between devices.
>
> Certain devices that use separate drivers (often on different busses)
> share resources (regulators, clocks, etc.). Typically these resources
> are reference-counted but in some cases there are additional interactions
> between them to consider, for example specific power-up sequence timings.
>
> ===
>
> The reason for tackling both of these problems in a single series is the
> fact the the platform I'm working on - Qualcomm RB5 - deals with both and
> both need to be addressed in order to enable WLAN and Bluetooth support
> upstream.
>
> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that
> takes inputs from the host and exposes LDO outputs consumed by the BT and
> WLAN modules which can be powered-up and down independently. However
> a delay of 100ms must be respected between enabling the BT- and
> WLAN-enable GPIOs.
>
> A similar design with a discreet PMU is also employed in other models of
> the WCN family of chips although we can often do without the delays. With
> this series we add support for the WCN7850 as well.
>
> ===
>
> We introduce a new subsystem here - the power sequencing framework. The
> qcom-wcn driver that we add is its first user. It implements the power-up
> sequences for QCA6390 and WCN7850 chips. However - we only use it to
> power-up the bluetooth module in the former. We use it to driver the WLAN
> modules in both. The reason for this is that for WCN7850 we have
> comprehensive bindings already upstream together with existing DT users.
> Porting them to using the pwrseq subsystem can be done separately and in
> an incremental manner once the subsystem itself is upstream. We will also
> have to ensure backward DT compatibility. To avoid overcomplicating this
> series, let's leave it out for now.
>
> ===
>
> This series is logically split into several sections. I'll go
> patch-by-patch and explain each step.
>
> Patches 1/16-5/16:
>
> These contain all relevant DT bindings changes. We add new documents for
> the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
> for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
> in QCA6390.
>
> Patches 6/16-8/16:
>
> These contain changes to device-tree sources for the three platforms we
> work with in this series. We model the PMUs of the WLAN/BT chips as
> top-level platform devices on the device tree. In order to limit the scope
> of this series and not introduce an excessive amount of confusion with
> deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
> as is (meaning: they continue to consumer the GPIOs and power inputs from
> the host). As the WCN7850 module doesn't require any specific timings, we can
> incrementally change that later.
>
> In both cases we add WLAN nodes that consume the power outputs of the PMU.
> For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
> of the PMU - we can do it as the bindings for this chip did not define any
> supply handles prior to this series meaning we are able to get this correct
> right away.
>
> Patches 9/16-12/16:
>
> These contain the bulk of the PCI changes for this series. We introduce
> a simple framework for powering up PCI devices before detecting them on
> the bus.
>
> The general approach is as follows: PCI devices that need special
> treatment before they can be powered up, scanned and bound to their PCI
> drivers must be described on the device-tree as child nodes of the PCI
> port node. These devices will be instantiated on the platform bus. They
> will in fact be generic platform devices with the compatible of the form
> used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We
> add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl
> drivers. These drivers are platform drivers that will now be matched
> against the devices instantiated from port children just like any other
> platform pairs.
>
> Both the power control platform device *AND* the associated PCI device
> reuse the same OF node and have access to the same properties. The goal
> of the platform driver is to request and bring up any required resources
> and let the pwrctl framework know that it's now OK to rescan the bus and
> detect the devices. When the device is bound, we are notified about it
> by the PCI bus notifier event and can establish a device link between the
> power control device and the PCI device so that any future extension for
> power-management will already be able to work with the correct hierachy.
>
> The reusing of the OF node is the reason for the small changes to the PCI
> OF core: as the bootloader can possibly leave the relevant regulators on
> before booting linux, the PCI device can be detected before its platform
> abstraction is probed. In this case, we find that device first and mark
> its OF node as reused. The pwrctl framework handles the opposite case
> (when the PCI device is detected only after the platform driver
> successfully enabled it).
>
> Patch 13/16 - 14/16:
>
> These add a relatively simple power sequencing subsystem and the first
> driver using it: the pwrseq module for the PMUs on the WCN family of chips.
>
> I'm proposing to add a subsystem that allows different devices to use a shared
> power sequence split into consumer-specific as well as common "units".
>
> A power sequence provider driver registers a set of units with pwrseq
> core. Each unit can be enabled and disabled and contains an optional list
> of other units which must be enabled before it itself can be. A unit
> represents a discreet chunk of the power sequence.
>
> It also registers a list of targets: a target is an abstraction wrapping
> a unit which allows consumers to tell pwrseq which unit they want to
> reach. Real-life example is the driver we're adding here: there's a set
> of common regulators, two PCIe-specific ones and two enable GPIOs: one
> for Bluetooth and one for WLAN.
>
> The Bluetooth driver requests a descriptor to the power sequencer and
> names the target it wants to reach:
>
>      pwrseq = devm_pwrseq_get(dev, "bluetooth");
>
> The pwrseq core then knows that when the driver calls:
>
>      pwrseq_power_on(pwrseq);
>
> It must enable the "bluetooth-enable" unit but it depends on the
> "regulators-common" unit so this one is enabled first. The provider
> driver is also in charge of assuring an appropriate delay between
> enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are
> handled by the "wlan-enable" unit and so are not enabled until the WLAN
> driver requests the "wlan" target to be powered on.
>
> Another thing worth discussing is the way we associate the consumer with
> the relevant power sequencer. DT maintainers have expressed a discontent
> with the existing mmc pwrseq bindings and have NAKed an earlier
> initiative to introduce global pwrseq bindings to the kernel[1].
>
> In this approach, we model the existing regulators and GPIOs in DT but
> the pwrseq subsystem requires each provider to provide a .match()
> callback. Whenever a consumer requests a power sequencer handle, we
> iterate over the list of pwrseq drivers and call .match() for each. It's
> up to the driver to verify in a platform-specific way whether it deals
> with its consumer and let the core pwrseq code know.
>
> The advantage of this over reusing the regulator or reset subsystem is
> that it's more generalized and can handle resources of all kinds as well
> as deal with any kind of power-on sequences: for instance, Qualcomm has
> a PCI switch they want a driver for but this switch requires enabling
> some resources first (PCI pwrctl) and then configuring the device over
> I2C (which can be handled by the pwrseq provider).
>
> Patch 15:
>
> This patch makes the Qualcomm Bluetooth driver get and use the power
> sequencer for QCA6390.
>
> Patch 16:
>
> While tiny, this patch is possibly the highlight of the entire series.
> It uses the two abstraction layers we introduced before to create an
> elegant power sequencing PCI power control driver and supports the ath11k
> module on QCA6390 and ath12k on WCN7850.
>
> With this series we can now enable BT and WLAN on several new Qualcomm
> boards upstream.
>
> Tested on RB5, sm8650-qrd and sm8550-qrd.
>
> Changelog:
>
> Since v5:
> - unify the approach to modelling the WCN WLAN/BT chips by always exposing
>    the PMU node on the device tree and making the WLAN and BT nodes become
>    consumers of its power outputs; this includes a major rework of the DT
>    sources, bindings and driver code; there's no more a separate PCI
>    pwrctl driver for WCN7850, instead its power-up sequence was moved
>    into the pwrseq driver common for all WCN chips
> - don't set load_uA from new regulator consumers
> - fix reported kerneldoc issues
> - drop voltage ranges for PMU outputs from DT
> - many minor tweaks and reworks
>
> v1: Original RFC:
>
> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/
>
> v2: First real patch series (should have been PATCH v2) adding what I
>      referred to back then as PCI power sequencing:
>
> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/
>
> v3: RFC for the DT representation of the PMU supplying the WLAN and BT
>      modules inside the QCA6391 package (was largely separate from the
>      series but probably should have been called PATCH or RFC v3):
>
> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/
>
> v4: Second attempt at the full series with changed scope (introduction of
>      the pwrseq subsystem, should have been RFC v4)
>
> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/
>
> v5: Two different ways of handling QCA6390 and WCN7850:
>
> https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/
>
> Bartosz Golaszewski (16):
>    regulator: dt-bindings: describe the PMU module of the QCA6390 package
>    regulator: dt-bindings: describe the PMU module of the WCN7850 package
>    dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
>    dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on
>      QCA6390
>    dt-bindings: net: wireless: describe the ath12k PCI module
>    arm64: dts: qcom: sm8550-qrd: add the Wifi node
>    arm64: dts: qcom: sm8650-qrd: add the Wifi node
>    arm64: dts: qcom: qrb5165-rb5: add the Wifi node
>    PCI: hold the rescan mutex when scanning for the first time
>    PCI/pwrctl: reuse the OF node for power controlled devices
>    PCI/pwrctl: create platform devices for child OF nodes of the port
>      node
>    PCI/pwrctl: add PCI power control core code
>    power: sequencing: implement the pwrseq core
>    power: pwrseq: add a driver for the PMU module on the QCom WCN
>      chipsets
>    Bluetooth: qca: use the power sequencer for QCA6390
>    PCI/pwrctl: add a PCI power control driver for power sequenced devices
>
>   .../net/bluetooth/qualcomm-bluetooth.yaml     |   17 +
>   .../net/wireless/qcom,ath11k-pci.yaml         |   46 +
>   .../bindings/net/wireless/qcom,ath12k.yaml    |  100 ++
>   .../bindings/regulator/qcom,qca6390-pmu.yaml  |  185 +++
>   MAINTAINERS                                   |    8 +
>   arch/arm64/boot/dts/qcom/qrb5165-rb5.dts      |  103 +-
>   arch/arm64/boot/dts/qcom/sm8250.dtsi          |   10 +
>   arch/arm64/boot/dts/qcom/sm8550-qrd.dts       |   97 ++
>   arch/arm64/boot/dts/qcom/sm8550.dtsi          |   10 +
>   arch/arm64/boot/dts/qcom/sm8650-qrd.dts       |   89 ++
>   arch/arm64/boot/dts/qcom/sm8650.dtsi          |   10 +
>   drivers/bluetooth/hci_qca.c                   |   74 +-
>   drivers/pci/Kconfig                           |    1 +
>   drivers/pci/Makefile                          |    1 +
>   drivers/pci/bus.c                             |    9 +-
>   drivers/pci/of.c                              |   14 +-
>   drivers/pci/probe.c                           |    2 +
>   drivers/pci/pwrctl/Kconfig                    |   17 +
>   drivers/pci/pwrctl/Makefile                   |    6 +
>   drivers/pci/pwrctl/core.c                     |  136 +++
>   drivers/pci/pwrctl/pci-pwrctl-pwrseq.c        |   89 ++
>   drivers/pci/remove.c                          |    2 +
>   drivers/power/Kconfig                         |    1 +
>   drivers/power/Makefile                        |    1 +
>   drivers/power/sequencing/Kconfig              |   28 +
>   drivers/power/sequencing/Makefile             |    6 +
>   drivers/power/sequencing/core.c               | 1065 +++++++++++++++++
>   drivers/power/sequencing/pwrseq-qcom-wcn.c    |  336 ++++++
>   include/linux/pci-pwrctl.h                    |   51 +
>   include/linux/pwrseq/consumer.h               |   56 +
>   include/linux/pwrseq/provider.h               |   75 ++
>   31 files changed, 2614 insertions(+), 31 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>   create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
>   create mode 100644 drivers/pci/pwrctl/Kconfig
>   create mode 100644 drivers/pci/pwrctl/Makefile
>   create mode 100644 drivers/pci/pwrctl/core.c
>   create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
>   create mode 100644 drivers/power/sequencing/Kconfig
>   create mode 100644 drivers/power/sequencing/Makefile
>   create mode 100644 drivers/power/sequencing/core.c
>   create mode 100644 drivers/power/sequencing/pwrseq-qcom-wcn.c
>   create mode 100644 include/linux/pci-pwrctl.h
>   create mode 100644 include/linux/pwrseq/consumer.h
>   create mode 100644 include/linux/pwrseq/provider.h


I tested the patchset on SM8550 and it does give me working WiFi. However I
seethe following warnings during boot.

[    5.973011] mhi mhi0: Requested to power ON
[    6.597591] mhi mhi0: Power on setup success
[    6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0'
[    6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S                 6.9.0-rc1-next-20240328-g955237c9980c #1
[    6.597635] Hardware name: AYN Odin 2 (DT)
[    6.597637] Workqueue: async async_run_entry_fn
[    6.597645] Call trace:
[    6.597646]  dump_backtrace+0xa0/0x128
[    6.597649]  show_stack+0x20/0x38
[    6.597650]  dump_stack_lvl+0x74/0x90
[    6.597653]  dump_stack+0x18/0x28
[    6.597654]  sysfs_warn_dup+0x6c/0x90
[    6.597658]  sysfs_add_bin_file_mode_ns+0xdc/0x100
[    6.597660]  sysfs_create_bin_file+0x7c/0xb8
[    6.597662]  pci_create_attr+0xb4/0x1a8
[    6.597665]  pci_create_resource_files+0x64/0xd0
[    6.597667]  pci_create_sysfs_dev_files+0x24/0x40
[    6.597669]  pci_bus_add_device+0x54/0x138
[    6.597670]  pci_bus_add_devices+0x40/0x98
[    6.597672]  pci_host_probe+0x70/0xf0
[    6.597673]  dw_pcie_host_init+0x248/0x658
[    6.597676]  qcom_pcie_probe+0x234/0x330
[    6.597677]  platform_probe+0x70/0xd8
[    6.597680]  really_probe+0xc8/0x3a0
[    6.597681]  __driver_probe_device+0x84/0x170
[    6.597682]  driver_probe_device+0x44/0x120
[    6.597683]  __device_attach_driver+0xc4/0x168
[    6.597684]  bus_for_each_drv+0x8c/0xf0
[    6.597686]  __device_attach_async_helper+0xb4/0x118
[    6.597687]  async_run_entry_fn+0x40/0x178
[    6.597689]  process_one_work+0x16c/0x410
[    6.597691]  worker_thread+0x284/0x3a0
[    6.597693]  kthread+0x118/0x128
[    6.597693]  ret_from_fork+0x10/0x20
[    6.597698] ------------[ cut here ]------------
[    6.597698] proc_dir_entry '0000:00/00.0' already registered
[    6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0
[    6.597713] Modules linked in:
[    6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S                 6.9.0-rc1-next-20240328-g955237c9980c #1
[    6.597715] Hardware name: AYN Odin 2 (DT)
[    6.597716] Workqueue: async async_run_entry_fn
[    6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[    6.597719] pc : proc_register+0x138/0x1d0
[    6.597721] lr : proc_register+0x138/0x1d0
[    6.597723] sp : ffff800081e3b9a0
[    6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0
[    6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540
[    6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004
[    6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff
[    6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910
[    6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165
[    6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528
[    6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000
[    6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff
[    6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000
[    6.597737] Call trace:
[    6.597737]  proc_register+0x138/0x1d0
[    6.597739]  proc_create_data+0x48/0x78
[    6.597741]  pci_proc_attach_device+0x84/0x118
[    6.597743]  pci_bus_add_device+0x5c/0x138
[    6.597744]  pci_bus_add_devices+0x40/0x98
[    6.597745]  pci_host_probe+0x70/0xf0
[    6.597746]  dw_pcie_host_init+0x248/0x658
[    6.597748]  qcom_pcie_probe+0x234/0x330
[    6.597749]  platform_probe+0x70/0xd8
[    6.597750]  really_probe+0xc8/0x3a0
[    6.597751]  __driver_probe_device+0x84/0x170
[    6.597752]  driver_probe_device+0x44/0x120
[    6.597753]  __device_attach_driver+0xc4/0x168
[    6.597754]  bus_for_each_drv+0x8c/0xf0
[    6.597756]  __device_attach_async_helper+0xb4/0x118
[    6.597757]  async_run_entry_fn+0x40/0x178
[    6.597759]  process_one_work+0x16c/0x410
[    6.597760]  worker_thread+0x284/0x3a0
[    6.597761]  kthread+0x118/0x128
[    6.597762]  ret_from_fork+0x10/0x20
[    6.597763] ---[ end trace 0000000000000000 ]---

This probably only occurs when the relevant drivers on compiled as built-in.
Similar behavior has been noticed before as well:

https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a

Thanks,
Xilin


^ permalink raw reply

* [Bug 218686] New: Fail to set energy_performance_preference of amd processor on asus ga403uv
From: bugzilla-daemon @ 2024-04-06  2:56 UTC (permalink / raw)
  To: linux-pm

https://bugzilla.kernel.org/show_bug.cgi?id=218686

            Bug ID: 218686
           Summary: Fail to set energy_performance_preference of amd
                    processor on asus ga403uv
           Product: Power Management
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: cpufreq
          Assignee: linux-pm@vger.kernel.org
          Reporter: al0uette@outlook.com
        Regression: No

Created attachment 306091
  --> https://bugzilla.kernel.org/attachment.cgi?id=306091&action=edit
ssdt table containing _CPC things

I enabled amd-pstate-epp driver by appending 'amd_pstate=active' to kernel
cmdline, and set scaling_governer to powersave, and

'cat
/sys/devices/system/cpu/cpu*/cpufreq/energy_performance_available_preferences'

do show

'default performance balance_performance balance_power power'.

However when I try to change epp to anything other than 'performance'(which is
the default) with something like

'echo power |sudo tee
/sys/devices/system/cpu/cpufreq/policy*/energy_performance_preference',

I get 'tee:
/sys/devices/system/cpu/cpufreq/policy0/energy_performance_preference: Unknown
error 524', which, according to source code, I guess is ENOTSUPP. And I guess
it's because cppc_set_epp_perf() in     linux/drivers/acpi/cppc_acpi.c checks
CPC_IN_PCC(epp_set_reg), and think the register doesn't exist. However I do see
the _CPC things in my ssdt. Don't know its' a bug in kernel or BIOS so I report
it here

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* [Bug 217931] amd-pstate lacks crucial features: CPU frequency and boost control
From: bugzilla-daemon @ 2024-04-06  1:43 UTC (permalink / raw)
  To: linux-pm
In-Reply-To: <bug-217931-137361@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=217931

--- Comment #58 from Artem S. Tashkinov (aros@gmx.com) ---
Thank you, Perry, there's no rush!

Not the last question, here's one more, I wrote about it earlier.

cpuinfo_max_freq is invalid for multiple cores here as it often reports insane
values:

$ lscpu -ae
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE    MAXMHZ   MINMHZ       MHZ
  0    0      0    0 0:0:0:0          yes 5137.0000 400.0000  400.0000
  1    0      0    0 0:0:0:0          yes 5137.0000 400.0000 1394.3910
  2    0      0    1 1:1:1:0          yes 6080.0000 400.0000  400.0000
  3    0      0    1 1:1:1:0          yes 6080.0000 400.0000  400.0000
  4    0      0    2 2:2:2:0          yes 5608.0000 400.0000 1396.2390
  5    0      0    2 2:2:2:0          yes 5608.0000 400.0000  400.0000
  6    0      0    3 3:3:3:0          yes 5293.0000 400.0000  400.0000
  7    0      0    3 3:3:3:0          yes 5293.0000 400.0000  400.0000
  8    0      0    4 4:4:4:0          yes 5449.0000 400.0000 1381.5620
  9    0      0    4 4:4:4:0          yes 5449.0000 400.0000 1393.0980
 10    0      0    5 5:5:5:0          yes 6080.0000 400.0000 1383.3900
 11    0      0    5 5:5:5:0          yes 6080.0000 400.0000 1397.3979
 12    0      0    6 6:6:6:0          yes 5764.0000 400.0000 1397.4200
 13    0      0    6 6:6:6:0          yes 5764.0000 400.0000  400.0000
 14    0      0    7 7:7:7:0          yes 5924.0000 400.0000 1396.0959
 15    0      0    7 7:7:7:0          yes 5924.0000 400.0000  400.0000

5137MHz is probably the best my CPU cores can do. No way 6080MHz is possible. I
wonder if you could fix it. HWiNFO64 reads maximum CPU cores frequencies
correctly.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* [Bug 217931] amd-pstate lacks crucial features: CPU frequency and boost control
From: bugzilla-daemon @ 2024-04-06  1:39 UTC (permalink / raw)
  To: linux-pm
In-Reply-To: <bug-217931-137361@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=217931

--- Comment #57 from Perry Yuan(AMD) (Perry.Yuan@amd.com) ---

Hello,

Thank you for your message. I am currently out of the office from afternoon
today to 4/8 for Chinese QingMing Festival. response will be delayed.


Best wishes,
Perry Yuan

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox