[PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask

[PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask

[raoxu]

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

Re: [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask

[Andy Shevchenko]

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

Re: [PATCH] mfd: intel-lpss: Clamp latency tolerance to the register value mask

[Andy Shevchenko]

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