* [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask
@ 2026-11-09 9:42 raoxu
2026-11-09 21:39 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: raoxu @ 2026-11-09 9:42 UTC (permalink / raw)
To: lee; +Cc: andriy.shevchenko, mika.westerberg, mfd, linux-kernel, raoxu, stable
From: Xu Rao <raoxu@uniontech.com>
The PM QoS latency tolerance interface accepts non-negative s32 values,
but the LPSS active and idle LTR registers have only a ten-bit value
field. intel_lpss_ltr_set() selects the larger scale for requests above
1023 us and ORs val >> 5 into the register without limiting its width.
For a request of 32768 us, the shifted value is 1024 and the latency
value field becomes zero. A request of 100000 us leaves only 53 in the
value field. Still larger requests can set bits outside both the value
and scale fields. Thus a relaxed latency request can produce a much
stricter tolerance and unnecessarily restrict power saving.
Clamp the shifted value to LPSS_PRIV_LTR_VALUE_MASK before combining it
with LPSS_PRIV_LTR_SCALE_32US. This saturates oversized requests instead
of wrapping the field, matching the Intel LTR handling in sdhci-pci.
Negative values and PM_QOS_LATENCY_ANY retain their existing handling.
The supplied fix was reviewed and this commit message prepared with
OpenCode AI assistance.
Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Cc: stable@vger.kernel.org
---
drivers/mfd/intel-lpss.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 29f30ef0da99..3c3e75c23720 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -23,6 +23,7 @@
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/mfd/core.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/pm.h>
#include <linux/pm_qos.h>
@@ -179,7 +180,8 @@ static void intel_lpss_ltr_set(struct device *dev, s32 val)
ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
if (val > LPSS_PRIV_LTR_VALUE_MASK)
- ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
+ ltr |= LPSS_PRIV_LTR_SCALE_32US |
+ min_t(u32, val >> 5, LPSS_PRIV_LTR_VALUE_MASK);
else
ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
}
--
2.50.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask
2026-11-09 9:42 [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask raoxu
@ 2026-11-09 21:39 ` Andy Shevchenko
2026-11-09 21:40 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2026-11-09 21:39 UTC (permalink / raw)
To: raoxu; +Cc: lee, mika.westerberg, mfd, linux-kernel, stable
On Mon, Nov 09, 2026 at 05:42:42PM +0800, raoxu wrote:
>
> The PM QoS latency tolerance interface accepts non-negative s32 values,
> but the LPSS active and idle LTR registers have only a ten-bit value
> field. intel_lpss_ltr_set() selects the larger scale for requests above
> 1023 us and ORs val >> 5 into the register without limiting its width.
>
> For a request of 32768 us, the shifted value is 1024 and the latency
> value field becomes zero. A request of 100000 us leaves only 53 in the
> value field. Still larger requests can set bits outside both the value
> and scale fields. Thus a relaxed latency request can produce a much
> stricter tolerance and unnecessarily restrict power saving.
>
> Clamp the shifted value to LPSS_PRIV_LTR_VALUE_MASK before combining it
> with LPSS_PRIV_LTR_SCALE_32US. This saturates oversized requests instead
> of wrapping the field, matching the Intel LTR handling in sdhci-pci.
> Negative values and PM_QOS_LATENCY_ANY retain their existing handling.
Shouldn't we rather forbid to apply wrong values?
> The supplied fix was reviewed and this commit message prepared with
> OpenCode AI assistance.
This paragraph should go after '---' line below.
> Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> Cc: stable@vger.kernel.org
> ---
...
> if (val > LPSS_PRIV_LTR_VALUE_MASK)
> - ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
> + ltr |= LPSS_PRIV_LTR_SCALE_32US |
> + min_t(u32, val >> 5, LPSS_PRIV_LTR_VALUE_MASK);
No min_t() in a new code. You mentioned clamping, use clamp(),
it's exactly for that.
> else
> ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask
2026-11-09 21:39 ` Andy Shevchenko
@ 2026-11-09 21:40 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-11-09 21:40 UTC (permalink / raw)
To: raoxu; +Cc: lee, mika.westerberg, mfd, linux-kernel, stable
On Mon, Nov 09, 2026 at 11:39:12PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 09, 2026 at 05:42:42PM +0800, raoxu wrote:
> >
> > The PM QoS latency tolerance interface accepts non-negative s32 values,
> > but the LPSS active and idle LTR registers have only a ten-bit value
> > field. intel_lpss_ltr_set() selects the larger scale for requests above
> > 1023 us and ORs val >> 5 into the register without limiting its width.
> >
> > For a request of 32768 us, the shifted value is 1024 and the latency
> > value field becomes zero. A request of 100000 us leaves only 53 in the
> > value field. Still larger requests can set bits outside both the value
> > and scale fields. Thus a relaxed latency request can produce a much
> > stricter tolerance and unnecessarily restrict power saving.
> >
> > Clamp the shifted value to LPSS_PRIV_LTR_VALUE_MASK before combining it
> > with LPSS_PRIV_LTR_SCALE_32US. This saturates oversized requests instead
> > of wrapping the field, matching the Intel LTR handling in sdhci-pci.
> > Negative values and PM_QOS_LATENCY_ANY retain their existing handling.
>
> Shouldn't we rather forbid to apply wrong values?
>
> > The supplied fix was reviewed and this commit message prepared with
> > OpenCode AI assistance.
>
> This paragraph should go after '---' line below.
...
> > Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> > Signed-off-by: Xu Rao <raoxu@uniontech.com>
> > Cc: stable@vger.kernel.org
Ah, and it's not a fix, it's just improving user experience. The interface even
not a sysfs one IIRC.
> > ---
...
> > if (val > LPSS_PRIV_LTR_VALUE_MASK)
> > - ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
> > + ltr |= LPSS_PRIV_LTR_SCALE_32US |
> > + min_t(u32, val >> 5, LPSS_PRIV_LTR_VALUE_MASK);
>
> No min_t() in a new code. You mentioned clamping, use clamp(),
> it's exactly for that.
>
> > else
> > ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-09 21:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-09 9:42 [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask raoxu
2026-11-09 21:39 ` Andy Shevchenko
2026-11-09 21:40 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®