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

[PATCH 0/3] 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 this.

Patch 3 adds the 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 and patch 3 is for the SCSI tree. They do not depend on each other
at build time and can be applied separately. Patch 3 only needs patch 1
for the error case described above, which requires OPPs and happens
before the controller is scaled for the first time.

Tested on a Radxa Dragon Q6A with UFS 3.1, with patch 3 applied:

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

Before patch 3 both files report 75000000 and keep doing so for as long
as clock scaling stays disabled.

Clock scaling itself still works. A 4 GiB direct read moved the controller to
300000000 and back, and trans_stat recorded it.

Bean Huo (3):
  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: Report the current clock frequency to devfreq

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


base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
-- 
2.34.1

[PATCH 1/3] 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>
---
 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

[PATCH 2/3] 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>
---
 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 3/3] 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. Both are 0 until
the controller is scaled for the first time, so return an error in that
case and let devfreq use the frequency it last set.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufshcd.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..45187a7e4f4f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1716,6 +1716,37 @@ 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);
+	unsigned long cur_freq;
+
+	if (!ufshcd_is_clkscaling_supported(hba))
+		return -EINVAL;
+
+	if (hba->use_pm_opp) {
+		cur_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);
+		cur_freq = clki->curr_freq;
+	}
+
+	/*
+	 * target_freq stays 0 until something scales the controller for the
+	 * first time. Report nothing rather than 0 so devfreq falls back to
+	 * the frequency it last set.
+	 */
+	if (!cur_freq)
+		return -EINVAL;
+
+	*freq = cur_freq;
+
+	return 0;
+}
+
 static int ufshcd_devfreq_init(struct ufs_hba *hba)
 {
 	struct list_head *clk_list = &hba->clk_list_head;
@@ -9612,6 +9643,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 3/3] scsi: ufs: core: Report the current clock frequency to devfreq

[Stanley Jhu]

On Mon, 31 Aug 2026 15:01:29 +0200, Bean Huo wrote:
> +     /*
> +      * target_freq stays 0 until something scales the controller for the
> +      * first time. Report nothing rather than 0 so devfreq falls back to
> +      * the frequency it last set.
> +      */
> +     if (!cur_freq)
> +             return -EINVAL;

If ufshcd_devfreq_get_cur_freq() returns -EINVAL here, devfreq falls back
to devfreq->previous_freq. However, because UFS does not set
devfreq_profile.initial_freq, previous_freq is also 0 at boot. On platforms
with use_pm_opp=true, the initial sysfs read or the first transition before
scaling still ends up seeing 0 Hz.

Would it be cleaner to initialize hba->clk_scaling.target_freq and
devfreq_profile.initial_freq to the max frequency in ufshcd_devfreq_init()?
That way, cur_freq is valid from the start and we wouldn't need to handle
the !cur_freq case here.

Thanks,
Stanley Jhu

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

[Bean Huo]

On Tue, 2026-09-01 at 09:33 +0800, Stanley Jhu wrote:
> On Mon, 31 Aug 2026 15:01:29 +0200, Bean Huo wrote:
> > +     /*
> > +      * target_freq stays 0 until something scales the controller for the
> > +      * first time. Report nothing rather than 0 so devfreq falls back to
> > +      * the frequency it last set.
> > +      */
> > +     if (!cur_freq)
> > +             return -EINVAL;
> 
> If ufshcd_devfreq_get_cur_freq() returns -EINVAL here, devfreq falls back
> to devfreq->previous_freq. However, because UFS does not set
> devfreq_profile.initial_freq, previous_freq is also 0 at boot. On platforms
> with use_pm_opp=true, the initial sysfs read or the first transition before
> scaling still ends up seeing 0 Hz.
> 
> Would it be cleaner to initialize hba->clk_scaling.target_freq and
> devfreq_profile.initial_freq to the max frequency in ufshcd_devfreq_init()?
> That way, cur_freq is valid from the start and we wouldn't need to handle
> the !cur_freq case here.
> 
> Thanks,
> Stanley Jhu

yes, thanks, that is better. I will do it in v2.
digging deeper, the problem is a bit wider than the sysfs read. with use_pm_opp,
ufshcd_init_clocks() already puts the controller at the highest OPP, but nothing
records that, so target_freq stays 0. Then ufshcd_devfreq_get_dev_status()
reports 0 Hz, the ondemand governor asks for the maximum frequency, and
ufshcd_devfreq_target() sees 0 != max and 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. This happens today, without my patch.

Patches 1 and 2 are not affected.

Kind regards,
Bean

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

[Stanley Jhu]

Thanks for digging into this, Bean.

> That holds up the queue for up to a second only to set the same OPP and the same
> gear again. This happens today, without my patch.

In Non-OPP mode (!use_pm_opp), devfreq_profile.initial_freq also needs to be
initialized to clki->max_freq. Otherwise, devfreq->previous_freq remains 0 and
devfreq_update_status() drops the initial time_in_state accounting.

Thanks,
Stanley Jhu