Message-ID: <e7247265d5309165140d7a9a3af646129a789d58.camel@egauge.net>
Subject: Re: [PATCH] wilc1000: Allow setting power_save before driver is
 initialized
From:   David Mosberger-Tang <davidm@egauge.net>
To:     Ajay.Kathat@microchip.com
Cc:     Claudiu.Beznea@microchip.com, kvalo@codeaurora.org,
        davem@davemloft.net, kuba@kernel.org,
        linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Fri, 24 Dec 2021 10:38:37 -0700
In-Reply-To: <9272b86e-61ab-1c25-0efb-3cdd2c590db8@microchip.com>
References: <20211212011835.3719001-1-davidm@egauge.net>
         <6fc9f00aa0b0867029fb6406a55c1e72d4c13af6.camel@egauge.net>
         <5378e756-8173-4c63-1f0d-e5836b235a48@microchip.com>
         <31d5e7447e4574d0fcfc46019d7ca96a3db4ecb6.camel@egauge.net>
         <49a5456d-6a63-652e-d356-9678f6a9b266@microchip.com>
         <523698d845e0b235e4cbb2a0f3cfaa0f5ed98ec0.camel@egauge.net>
         <122f79b7-7936-325c-b2d9-e15db6642d0f@microchip.com>
         <be3c95c8310504222e88c602a937b7f05cc01286.camel@egauge.net>
         <9272b86e-61ab-1c25-0efb-3cdd2c590db8@microchip.com>
Organization: eGauge Systems LLC
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

Ajay,

On Fri, 2021-12-24 at 16:20 +0000, Ajay.Kathat@microchip.com wrote:
> On 23/12/21 22:38, David Mosberger-Tang wrote:
> > First, on a freshly booted system and with wilc1000-spi autoloaded by
> > the kernel, try this sequence (copy & paste the commands):
> > 
> >     /usr/sbin/wpa_supplicant -Bs -iwlan0 -c/etc/wpa_supplicant.conf
> >     sleep 10
> >     iw dev wlan0 set power_save on
> > 
> > The above yields a power consumption of 1.4W reliably.  The "sleep 10"
> > doesn't matter here; the behavior is the same with or without it.  I
> > tried waiting up to 120 seconds with no difference.
> 
> I have tested by making the WILC as build-in module to insert driver 
> automatically at boot-up. I hope it should be fine. Because I have 
> already tested as loadable module earlier.
> 
> Below are the number observed
> ------------------------------ --------------------------
> - before starting wpa_supplicant             : ~16.3 mA
> - wpa_supplicant started                         : ~40 mA
> - PSM on                                                  :  ~6 mA
> 
> 
> The 'sleep 10' would have no impact in my setup because I have measured 
> the current consumption for wilc1000 chip.
> 
> I have shared the screenshot at https://postimg.cc/67S41dkb

Huh, that's curious.  I definitely cannot reproduce this.  To match
your setup as closely as possibly, I also built wilc1000-spi into the
kernel, but that makes no difference (as expected).

What kernel version are you on?  I switched to wireless-drivers-next as
of today (latest commit d430dffbe9dd30759f3c64b65bf85b0245c8d8ab).

With this kernel, the numbers are about 100mW lower than reported
before, but the relative behavior is the same: about 300mW higher
power-consumption when PSM is not taking effect properly.

To recap, back with wilc1000-spi being a module again, after freshly
booting the system and issuing this commands:

   /usr/sbin/wpa_supplicant -Bs -iwlan0 -c/etc/wpa_supplicant.conf
   /usr/sbin/iw dev wlan0 set power_save on

I see a power-consumption of about 1.25W.  PSM on/off makes no
difference in this state.  Then, if I issue the commands:

rmmod wilc1000-spi
modprobe wilc1000-spi
sleep 10
iw dev wlan0 set power_save on

power-consumption drops to about 0.9W.

Here is a screenshot that shows the annotated power-measurements:

   https://postimg.cc/3dbKSGht

Apart from kernel version, the only things that I can think of that'd
be different is that we don't have the ENABLE pin wired to a GPIO.
 Instead, the chip is always enabled.  I doubt this would explain the
difference (~RESET is wired to a GPIO).


  --david