On Thu, 2021-12-09 at 09:20 +0000, Ajay.Kathat@xxxxxxxxxxxxx wrote: > On 09/12/21 00:20, David Mosberger-Tang wrote: > The "chip_wakeup" & "chip_allow_sleep" API's are required to configure > the wake-up registers in wilc. These registers must be configured > correctly when the power save mode is enabled. I see. > > Here is what I see: on a system configured for minimum power consumption > > (all unnecessary daemons disabled, Ethernet unplugged) I measured 1,180 mW > > when the WILC chip is in RESET (the ENABLE pin is always high on our > > platform). > > > > With the wilc1000-spi/wilc1000 modules loaded and the WILC chip > > running but without being associated with a WLAN network, measured > > power consumption was 1,460 mW, regardless of whether chip sleep was > > enabled or disabled. > > Did you test by enabling the power-save mode with "wpa_supplicant" or > using "iw" command. For power consumption measurement, please try by > disabling the PSM mode. No, I have not played with power-saving mode. There are some disconcerting messages when turning PSM on: [ 1764.070375] wilc1000_spi spi1.0: Failed cmd, cmd (ca), resp (00) [ 1764.076403] wilc1000_spi spi1.0: Failed cmd, read reg (000013f4)... The driver does still work, but occasionally, I get long streams of these messages, sometimes it gets to the point where there are so many that the watchdog timer ends up rebooting the system. > The chip-sleep code is needed to configure the registers properly for > wilc and not using these sequence would have impact on sending the data > to wilc, especially when the PS mode is enabled. How about something along the patch below then? With this patch, I see the following iperf3 performance (120 second average): PSM on: [ 4] 0.00-120.00 sec 47.6 MBytes 3.33 Mbits/sec 0 sender PSM off: [ 4] 0.00-120.00 sec 201 MBytes 14.0 Mbits/sec 12 sender In terms of power consumption: baseline : 1.2 W PSM on, idle : 1.2 W PSM on, TX @ 3.33 Mbps : 1.7 W PSM off, idle : 1.5 W PSM off, TX @ 14.0 Mbps : 2.4 W --david
diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c index 44e93cd5e3df..09aa5ced19d3 100644 --- a/drivers/net/wireless/microchip/wilc1000/hif.c +++ b/drivers/net/wireless/microchip/wilc1000/hif.c @@ -1912,6 +1912,7 @@ int wilc_edit_station(struct wilc_vif *vif, const u8 *mac, int wilc_set_power_mgmt(struct wilc_vif *vif, bool enabled, u32 timeout) { + struct wilc *wilc = vif->wilc; struct wid wid; int result; s8 power_mode; @@ -1927,6 +1928,8 @@ int wilc_set_power_mgmt(struct wilc_vif *vif, bool enabled, u32 timeout) result = wilc_send_config_pkt(vif, WILC_SET_CFG, &wid, 1); if (result) netdev_err(vif->ndev, "Failed to send power management\n"); + else + wilc->power_save_mode = enabled; return result; } diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h index 152408232d51..db149abc5d0d 100644 --- a/drivers/net/wireless/microchip/wilc1000/netdev.h +++ b/drivers/net/wireless/microchip/wilc1000/netdev.h @@ -267,6 +267,7 @@ struct wilc { int clients_count; struct workqueue_struct *hif_workqueue; + bool power_save_mode; enum chip_ps_states chip_ps_state; struct wilc_cfg cfg; void *bus_data; diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c index ddd382996275..0e0e325d396e 100644 --- a/drivers/net/wireless/microchip/wilc1000/wlan.c +++ b/drivers/net/wireless/microchip/wilc1000/wlan.c @@ -18,13 +18,13 @@ static inline bool is_wilc1000(u32 id) static inline void acquire_bus(struct wilc *wilc, enum bus_acquire acquire) { mutex_lock(&wilc->hif_cs); - if (acquire == WILC_BUS_ACQUIRE_AND_WAKEUP) + if (acquire == WILC_BUS_ACQUIRE_AND_WAKEUP && wilc->power_save_mode) chip_wakeup(wilc); } static inline void release_bus(struct wilc *wilc, enum bus_release release) { - if (release == WILC_BUS_RELEASE_ALLOW_SLEEP) + if (release == WILC_BUS_RELEASE_ALLOW_SLEEP && wilc->power_save_mode) chip_allow_sleep(wilc); mutex_unlock(&wilc->hif_cs); }