netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael Büsch" <m@bues.ch>
To: Lucas Stach <dev@lynxeye.de>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
	b43-dev@lists.infradead.org
Subject: Re: [PATCH RFC] b43: stop hardcoding LED behavior
Date: Mon, 25 Apr 2016 20:32:05 +0200	[thread overview]
Message-ID: <20160425203205.46f6774b@wiggum> (raw)
In-Reply-To: <1461608496.2364.36.camel@lynxeye.de>

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

On Mon, 25 Apr 2016 20:21:36 +0200
Lucas Stach <dev@lynxeye.de> wrote:

> > Numbers please. Did you measure that is actually causes more
> > _wakeups_?
> > How many?
> > The led work is placed in the mac80211 workqueue and LED updates only
> > happen on behalf of mac80211 activities (by default). It only causes
> > additional wakeups, if there's nothing else scheduled on the
> > workqueue
> > anyways (which might well be the case. So we need numbers. :)
> >   
> The blinking LEDs use a timer to enforce a constant blink rate at a
> 50ms on/off interval. While they are only triggered if there is some
> RX/TX activity in the system, they cause up to 20 wakeups/second/led.
> As the timers used for LED activity aren't deferrable, this hardcode is
> causing 40 unnecessary CPU wakeups/s in my system.


Ok this is 40 to 40k for the interrupt requests?
We need real measured numbers and a percentage on how much the b43 LEDs
increase the system wakeups in relation to all other wakeups.


> There are some people that find those kinds of blinking LEDs
> distracting,


Those can already disable them via the standard LED framework.


> so a module parameter to disable them altogether might be
> a good thing to have.


No. We have a standard API for this.


> Causing CPU wakeups in a system where those LEDs
> aren't even physically populated is clearly undesired behavior.


Yes, but this is not going to be fixed via regressions.


> If checking that the SPROM doesn't define any LED behavior is enough to
> not regress your use case, I would be glad to rework the patch
> accordingly.


As it turns out I don't have that card here and I don't have a dump of
its SPROM as I expected. So I cannot really verify this. But I'm pretty
sure that this card did not define any LEDs in its SPROM at all.
I'm not aware of any card that only partially defines LEDs in the
SPROM. So that fix would be OK.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

      reply	other threads:[~2016-04-25 18:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-25  7:40 [PATCH RFC] b43: stop hardcoding LED behavior Lucas Stach
     [not found] ` <1461570051-3950-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
2016-04-25 15:53   ` Michael Büsch
2016-04-25 18:21     ` Lucas Stach
2016-04-25 18:32       ` Michael Büsch [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160425203205.46f6774b@wiggum \
    --to=m@bues.ch \
    --cc=b43-dev@lists.infradead.org \
    --cc=dev@lynxeye.de \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).