Re: [PATCH 17/17] staging: speakup: remove custom locking macro definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux