On Mon, May 13, 2013 at 11:52:05AM -0500, William Hubbs wrote: > On Mon, May 13, 2013 at 07:28:41PM +0300, Dan Carpenter wrote: > > On Mon, May 13, 2013 at 11:17:30AM -0500, William Hubbs wrote: > > > On Mon, May 13, 2013 at 10:50:25AM +0300, Dan Carpenter wrote: > > > > On Mon, May 13, 2013 at 12:03:09AM -0500, William Hubbs wrote: > > > > > Signed-off-by: William Hubbs <w.d.hubbs@xxxxxxxxx> > > > > > --- > > > > > drivers/staging/speakup/spk_priv.h | 12 ------------ > > > > > 1 file changed, 12 deletions(-) > > > > > > > > > > diff --git a/drivers/staging/speakup/spk_priv.h b/drivers/staging/speakup/spk_priv.h > > > > > index 141125c..637ba67 100644 > > > > > --- a/drivers/staging/speakup/spk_priv.h > > > > > +++ b/drivers/staging/speakup/spk_priv.h > > > > > @@ -77,16 +77,4 @@ extern struct speakup_info_t speakup_info; > > > > > > > > > > extern struct var_t synth_time_vars[]; > > > > > > > > > > -/* Protect the whole speakup machinery, must be taken at each kernel->speakup > > > > > - * transition and released at all corresponding speakup->kernel transitions > > > > > - * (flags must be the same variable between lock/trylock and unlock). > > > > > - * > > > > > - * The progression thread only interferes with the speakup machinery through > > > > > - * the synth buffer, and so only needs to take the lock while tinkering with > > > > > - * it. > > > > > - */ > > > > > -/* Speakup needs to disable the keyboard IRQ, hence _irqsave/restore */ > > > > > > > > These comments are nice. The should go next to the speakup_info > > > > definition in drivers/staging/speakup/synth.c. > > > > > > What about in drivers/staging/speakup/spk_types.h since that is where > > > struct speakup_info_t is defined? > > > > > > > My thinking was that it was documenting the speakup_info object > > itself not the type. > > It is actually more specific than that. It documents the use of the > spinlock field inside the object. Where is the best spot to put it to > show that? Yeah. There is kernel doc format for describing structs or you could just put it inline next to the spinlock element. The thing is there is only one speakup_info_t object which is speakup_info. If we create more then we'd probably pull the lock outside the object by itself. So I see speakup_info.spinlock as the important thing rather than the type. But I don't feel strongly about this so long as it's documented somewhere. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel