2020-10-30 08:51:12

by Andy Huang

[permalink] [raw]
Subject: [PATCH v2 1/4] rtw88: sync the power state between driver and firmware

From: Chin-Yen Lee <[email protected]>

In current flow, driver issues power requests to firmware for entering or
leaving deep power mode. But driver does not wait for an ack from firmware
via reading CPWM register when driver requests to enter deep power mode.
The behavior will lead to unsynchronized state between firmware and driver.
Furthermore, consecutive same power requests may confuse firmware and leads
to unexpected value of CPWM register. This patch ensures that the power
request will wait for an ack from firmware and only send one power request
each time.

Signed-off-by: Chin-Yen Lee <[email protected]>
Signed-off-by: Tzu-En Huang <[email protected]>
---
drivers/net/wireless/realtek/rtw88/ps.c | 69 +++++++++++--------------
1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 7a189a9926fe..8ed243628dcb 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -68,48 +68,39 @@ int rtw_leave_ips(struct rtw_dev *rtwdev)
void rtw_power_mode_change(struct rtw_dev *rtwdev, bool enter)
{
u8 request, confirm, polling;
- u8 polling_cnt;
- u8 retry_cnt = 0;
-
- for (retry_cnt = 0; retry_cnt < 3; retry_cnt++) {
- request = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr);
- confirm = rtw_read8(rtwdev, rtwdev->hci.cpwm_addr);
-
- /* toggle to request power mode, others remain 0 */
- request ^= request | BIT_RPWM_TOGGLE;
- if (!enter) {
- request |= POWER_MODE_ACK;
- } else {
- request |= POWER_MODE_LCLK;
- if (rtw_fw_lps_deep_mode == LPS_DEEP_MODE_PG)
- request |= POWER_MODE_PG;
- }
-
- rtw_write8(rtwdev, rtwdev->hci.rpwm_addr, request);
-
- if (enter)
- return;
+ int ret;

- /* check confirm power mode has left power save state */
- for (polling_cnt = 0; polling_cnt < 50; polling_cnt++) {
- polling = rtw_read8(rtwdev, rtwdev->hci.cpwm_addr);
- if ((polling ^ confirm) & BIT_RPWM_TOGGLE)
- return;
- udelay(100);
- }
-
- /* in case of fw/hw missed the request, retry */
- rtw_warn(rtwdev, "failed to leave deep PS, retry=%d\n",
- retry_cnt);
+ request = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr);
+ confirm = rtw_read8(rtwdev, rtwdev->hci.cpwm_addr);
+
+ /* toggle to request power mode, others remain 0 */
+ request ^= request | BIT_RPWM_TOGGLE;
+ if (enter) {
+ request |= POWER_MODE_LCLK;
+ if (rtw_fw_lps_deep_mode == LPS_DEEP_MODE_PG)
+ request |= POWER_MODE_PG;
}
+ /* Each request require an ack from firmware */
+ request |= POWER_MODE_ACK;

- /* Hit here means that driver failed to change hardware power mode to
- * active state after retry 3 times. If the power state is locked at
- * Deep sleep, most of the hardware circuits is not working, even
- * register read/write. It should be treated as fatal error and
- * requires an entire analysis about the firmware/hardware
- */
- WARN(1, "Hardware power state locked\n");
+ rtw_write8(rtwdev, rtwdev->hci.rpwm_addr, request);
+
+ /* Check firmware get the power requset and ack via cpwm register */
+ ret = read_poll_timeout_atomic(rtw_read8, polling,
+ (polling ^ confirm) & BIT_RPWM_TOGGLE,
+ 100, 15000, true, rtwdev,
+ rtwdev->hci.cpwm_addr);
+ if (ret) {
+ /* Hit here means that driver failed to get an ack from firmware.
+ * The reason could be that hardware is locked at Deep sleep,
+ * so most of the hardware circuits are not working, even
+ * register read/write; or firmware is locked in some state and
+ * cannot get the request. It should be treated as fatal error
+ * and requires an entire analysis about the firmware/hardware.
+ */
+ WARN(1, "firmware failed to ack driver for %s Deep Power mode\n",
+ enter ? "entering" : "leaving");
+ }
}
EXPORT_SYMBOL(rtw_power_mode_change);

--
2.17.1


2020-11-07 15:52:23

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] rtw88: sync the power state between driver and firmware

<[email protected]> wrote:

> From: Chin-Yen Lee <[email protected]>
>
> In current flow, driver issues power requests to firmware for entering or
> leaving deep power mode. But driver does not wait for an ack from firmware
> via reading CPWM register when driver requests to enter deep power mode.
> The behavior will lead to unsynchronized state between firmware and driver.
> Furthermore, consecutive same power requests may confuse firmware and leads
> to unexpected value of CPWM register. This patch ensures that the power
> request will wait for an ack from firmware and only send one power request
> each time.
>
> Signed-off-by: Chin-Yen Lee <[email protected]>
> Signed-off-by: Tzu-En Huang <[email protected]>

4 patches applied to wireless-drivers-next.git, thanks.

690390d841d2 rtw88: sync the power state between driver and firmware
a95949606828 rtw88: store firmware feature in firmware header
f31e039fab38 rtw88: add C2H response for checking firmware leave lps
fc3ac64a3a28 rtw88: decide lps deep mode from firmware feature.

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches