[PATCH] rfkill-input: remove unused code

[PATCH] rfkill-input: remove unused code

[Johannes Berg]

There's a lot of rfkill-input code that cannot ever be
compiled and is useless until somebody needs and tests
it -- therefore remove it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/rfkill/rfkill-input.c |   69 ----------------------------------------------
 1 file changed, 69 deletions(-)

--- wireless-testing.orig/net/rfkill/rfkill-input.c	2009-03-27 13:14:54.000000000 +0100
+++ wireless-testing/net/rfkill/rfkill-input.c	2009-03-27 13:15:29.000000000 +0100
@@ -47,12 +47,6 @@ enum rfkill_global_sched_op {
 	RFKILL_GLOBAL_OP_UNBLOCK,
 };
 
-/*
- * Currently, the code marked with RFKILL_NEED_SWSET is inactive.
- * If handling of EV_SW SW_WLAN/WWAN/BLUETOOTH/etc is needed in the
- * future, when such events are added, that code will be necessary.
- */
-
 struct rfkill_task {
 	struct delayed_work dwork;
 
@@ -65,14 +59,6 @@ struct rfkill_task {
 	/* pending regular switch operations (1=pending) */
 	unsigned long sw_pending[BITS_TO_LONGS(RFKILL_TYPE_MAX)];
 
-#ifdef RFKILL_NEED_SWSET
-	/* set operation pending (1=pending) */
-	unsigned long sw_setpending[BITS_TO_LONGS(RFKILL_TYPE_MAX)];
-
-	/* desired state for pending set operation (1=unblock) */
-	unsigned long sw_newstate[BITS_TO_LONGS(RFKILL_TYPE_MAX)];
-#endif
-
 	/* should the state be complemented (1=yes) */
 	unsigned long sw_togglestate[BITS_TO_LONGS(RFKILL_TYPE_MAX)];
 
@@ -111,24 +97,6 @@ static void __rfkill_handle_global_op(en
 	}
 }
 
-#ifdef RFKILL_NEED_SWSET
-static void __rfkill_handle_normal_op(const enum rfkill_type type,
-			const bool sp, const bool s, const bool c)
-{
-	enum rfkill_state state;
-
-	if (sp)
-		state = (s) ? RFKILL_STATE_UNBLOCKED :
-			      RFKILL_STATE_SOFT_BLOCKED;
-	else
-		state = rfkill_get_global_state(type);
-
-	if (c)
-		state = rfkill_state_complement(state);
-
-	rfkill_switch_all(type, state);
-}
-#else
 static void __rfkill_handle_normal_op(const enum rfkill_type type,
 			const bool c)
 {
@@ -140,7 +108,6 @@ static void __rfkill_handle_normal_op(co
 
 	rfkill_switch_all(type, state);
 }
-#endif
 
 static void rfkill_task_handler(struct work_struct *work)
 {
@@ -171,21 +138,11 @@ static void rfkill_task_handler(struct w
 						i < RFKILL_TYPE_MAX) {
 				if (test_and_clear_bit(i, task->sw_pending)) {
 					bool c;
-#ifdef RFKILL_NEED_SWSET
-					bool sp, s;
-					sp = test_and_clear_bit(i,
-							task->sw_setpending);
-					s = test_bit(i, task->sw_newstate);
-#endif
 					c = test_and_clear_bit(i,
 							task->sw_togglestate);
 					spin_unlock_irq(&task->lock);
 
-#ifdef RFKILL_NEED_SWSET
-					__rfkill_handle_normal_op(i, sp, s, c);
-#else
 					__rfkill_handle_normal_op(i, c);
-#endif
 
 					spin_lock_irq(&task->lock);
 				}
@@ -238,32 +195,6 @@ static void rfkill_schedule_global_op(en
 	spin_unlock_irqrestore(&rfkill_task.lock, flags);
 }
 
-#ifdef RFKILL_NEED_SWSET
-/* Use this if you need to add EV_SW SW_WLAN/WWAN/BLUETOOTH/etc handling */
-
-static void rfkill_schedule_set(enum rfkill_type type,
-				enum rfkill_state desired_state)
-{
-	unsigned long flags;
-
-	if (rfkill_is_epo_lock_active())
-		return;
-
-	spin_lock_irqsave(&rfkill_task.lock, flags);
-	if (!rfkill_task.global_op_pending) {
-		set_bit(type, rfkill_task.sw_pending);
-		set_bit(type, rfkill_task.sw_setpending);
-		clear_bit(type, rfkill_task.sw_togglestate);
-		if (desired_state)
-			set_bit(type,  rfkill_task.sw_newstate);
-		else
-			clear_bit(type, rfkill_task.sw_newstate);
-		rfkill_schedule_ratelimited();
-	}
-	spin_unlock_irqrestore(&rfkill_task.lock, flags);
-}
-#endif
-
 static void rfkill_schedule_toggle(enum rfkill_type type)
 {
 	unsigned long flags;

Re: [PATCH] rfkill-input: remove unused code

[Henrique de Moraes Holschuh]

On Fri, 27 Mar 2009, Johannes Berg wrote:
> There's a lot of rfkill-input code that cannot ever be
> compiled and is useless until somebody needs and tests
> it -- therefore remove it.

Just so that you guys know, that code *is* tested :-)

I'd suggest that the code removal be done leaving a comment in place, like this:

/* Currently, code to implement EV_SW SW_WLAN/WWAN/BLUETOOTH/etc is not
 * in place since it was unused and was therefore removed.  Please look
 * at past history to check how it was supposed to be done, if you need
 * to implement handling for EV_SW.
 */

Because that codepath is _not_ the most obvious thing in the world to
reimplement without hitting a pitfall, so people better look how it was done
once in time...

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] rfkill-input: remove unused code

[Michael Buesch]

On Friday 27 March 2009 15:36:08 Henrique de Moraes Holschuh wrote:
> On Fri, 27 Mar 2009, Johannes Berg wrote:
> > There's a lot of rfkill-input code that cannot ever be
> > compiled and is useless until somebody needs and tests
> > it -- therefore remove it.
> 
> Just so that you guys know, that code *is* tested :-)
> 
> I'd suggest that the code removal be done leaving a comment in place, like this:
> 
> /* Currently, code to implement EV_SW SW_WLAN/WWAN/BLUETOOTH/etc is not
>  * in place since it was unused and was therefore removed.  Please look
>  * at past history to check how it was supposed to be done, if you need
>  * to implement handling for EV_SW.
>  */
> 
> Because that codepath is _not_ the most obvious thing in the world to
> reimplement without hitting a pitfall, so people better look how it was done
> once in time...
> 

In practice this stuff does bitrot.
So in my opinion it's _not_ a good idea to lookup some old code for implementing
a feature. Instead one should think about it and understand the stuff and then implement it.
And if there are pitfalls that are not obvious after one completely read the code, one
should consider a redesign.

-- 
Greetings, Michael.

Re: [PATCH] rfkill-input: remove unused code

[Johannes Berg]

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

On Fri, 2009-03-27 at 11:36 -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 27 Mar 2009, Johannes Berg wrote:
> > There's a lot of rfkill-input code that cannot ever be
> > compiled and is useless until somebody needs and tests
> > it -- therefore remove it.
> 
> Just so that you guys know, that code *is* tested :-)

At some point :)

> I'd suggest that the code removal be done leaving a comment in place, like this:
> 
> /* Currently, code to implement EV_SW SW_WLAN/WWAN/BLUETOOTH/etc is not
>  * in place since it was unused and was therefore removed.  Please look
>  * at past history to check how it was supposed to be done, if you need
>  * to implement handling for EV_SW.
>  */
> 
> Because that codepath is _not_ the most obvious thing in the world to
> reimplement without hitting a pitfall, so people better look how it was done
> once in time...

Works for me. Do we expect it to be needed though?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] rfkill-input: remove unused code

[Henrique de Moraes Holschuh]

On Fri, 27 Mar 2009, Michael Buesch wrote:
> So in my opinion it's _not_ a good idea to lookup some old code for implementing
> a feature. Instead one should think about it and understand the stuff and then implement it.
> And if there are pitfalls that are not obvious after one completely read the code, one
> should consider a redesign.

Or better comments in the code, because right now there are no comments
since the code was quite easy to read and understand, and it was
feature-complete (even if part of it was commented out due to lack of
users).

Well, I certainly don't fell too strongly about it.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] rfkill-input: remove unused code

[Johannes Berg]

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

On Fri, 2009-03-27 at 11:36 -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 27 Mar 2009, Johannes Berg wrote:
> > There's a lot of rfkill-input code that cannot ever be
> > compiled and is useless until somebody needs and tests
> > it -- therefore remove it.
> 
> Just so that you guys know, that code *is* tested :-)

At some point :)

> I'd suggest that the code removal be done leaving a comment in place, like this:
> 
> /* Currently, code to implement EV_SW SW_WLAN/WWAN/BLUETOOTH/etc is not
>  * in place since it was unused and was therefore removed.  Please look
>  * at past history to check how it was supposed to be done, if you need
>  * to implement handling for EV_SW.
>  */
> 
> Because that codepath is _not_ the most obvious thing in the world to
> reimplement without hitting a pitfall, so people better look how it was done
> once in time...

Done, will repost. Do we expect it to be needed though?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] rfkill-input: remove unused code

[Henrique de Moraes Holschuh]

On Fri, 27 Mar 2009, Johannes Berg wrote:
> > Because that codepath is _not_ the most obvious thing in the world to
> > reimplement without hitting a pitfall, so people better look how it was done
> > once in time...
> 
> Works for me. Do we expect it to be needed though?

Well, that depends.  Did anyone produce a device with a
switch/push-button labelled "bluetooth", "wimax", "uwb" (instead of
just using a normal key)?  Because if anyone does, we need EV_SW
SW_<whatever>, and that code to handle it in rfkill-input.

I *certainly* don't know of any such device, or I'd have asked for the
EV_SW SW_foo codepoints already, and uncommented that code :-)  People
like the "kill all radios" push-button/switch, but one for a specific
type of transmitter I haven't seen yet.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh