Re: [PATCH 3/3] carlfw: disable buggy PSM to prevent USB command timeouts

[Christian Lamparter <chunkeey@gmail.com>]

On 3/17/26 10:11 AM, iamdevnull wrote:
> From: Masi Osmani <mas-i@hotmail.de>
> 
> The carl9170 firmware power save implementation causes the SH-2
> processor to stop responding to USB commands after entering PS mode.
> Powering down the ADDAC and synthesizer via rf_psm() makes the
> device miss host command responses, triggering -ETIMEDOUT (-110)
> on the host every 45-135 seconds during normal operation.
> 
> The kernel.org driver documentation confirms:
> "Power Save Mode, It's implemented but buggy"
> 
> Three changes:
> - fw.c: remove CARL9170FW_PSM and CARL9170FW_FIXED_5GHZ_PSM from
>    firmware capability bitmask so the driver never enables PS
> - rf.c: rf_psm() early return — never power down ADDAC/synthesizer
> - hostif.c: accept but ignore CARL9170_CMD_PSM commands gracefully
> 
> With PSM disabled, the adapter stays fully responsive on USB.
> Tested: 0 crashes in 180s (previously every 45-135s). The host
> cannot force PS on even via iw set power_save on since the
> firmware no longer advertises the capability.

I have an idea here: Can you please add a new
config option (i.e. CARL9170FW_POWERSAVEMANAGEMENT),
let it depend on CARL9170FW_BROKEN_FEATURES (default n)
and then #ifdef / #endif around the feature flags and
make a rf_psm() stub when that feature isn't set?

Thanks
Christian

> Signed-off-by: Masi Osmani <mas-i@hotmail.de>
> ---
>   carlfw/src/fw.c     | 6 ++++--
>   carlfw/src/hostif.c | 4 ++--
>   carlfw/src/rf.c     | 7 +++++++
>   3 files changed, 13 insertions(+), 4 deletions(-)
> 
> --- a/carlfw/src/fw.c	2026-03-16 23:38:46.184137155 +0100
> +++ b/carlfw/src/fw.c	2026-03-16 23:38:59.714232929 +0100
> @@ -48,8 +48,10 @@ const struct carl9170_firmware_descripto
>   #endif /* CONFIG_CARL9170FW_USB_DOWN_STREAM */
>   #ifdef CONFIG_CARL9170FW_RADIO_FUNCTIONS
>   					BIT(CARL9170FW_COMMAND_PHY) |
> -					BIT(CARL9170FW_PSM) |
> -					BIT(CARL9170FW_FIXED_5GHZ_PSM) |
> +					/*
> +					 * PSM capability removed — firmware
> +					 * PS causes USB command timeouts.
> +					 */
>   #endif /* CONFIG_CARL9170FW_RADIO_FUNCTIONS */
>   #ifdef CONFIG_CARL9170FW_SECURITY_ENGINE
>   					BIT(CARL9170FW_COMMAND_CAM) |
> --- a/carlfw/src/rf.c	2026-03-16 23:38:46.188101929 +0100
> +++ b/carlfw/src/rf.c	2026-03-16 23:39:12.970421845 +0100
> @@ -237,6 +237,13 @@ void rf_psm(void)
>   {
>   	u32 bank3;
>   
> +	/*
> +	 * PSM disabled — powering down ADDAC/synthesizer causes the
> +	 * SH-2 to miss USB command responses, triggering host-side
> +	 * -ETIMEDOUT and device crash. Always stay awake.
> +	 */
> +	return;
> +
>   	if (fw.phy.psm.state == CARL9170_PSM_SOFTWARE) {
>   		/* not enabled by the driver */
>   		return;
> --- a/carlfw/src/hostif.c	2026-03-16 23:38:46.192102245 +0100
> +++ b/carlfw/src/hostif.c	2026-03-16 23:39:27.262628301 +0100
> @@ -285,9 +285,9 @@ void handle_cmd(struct carl9170_rsp *res
>   		break;
>   
>   	case CARL9170_CMD_PSM:
> +		/* PSM commands accepted but ignored — PS is disabled
> +		 * to prevent USB command timeout crashes. */
>   		resp->hdr.len = 0;
> -		fw.phy.psm.state = le32_to_cpu(cmd->psm.state);
> -		rf_psm();

Hmm this removal of rf_psm() seems to be a leftover then.. since you
added the return; in the function too?

>   		break;
>   #endif /* CONFIG_CARL9170FW_RADIO_FUNCTIONS */
>