[Sorry, my first email was accidentally had the patch appended as a MIME
attachment.]
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);
}
