Messages in this thread | | | Date | Fri, 12 Sep 2008 04:33:32 -0400 | From | Christoph Hellwig <> | Subject | Re: [PATCH 01/18] lirc core device driver infrastructure |
| |
On Fri, Sep 12, 2008 at 02:16:30AM +0200, Janne Grunau wrote: > I'm pretty sure LIRC will be recognized by its current users, it is > probably enough if it is in the help text though.
But we don't care just for your current users :) Kernel features / drivers do need good descriptions.
> > > + } else { > > > + /* if device not opened so we can sleep half a second */ > > > + set_current_state(TASK_INTERRUPTIBLE); > > > + schedule_timeout(HZ/2); > > > + } > > > > Yikes. This should use some form of more fine-grained wakeus. > > added a waitqueue and wait_event
I don't think you'll need a wait queue - you can just call wake_up_process on ir->task while the thread just sleeps in schedule().
> > > +int lirc_unregister_plugin(int minor) > > > > Why doesn't this one take a struct lirc_plugin pointer? > > I don't know, It doesn't really help though since the struct lirc_plugin > is copied by value to irctl.p in lirc_register_plugin.
Well, the lifetime rules for these structures seems a litte odd. I would except this to work like:
struct lirc_plugin statically allocated by the driver, then passed into lirc_register_plugin, which dynamically allocates an irdev. The irdev would point to the plugin, not copy it.
And btw, any chance for an s/plugin/driver/g - that's the terminology we use everywhere else in the kernel.
| |