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:4.239 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site