lkml.org 
[lkml]   [2007]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC] rfkill - Add support for input key to control wireless radio
Date
From
Hi,

> > + /*
> > + * Pointer to rfkill structure
> > + * that was filled in by key driver.
> > + */
> > + struct rfkill *rfkill;
>
> Since rfkill is basically a function pointer table,
> can it be made const?

Sounds good to me.

> > + /*
> > + * Once key status change has been detected, the toggled
> > + * field should be set to indicate a notification to
> > + * user or driver should be performed.
> > + */
> > + int toggled;
> > +
> > + /*
> > + * Current state of the device radio, this state will
> > + * change after the radio has actually been toggled since
> > + * receiving the radio key event.
> > + */
> > + int radio_status;
> > +
> > + /*
> > + * Current status of the key which controls the radio,
> > + * this value will change after the key state has changed
> > + * after polling, or the key driver has send the new state
> > + * manually.
> > + */
> > + int key_status;
>
>
> Maybe turn these bits into a bit values (set_bit/clear_bit) in an unsigned long.

Will do.

> > + /*
> > + * Input device for this key,
> > + * we also keep track of the number of
> > + * times this input device is open. This
> > + * is important for determining to whom we
> > + * should report key events.
> > + */
> > + struct input_dev *input;
> > + unsigned int open_count;
>
> atomic on open_count?

There seems to have gone something wrong with the patch,
latest version should have had this field removed.

> > + /*
> > + * Name of this radio type.
> > + */
> > + char *name;
>
> const?

Will do.

> > + /*
> > + * All access to the master structure
> > + * and its children (the keys) are protected
> > + * by this key lock.
> > + */
> > + struct semaphore key_sem;
>
> mutex instead of semaphort

Strange, this should have already be fixed. :S

> > + /*
> > + * Work structures for periodic polling,
> > + * as well as the scheduled radio toggling.
> > + */
> > + struct work_struct toggle_work;
> > + struct work_struct poll_work;
>
> delayed_rearming_work instead?

Same here, rfkill should already have the new workqueue api...

I'll resubmit this within a few moments.

Thanks

Ivo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-01-31 11:43    [W:0.432 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site