2018-04-11 18:42:09

by Roman Kiryanov

[permalink] [raw]
Subject: [PATCH] ANDROID: mac80211_hwsim: support/ignore power state changes

From: Bjoern Johansson <[email protected]>

Indicate support for power state changes and handle them by calling an
empty function.
The important part is "ieee80211_hw_set(hw, SUPPORTS_PS);" at the
bottom of the diff. Without this upper layers in the kernel will return an
error code when trying to set the power state because the driver doesn't
indicate power state support. This in turn causes VTS
(Android Vendor Test Suite) failures because the WiFi HAL can't enable
power saving mode. The remaining code is just there to deal with the
incoming state change request.

Signed-off-by: Bjoern Johansson <[email protected]>
Signed-off-by: Lingfeng Yang <[email protected]>
Signed-off-by: Roman Kiryanov <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 6afe896e5cb8..ff4741188814 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1637,6 +1637,11 @@ static const char * const hwsim_chanwidths[] = {
[NL80211_CHAN_WIDTH_160] = "vht160",
};

+static void mac80211_power_state_changed(bool enabled)
+{
+ /* TODO: Do something when the power state changes */
+}
+
static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
{
struct mac80211_hwsim_data *data = hw->priv;
@@ -1649,6 +1654,12 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
};
int idx;

+ if (changed & IEEE80211_CONF_CHANGE_PS) {
+ bool enabled = (conf->flags & IEEE80211_CONF_PS) != 0;
+
+ mac80211_power_state_changed(enabled);
+ }
+
if (conf->chandef.chan)
wiphy_dbg(hw->wiphy,
"%s (freq=%d(%d - %d)/%s idle=%d ps=%d smps=%s)\n",
@@ -2650,7 +2661,9 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
ieee80211_hw_set(hw, AMPDU_AGGREGATION);
ieee80211_hw_set(hw, MFP_CAPABLE);
ieee80211_hw_set(hw, SIGNAL_DBM);
+ ieee80211_hw_set(hw, SUPPORTS_PS);
ieee80211_hw_set(hw, TDLS_WIDER_BW);
+
if (rctbl)
ieee80211_hw_set(hw, SUPPORTS_RC_TABLE);

--
2.17.0.484.g0c8726318c-goog


2018-04-23 01:26:29

by kernel test robot

[permalink] [raw]
Subject: [lkp-robot] [ANDROID] c3e497719d: hwsim.p2p_set.fail


FYI, we noticed the following commit (built with gcc-7):

commit: c3e497719da926c624ef31a642fd16f877911896 ("ANDROID: mac80211_hwsim: support/ignore power state changes")
url: https://github.com/0day-ci/linux/commits/rkir-google-com/ANDROID-mac80211_hwsim-support-ignore-power-state-changes/20180413-061634
base: https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git master

in testcase: hwsim
with following parameters:

group: hwsim-10



on test machine: 8 threads Intel(R) Core(TM) i7-2600K CPU @ 3.40GHz with 8G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


START p2p_set 1/1
Test: P2P_SET commands
Invalid P2P_SET accepted: ps 1
Traceback (most recent call last):
File "./run-tests.py", line 455, in main
t(dev)
File "/lkp/benchmarks/hwsim/tests/hwsim/test_p2p_set.py", line 38, in test_p2p_set
raise Exception("Invalid P2P_SET accepted: " + cmd)
Exception: Invalid P2P_SET accepted: ps 1
FAIL p2p_set 0.029139 2018-04-19 10:23:03.608924


Thanks,
Xiaolong


Attachments:
(No filename) (1.02 kB)
config-4.16.0-rc6-01764-gc3e4977 (162.08 kB)
job-script (4.45 kB)
kmsg.xz (296.44 kB)
hwsim (132.98 kB)
job.yaml (3.82 kB)
reproduce (8.33 kB)
Download all attachments

2018-04-12 04:23:42

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ANDROID: mac80211_hwsim: support/ignore power state changes

[email protected] writes:

> From: Bjoern Johansson <[email protected]>
>
> Indicate support for power state changes and handle them by calling an
> empty function.
> The important part is "ieee80211_hw_set(hw, SUPPORTS_PS);" at the
> bottom of the diff. Without this upper layers in the kernel will return an
> error code when trying to set the power state because the driver doesn't
> indicate power state support. This in turn causes VTS
> (Android Vendor Test Suite) failures because the WiFi HAL can't enable
> power saving mode. The remaining code is just there to deal with the
> incoming state change request.
>
> Signed-off-by: Bjoern Johansson <[email protected]>
> Signed-off-by: Lingfeng Yang <[email protected]>
> Signed-off-by: Roman Kiryanov <[email protected]>

Why the odd "ANDROID:" prefix?

--
Kalle Valo

2018-04-12 17:04:33

by Roman Kiryanov

[permalink] [raw]
Subject: Re: [PATCH] ANDROID: mac80211_hwsim: support/ignore power state changes

Hi Kalle,

thank you for reviewing our patch. I am ok with applying this patch without
"ANDROID:" prefix. Do you want me to send an updated patch?

Regards,
Roman.

On Wed, Apr 11, 2018 at 9:23 PM Kalle Valo <[email protected]> wrote:

> [email protected] writes:

> > From: Bjoern Johansson <[email protected]>
> >
> > Indicate support for power state changes and handle them by calling an
> > empty function.
> > The important part is "ieee80211_hw_set(hw, SUPPORTS_PS);" at the
> > bottom of the diff. Without this upper layers in the kernel will return
an
> > error code when trying to set the power state because the driver doesn't
> > indicate power state support. This in turn causes VTS
> > (Android Vendor Test Suite) failures because the WiFi HAL can't enable
> > power saving mode. The remaining code is just there to deal with the
> > incoming state change request.
> >
> > Signed-off-by: Bjoern Johansson <[email protected]>
> > Signed-off-by: Lingfeng Yang <[email protected]>
> > Signed-off-by: Roman Kiryanov <[email protected]>

> Why the odd "ANDROID:" prefix?

> --
> Kalle Valo

2018-04-12 19:15:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] ANDROID: mac80211_hwsim: support/ignore power state changes

On Thu, 2018-04-12 at 17:04 +0000, Roman Kiryanov wrote:
> Hi Kalle,
>
> thank you for reviewing our patch. I am ok with applying this patch without
> "ANDROID:" prefix. Do you want me to send an updated patch?

I can fix it up.

johannes