[PATCH v2 0/4] devfreq: check the get_cur_freq() return value and use it in ufshcd

[PATCH v2 0/4] devfreq: check the get_cur_freq() return value and use it in ufshcd

[Bean Huo]

The devfreq core has three users of the optional ->get_cur_freq()
callback. Two of them check the return value, the third one does not and
passes an uninitialized frequency to the transition notifiers when the
callback fails. Patch 1 fixes that.

Patch 2 writes down what a driver is expected to return from the
callback. Today this has to be found by reading the devfreq core.

Patch 3 records the frequency the controller starts at.
ufshcd_init_clocks() puts the controller at its highest frequency, but
nothing writes that down, so clk_scaling.target_freq stays 0 and devfreq
starts with previous_freq at 0 as well. With use_pm_opp this makes
ufshcd_devfreq_get_dev_status() report 0 Hz, the ondemand governor then
asks for the maximum frequency, and ufshcd_devfreq_target() runs a full
ufshcd_devfreq_scale() that holds up the queue for up to a second only to
set the same OPP and the same gear again.

Patch 4 adds the ->get_cur_freq() callback to ufshcd. Without it the
cur_freq attribute shows the last frequency the governor selected, which
is wrong whenever the controller is scaled outside the governor, for
example after writing 0 to clkscale_enable.

The patches touch two subsystems. Patches 1 and 2 are for the devfreq
tree, patches 3 and 4 are for the SCSI tree. The two halves are
independent, at build time and at run time, and can be applied in either
order.

Patch was tested on a Radxa Dragon Q6A (1d84000.ufshc):

  before "echo 0 > clkscale_enable":  cur_freq 75000000, target_freq 75000000
  after  "echo 0 > clkscale_enable":  cur_freq 300000000, target_freq 75000000

Without it both files report 75000000 and keep doing so for as long as
clock scaling stays disabled. A 4 GiB direct read after enabling clock
scaling again counted the transitions in trans_stat and attributed time
to the 300000000 state, so the frequency the callback returns is one that
devfreq recognises.

One thing to be aware of: devfreq_monitor_resume() copies previous_freq
from the callback, but it does not call devfreq_update_status(). A
frequency change made while the governor was suspended therefore does not
show up as a transition. That is how devfreq behaves today and this
series does not change it.

Changes since v1:
- New patch 3, so that target_freq and devfreq's previous_freq are not 0
  at boot (suggested by Stanley Jhu).
- Patch 4: drop the !cur_freq check, it cannot happen any more.
- Drop the now stale comment in ufshcd_devfreq_get_dev_status().
- Patches 1 and 2 are unchanged.
- The devfreq and the ufshcd patches no longer depend on each other.
- Avri's Reviewed-by is on patches 1, 2 and 4. Patch 3 is new, so it does
  not carry it. Avri, please note that patch 4 changed since you reviewed
  it, the !cur_freq check is gone. Tell me if you want the tag dropped.


Bean Huo (4):
  PM / devfreq: Fall back to previous_freq when get_cur_freq() fails
  PM / devfreq: Add more details to the get_cur_freq() comment
  scsi: ufs: core: Record the frequency the controller starts at
  scsi: ufs: core: Report the current clock frequency to devfreq

 drivers/devfreq/devfreq.c |  5 ++---
 drivers/ufs/core/ufshcd.c | 37 +++++++++++++++++++++++++++++++------
 include/linux/devfreq.h   |  7 +++++--
 3 files changed, 38 insertions(+), 11 deletions(-)


base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
-- 
2.34.1

[PATCH v2 1/4] PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

devfreq_set_target() calls the optional ->get_cur_freq() callback to get
the frequency that is passed as freqs.old to the DEVFREQ_PRECHANGE and
DEVFREQ_POSTCHANGE notifiers, but it does not check the return value. If
the callback fails without setting @freq, cur_freq is never assigned, and
an uninitialized stack value is passed to the notifiers.

hisi_uncore_get_cur_freq() can hit this. It returns -ENODEV without
setting @freq when its PCC channel is missing. On the mailbox error path
it sets @freq to 0 instead, so that the core does not read a random
value.

The other two callers, cur_freq_show() and devfreq_monitor_resume(),
already check the return value and use devfreq->previous_freq when the
callback fails. Do the same in devfreq_set_target().

This does not seem to cause a visible problem today. The passive governor
is the only DEVFREQ_TRANSITION_NOTIFIER user in the tree, and it only
reads freqs.new. So this patch is not marked for stable.

Fixes: 0fe3a66410a3 ("PM / devfreq: Add new DEVFREQ_TRANSITION_NOTIFIER notifier")
Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>
---
 drivers/devfreq/devfreq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index f08fc6966eae..f20d9a660779 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -348,9 +348,8 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
 	unsigned long cur_freq;
 	int err = 0;
 
-	if (devfreq->profile->get_cur_freq)
-		devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
-	else
+	if (!devfreq->profile->get_cur_freq ||
+	    devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq))
 		cur_freq = devfreq->previous_freq;
 
 	freqs.old = cur_freq;
-- 
2.34.1

Re: [PATCH v2 1/4] PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] devfreq->resume_freq is incorrectly overwritten with devfreq->suspend_freq during system suspend, causing devices to resume to their suspend frequency rather than their active frequency.
--

commit 252cf4045ad4d642d45961b4d6845aa317e8ee57
Author: Bean Huo <beanhuo@micron.com>

PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

This commit updates devfreq_set_target() to fall back to the previous
frequency when get_cur_freq() returns an error. This prevents uninitialized
stack values from being passed to notifiers during frequency changes.

> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eaee..f20d9a6607791 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -348,9 +348,8 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
>  	unsigned long cur_freq;
>  	int err = 0;
>  
> -	if (devfreq->profile->get_cur_freq)
> -		devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
> -	else
> +	if (!devfreq->profile->get_cur_freq ||
> +	    devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq))
>  		cur_freq = devfreq->previous_freq;
>  
>  	freqs.old = cur_freq;

[Severity: Medium]
This is a pre-existing issue, but since we are modifying devfreq_set_target(),
I noticed a potential issue with how resume frequencies are stored.

Further down in devfreq_set_target(), there is this unconditional assignment:

drivers/devfreq/devfreq.c:devfreq_set_target() {
    ...
    if (devfreq->suspend_freq)
        devfreq->resume_freq = new_freq;

    return err;
}

When a device defines a suspend_freq, system suspend invokes
devfreq_suspend_device(), which places the device in the low-power state
by calling:

drivers/devfreq/devfreq.c:devfreq_suspend_device() {
    ...
    ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0);
    ...
}

Does this cause devfreq_set_target() to unintentionally overwrite the saved
active frequency with the suspend frequency?

It appears that during every system suspend cycle, resume_freq gets updated to
the suspend_freq target. Will this cause devices to resume to their lowest
suspend frequency instead of their pre-suspend active frequency, degrading
performance until the governor recalculates a new target?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192140.2701755-1-beanhuo@iokpp.de?part=1

Re: [PATCH v2 1/4] PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

[Bean Huo]

On Mon, 2026-09-07 at 19:33 +0000, sashiko-bot@kernel.org wrote:
> Pre-existing issues:
> - [Medium] devfreq->resume_freq is incorrectly overwritten with devfreq-
> >suspend_freq during system suspend, causing devices to resume to their
> suspend frequency rather than their active frequency.
> --
> 
> commit 252cf4045ad4d642d45961b4d6845aa317e8ee57
> Author: Bean Huo <beanhuo@micron.com>
> 
> PM / devfreq: Fall back to previous_freq when get_cur_freq() fails
> 
> This commit updates devfreq_set_target() to fall back to the previous
> frequency when get_cur_freq() returns an error. This prevents uninitialized
> stack values from being passed to notifiers during frequency changes.
> 
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index f08fc6966eaee..f20d9a6607791 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -348,9 +348,8 @@ static int devfreq_set_target(struct devfreq *devfreq,
> > unsigned long new_freq,
> >         unsigned long cur_freq;
> >         int err = 0;
> >   
> > -       if (devfreq->profile->get_cur_freq)
> > -               devfreq->profile->get_cur_freq(devfreq->dev.parent,
> > &cur_freq);
> > -       else
> > +       if (!devfreq->profile->get_cur_freq ||
> > +           devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq))
> >                 cur_freq = devfreq->previous_freq;
> >   
> >         freqs.old = cur_freq;
> 
> [Severity: Medium]
> This is a pre-existing issue, but since we are modifying devfreq_set_target(),
> I noticed a potential issue with how resume frequencies are stored.
> 
> Further down in devfreq_set_target(), there is this unconditional assignment:
> 
> drivers/devfreq/devfreq.c:devfreq_set_target() {
>     ...
>     if (devfreq->suspend_freq)
>         devfreq->resume_freq = new_freq;
> 
>     return err;
> }

this should be a seperated fix patch, not for this patch.

[PATCH v2 2/4] PM / devfreq: Add more details to the get_cur_freq() comment

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

The comment for ->get_cur_freq() only says that the device should
provide the frequency at which it is operating. It does not tell the
driver author which unit to use, what the return value means, or that
the frequency should be one of @freq_table. These have to be found by
reading the devfreq core.

Add these details to the comment.

Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>
---
 include/linux/devfreq.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index dc1075dc3446..809ef29b9af5 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -90,8 +90,11 @@ struct devfreq_dev_status {
  *			use this directly. Instead, governors are recommended
  *			to use devfreq_update_stats() along with
  *			devfreq.last_status.
- * @get_cur_freq:	The device should provide the current frequency
- *			at which it is operating.
+ * @get_cur_freq:	The device should provide the frequency, in Hz, at
+ *			which it is currently operating, and return 0, or a
+ *			negative errno on failure. The frequency should be one
+ *			of @freq_table, as it is used for the transition
+ *			statistics.
  * @exit:		An optional callback that is called when devfreq
  *			is removing the devfreq object due to error or
  *			from devfreq_remove_device() call. If the user
-- 
2.34.1

[PATCH v2 3/4] scsi: ufs: core: Record the frequency the controller starts at

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

ufshcd_init_clocks() puts the controller at its highest frequency, but
nothing writes that down. clk_scaling.target_freq stays 0, and
devfreq_dev_profile.initial_freq is never set, so devfreq->previous_freq
is 0 as well.

With use_pm_opp this shows up in a few places. The target_freq attribute
reads 0 until the governor scales for the first time.
ufshcd_devfreq_get_dev_status() reports 0 Hz, which makes the ondemand
governor ask for the maximum frequency. ufshcd_devfreq_target() then
sees 0 != max and runs a full ufshcd_devfreq_scale(), which holds up the
queue for up to a second only to set the same OPP and the same gear
again. Without OPPs the frequency is not reported as 0, but
previous_freq is, and devfreq_update_status() then drops the first
time_in_state update.

Record the maximum frequency in ufshcd_devfreq_init() instead.
ufshcd_add_lus() runs after ufshcd_probe_hba() has geared up to
hba->max_pwr_info.info, so the clocks and the gear are both at their
maximum by the time we get here. The only difference is that the first
governor poll no longer redoes work that is already done. From the
second poll on nothing changes, because target_freq held the maximum
frequency there anyway.

That first scale also re-applied the gear that
ufshcd_vops_freq_to_gear_speed() maps the maximum frequency to, so it
quietly corrected the link if the OPP table and the gear negotiated at
probe disagreed. That does not happen any more. On ufs-qcom the two
cannot disagree, because ufs_qcom_negotiate_pwr_mode() clamps the gear
through ufshcd_negotiate_pwr_params() against the same controller
capability the OPP table is written from.

clki->max_freq is the right value in both modes.
ufshcd_parse_clock_min_max_freq() fills it from the highest OPP, and
ufshcd_clkscale_enable_store() already uses it the same way.

Suggested-by: Stanley Jhu <stanleyjhu@google.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufshcd.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..351c76094b9f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1681,11 +1681,6 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
 	if (!scaling->window_start_t)
 		goto start_window;
 
-	/*
-	 * If current frequency is 0, then the ondemand governor considers
-	 * there's no initial frequency set. And it always requests to set
-	 * to max. frequency.
-	 */
 	if (hba->use_pm_opp) {
 		stat->current_frequency = hba->clk_scaling.target_freq;
 	} else {
@@ -1727,12 +1722,21 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
 	if (list_empty(clk_list))
 		return 0;
 
+	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
+
 	if (!hba->use_pm_opp) {
-		clki = list_first_entry(clk_list, struct ufs_clk_info, list);
 		dev_pm_opp_add(hba->dev, clki->min_freq, 0);
 		dev_pm_opp_add(hba->dev, clki->max_freq, 0);
 	}
 
+	/*
+	 * ufshcd_init_clocks() has already set the clocks to the highest
+	 * frequency, and nothing has changed them since. Save that frequency,
+	 * so that devfreq and the clock scaling code know where we start.
+	 */
+	hba->clk_scaling.target_freq = clki->max_freq;
+	hba->vps->devfreq_profile.initial_freq = clki->max_freq;
+
 	ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
 					 &hba->vps->ondemand_data);
 	devfreq = devfreq_add_device(hba->dev,
-- 
2.34.1

Re: [PATCH v2 3/4] scsi: ufs: core: Record the frequency the controller starts at

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unprotected concurrent modification of the globally shared `ufs_hba_vps` structure during asynchronous probe.
--

commit d1d1b66402811c53b2fbc0d56ed07cd4f77540ee
Author: Bean Huo <beanhuo@micron.com>

scsi: ufs: core: Record the frequency the controller starts at

This commit records the maximum frequency during ufshcd_devfreq_init(),
preventing devfreq from reporting an initial 0 Hz frequency. It avoids
an unnecessary governor scale that resets the same frequency and gear.
It addresses the issue for both modes with and without OPPs.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2ba244cf40ac7..351c76094b9ff 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -1727,12 +1722,21 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  	if (list_empty(clk_list))
>  		return 0;
>  
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +
>  	if (!hba->use_pm_opp) {
> -		clki = list_first_entry(clk_list, struct ufs_clk_info, list);
>  		dev_pm_opp_add(hba->dev, clki->min_freq, 0);
>  		dev_pm_opp_add(hba->dev, clki->max_freq, 0);
>  	}
>  
> +	/*
> +	 * ufshcd_init_clocks() has already set the clocks to the highest
> +	 * frequency, and nothing has changed them since. Save that frequency,
> +	 * so that devfreq and the clock scaling code know where we start.
> +	 */
> +	hba->clk_scaling.target_freq = clki->max_freq;
> +	hba->vps->devfreq_profile.initial_freq = clki->max_freq;

[Severity: Medium]
Could this introduce a data race during asynchronous probe?

Because ufs_hba_vps is a globally shared static structure assigned to all UFS
host controllers by default (hba->vps = &ufs_hba_vps), writing directly to
hba->vps->devfreq_profile.initial_freq mutates this shared state.

If multiple controllers probe concurrently during system initialization
(e.g. via ufshcd_async_scan), they will race to overwrite this field.
If the controllers have different maximum frequencies, they might overwrite
each other's initial devfreq settings, causing devfreq to start with an
incorrect frequency on one or more controllers.

> +
>  	ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
>  					 &hba->vps->ondemand_data);
>  	devfreq = devfreq_add_device(hba->dev,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192140.2701755-1-beanhuo@iokpp.de?part=3

Re: [PATCH v2 3/4] scsi: ufs: core: Record the frequency the controller starts at

[Bean Huo]

On Mon, 2026-09-07 at 19:41 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] Unprotected concurrent modification of the globally shared
> `ufs_hba_vps` structure during asynchronous probe.
> --
> 
> commit d1d1b66402811c53b2fbc0d56ed07cd4f77540ee
> Author: Bean Huo <beanhuo@micron.com>
> 
> scsi: ufs: core: Record the frequency the controller starts at
> 
> This commit records the maximum frequency during ufshcd_devfreq_init(),
> preventing devfreq from reporting an initial 0 Hz frequency. It avoids
> an unnecessary governor scale that resets the same frequency and gear.
> It addresses the issue for both modes with and without OPPs.
> 
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 2ba244cf40ac7..351c76094b9ff 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> 
> [ ... ]
> 
> > @@ -1727,12 +1722,21 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
> >         if (list_empty(clk_list))
> >                 return 0;
> >  
> > +       clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> > +
> >         if (!hba->use_pm_opp) {
> > -               clki = list_first_entry(clk_list, struct ufs_clk_info,
> > list);
> >                 dev_pm_opp_add(hba->dev, clki->min_freq, 0);
> >                 dev_pm_opp_add(hba->dev, clki->max_freq, 0);
> >         }
> >  
> > +       /*
> > +        * ufshcd_init_clocks() has already set the clocks to the highest
> > +        * frequency, and nothing has changed them since. Save that
> > frequency,
> > +        * so that devfreq and the clock scaling code know where we start.
> > +        */
> > +       hba->clk_scaling.target_freq = clki->max_freq;
> > +       hba->vps->devfreq_profile.initial_freq = clki->max_freq;
> 
> [Severity: Medium]
> Could this introduce a data race during asynchronous probe?
> 
> Because ufs_hba_vps is a globally shared static structure assigned to all UFS
> host controllers by default (hba->vps = &ufs_hba_vps), writing directly to
> hba->vps->devfreq_profile.initial_freq mutates this shared state.
> 
> If multiple controllers probe concurrently during system initialization
> (e.g. via ufshcd_async_scan), they will race to overwrite this field.
> If the controllers have different maximum frequencies, they might overwrite
> each other's initial devfreq settings, causing devfreq to start with an
> incorrect frequency on one or more controllers.

hba->vps points at one global structure shared by every controller, but this is
not new in this patch, The same issue exists in wb_flush_threshold, which is
even writable from sysfs, so a write on one controller changes the other one,
for the system with multiple UFS controllers, it is better to seperate
ufs_hba_vps by using devm_kmemdup.





> 
> > +
> >         ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
> >                                          &hba->vps->ondemand_data);
> >         devfreq = devfreq_add_device(hba->dev,
> 

[PATCH v2 4/4] scsi: ufs: core: Report the current clock frequency to devfreq

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

When a driver does not provide a ->get_cur_freq() callback, the cur_freq
sysfs attribute shows devfreq->previous_freq, which only tracks the
scaling that the governor itself did.

The UFS controller is also scaled outside the governor. The clearest
example is writing 0 to clkscale_enable: ufshcd_clkscale_enable_store()
sets the clocks to max_freq through ufshcd_devfreq_scale() and suspends
the governor, so devfreq_set_target() is never called. After that,
cur_freq keeps showing the last frequency the governor chose instead of
the one the controller runs at, and it does so as long as clock scaling
stays disabled.

Add ufshcd_devfreq_get_cur_freq(). It reports clk_scaling.target_freq
when OPPs are used and the first clock's curr_freq otherwise, the same
values that ufshcd_devfreq_get_dev_status() reports.

Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>
---
 drivers/ufs/core/ufshcd.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 351c76094b9f..bac95e891de5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1711,6 +1711,26 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
 	return 0;
 }
 
+static int ufshcd_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+
+	if (!ufshcd_is_clkscaling_supported(hba))
+		return -EINVAL;
+
+	if (hba->use_pm_opp) {
+		*freq = hba->clk_scaling.target_freq;
+	} else {
+		struct ufs_clk_info *clki;
+
+		clki = list_first_entry(&hba->clk_list_head,
+					struct ufs_clk_info, list);
+		*freq = clki->curr_freq;
+	}
+
+	return 0;
+}
+
 static int ufshcd_devfreq_init(struct ufs_hba *hba)
 {
 	struct list_head *clk_list = &hba->clk_list_head;
@@ -9616,6 +9636,7 @@ static struct ufs_hba_variant_params ufs_hba_vps = {
 	.devfreq_profile.polling_ms	= 100,
 	.devfreq_profile.target		= ufshcd_devfreq_target,
 	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
+	.devfreq_profile.get_cur_freq	= ufshcd_devfreq_get_cur_freq,
 	.ondemand_data.upthreshold	= 70,
 	.ondemand_data.downdifferential	= 5,
 };
-- 
2.34.1

Re: [PATCH v2 4/4] scsi: ufs: core: Report the current clock frequency to devfreq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Lockless read of shared frequency states (target_freq and curr_freq) introduces a data race.
--

commit 33056f7bc209eb72a0cfba73ae939b67d7a4ed81
Author: Bean Huo <beanhuo@micron.com>

scsi: ufs: core: Report the current clock frequency to devfreq

When a driver does not provide a ->get_cur_freq() callback, the cur_freq
sysfs attribute shows devfreq->previous_freq. This commit adds a
->get_cur_freq() callback to report the current clock frequency accurately.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 351c76094b9ff..bac95e891de5d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -1711,6 +1711,26 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
>  	return 0;
>  }
>  
> +static int ufshcd_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +
> +	if (!ufshcd_is_clkscaling_supported(hba))
> +		return -EINVAL;
> +
> +	if (hba->use_pm_opp) {
> +		*freq = hba->clk_scaling.target_freq;

[Severity: Low]
Is it possible for this lockless read of target_freq to race with frequency
scaling updates?

If the devfreq sysfs cur_freq attribute is read concurrently with updates like
ufshcd_clkscale_enable_store() or ufshcd_devfreq_target(), could it trigger a
KCSAN data race warning or result in a stale read? Should this be protected or
use READ_ONCE()?

> +	} else {
> +		struct ufs_clk_info *clki;
> +
> +		clki = list_first_entry(&hba->clk_list_head,
> +					struct ufs_clk_info, list);
> +		*freq = clki->curr_freq;

[Severity: Low]
Similarly, could this lockless read of curr_freq race with updates in the same
way? Might READ_ONCE() be needed here to formally resolve the race?

> +	}
> +
> +	return 0;
> +}
> +
>  static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  {
>  	struct list_head *clk_list = &hba->clk_list_head;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192140.2701755-1-beanhuo@iokpp.de?part=4

Re: [PATCH v2 4/4] scsi: ufs: core: Report the current clock frequency to devfreq

[Bean Huo]

On Mon, 2026-09-07 at 19:35 +0000, sashiko-bot@kernel.org wrote:
> > +               *freq = hba->clk_scaling.target_freq;
> 
> [Severity: Low]
> Is it possible for this lockless read of target_freq to race with frequency
> scaling updates?
> 
> If the devfreq sysfs cur_freq attribute is read concurrently with updates like
> ufshcd_clkscale_enable_store() or ufshcd_devfreq_target(), could it trigger a
> KCSAN data race warning or result in a stale read? Should this be protected or
> use READ_ONCE()?
> 
> > +       } else {
> > +               struct ufs_clk_info *clki;
> > +
> > +               clki = list_first_entry(&hba->clk_list_head,
> > +                                       struct ufs_clk_info, list);
> > +               *freq = clki->curr_freq;
> 
> [Severity: Low]
> Similarly, could this lockless read of curr_freq race with updates in the same
> way? Might READ_ONCE() be needed here to formally resolve the race?

The read is lockless, but a lock would not help here.

Re: [PATCH v2 0/4] devfreq: check the get_cur_freq() return value and use it in ufshcd

[Stanley Jhu]

On Mon, 7 Sep 2026 21:21:36 +0200, Bean Huo wrote:
> Changes since v1:
> - New patch 3, so that target_freq and devfreq's previous_freq are not 0
>   at boot (suggested by Stanley Jhu).
> - Patch 4: drop the !cur_freq check, it cannot happen any more.

Thanks for addressing the initial frequency desync in v2.

For the series:
Reviewed-by: Stanley Jhu <stanleyjhu@google.com>

Re: [PATCH v2 0/4] devfreq: check the get_cur_freq() return value and use it in ufshcd

[Martin K. Petersen (Oracle)]

Bean,

> The patches touch two subsystems. Patches 1 and 2 are for the devfreq
> tree, patches 3 and 4 are for the SCSI tree. The two halves are
> independent, at build time and at run time, and can be applied in either
> order.

Patches 3 + 4 applied to 7.4/scsi-staging, thanks!

-- 
Martin K. Petersen