linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivo van Doorn <ivdoorn@gmail.com>
To: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH] rfkill: add default global states
Date: Wed, 23 Jul 2008 20:28:49 +0200	[thread overview]
Message-ID: <200807232028.49239.IvDoorn@gmail.com> (raw)
In-Reply-To: <1216775046-9506-3-git-send-email-hmh@hmh.eng.br>

>  #endif /* __RFKILL_INPUT_H */
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index 0a78e32..eaaabbd 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -45,6 +45,8 @@ MODULE_PARM_DESC(default_state,
>  		 "Default initial state for all radio types, 0 = radio off");
>  
>  static enum rfkill_state rfkill_states[RFKILL_TYPE_MAX];
> +static enum rfkill_state rfkill_default_states[RFKILL_TYPE_MAX];
> +static unsigned long rfkill_states_lockdflt[BITS_TO_LONGS(RFKILL_TYPE_MAX)];

I have a slight distaste for using multiple arrays where each variable is a different
variable for the same object. Perhaps it is time to move it into a structure:

struct .. {
	enum rfkill_state current
	enum rfkill_state default
};

static struct .. rfkill_states[RFKILL_TYPE_MAX];

It probably won't optimize anything, but I think it looks a bit nicer.

Just a suggestion, if it easier to keep it as multiple arrays I would still ack it. ;)

>  	list_add_tail(&rfkill->node, &rfkill_list);
> @@ -689,6 +737,49 @@ void rfkill_unregister(struct rfkill *rfkill)
>  }
>  EXPORT_SYMBOL(rfkill_unregister);
>  
> +/**
> + * rfkill_set_default - set initial value for a switch type
> + * @type - the type of switch to set the default state of
> + * @state - the new default state for that group of switches
> + *
> + * Sets the initial state rfkill should use for a given type.  Only "soft"
> + * states are allowed, i.e. RFKILL_STATE_SOFT_BLOCKED and
> + * RFKILL_STATE_UNBLOCKED.
> + *
> + * This function is meant to be used by platform drivers for platforms that
> + * can save switch state across power down/reboot.

As sideinfo: I found a hardware bug in a broadcom rfkill key a few days ago
which would not work well with this function:

* laptop was powered on, HW rfkill was set to allow WiFi
* laptop halted, something about a missing power supply and a battery which was drained
	without any userspace tool checking the battery status. ;)
* laptop was powered on and booted,  HW rkfill decided to block WiFi.

In any case not really important for this patch, but just for information. ;)

>   * Rfkill module initialization/deinitialization.
>   */
> @@ -702,8 +793,8 @@ static int __init rfkill_init(void)
>  	    rfkill_default_state != RFKILL_STATE_UNBLOCKED)
>  		return -EINVAL;
>  
> -	for (i = 0; i < ARRAY_SIZE(rfkill_states); i++)
> -		rfkill_states[i] = rfkill_default_state;
> +	for (i = 0; i < ARRAY_SIZE(rfkill_default_states); i++)
> +		rfkill_default_states[i] = rfkill_default_state;

rfkill_states[] is now no longer initialized, I guess that is a bad thing ;)

The rest of the patch looks good.

Ivo

  reply	other threads:[~2008-07-23 18:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-23  1:04 [GIT PATCH] RFC: next batch of rfkill changes Henrique de Moraes Holschuh
2008-07-23  1:04 ` [PATCH] rfkill: detect bogus double-registering Henrique de Moraes Holschuh
2008-07-23  3:41   ` Johannes Berg
2008-07-23 15:27     ` Henrique de Moraes Holschuh
2008-07-23 16:30       ` Johannes Berg
2008-07-23 17:41         ` Henrique de Moraes Holschuh
2008-07-23 18:12           ` Ivo van Doorn
2008-07-23  1:04 ` [PATCH] rfkill: add default global states Henrique de Moraes Holschuh
2008-07-23 18:28   ` Ivo van Doorn [this message]
2008-07-23 18:42     ` Henrique de Moraes Holschuh
2008-07-23 19:20       ` Ivo van Doorn
2008-07-23  1:04 ` [PATCH] rfkill: add master_switch_mode functionality Henrique de Moraes Holschuh
2008-07-23 18:39   ` Ivo van Doorn
2008-07-23 19:37     ` Henrique de Moraes Holschuh
2008-07-23  1:04 ` [PATCH] rfkill: add EPO lock to rfkill-input Henrique de Moraes Holschuh
2008-07-23 18:44   ` Ivo van Doorn
2008-07-23 19:01     ` Henrique de Moraes Holschuh
2008-07-23 19:28       ` Ivo van Doorn
2008-07-23  1:04 ` [PATCH] rfkill: rename rfkill_mutex to rfkill_global_mutex Henrique de Moraes Holschuh
2008-07-23 18:44   ` Ivo van Doorn
2008-07-23  1:04 ` [PATCH] rfkill: rate-limit rfkill-input workqueue usage Henrique de Moraes Holschuh
2008-07-23 18:46   ` Ivo van Doorn
2008-07-23 19:43     ` Dmitry Torokhov
2008-07-23 20:27       ` Henrique de Moraes Holschuh
2008-07-23 20:39         ` Dmitry Torokhov
2008-07-23  1:12 ` [GIT PATCH] RFC: next batch of rfkill changes Henrique de Moraes Holschuh
2008-07-23 18:08 ` Ivo van Doorn
2008-07-23 19:09   ` Henrique de Moraes Holschuh
2008-08-01 18:11 ` John W. Linville
2008-08-01 19:35   ` Henrique de Moraes Holschuh

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=200807232028.49239.IvDoorn@gmail.com \
    --to=ivdoorn@gmail.com \
    --cc=hmh@hmh.eng.br \
    --cc=linux-wireless@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).