Re: [PATCH 1/2] external interrupts: abstraction layer

[Christoph Hellwig <hch@infradead.org>]

> > > +static int extint_counter_open(struct inode *inode, struct file *filp)
> > > +{
> > > +	struct extint_device *ed = file_to_extint_device(filp);
> > 
> > you don't need the file but just the inode (strictly speaking the cdev),
> > and doing this based on the file is rather confusing to the reader because
> > the struct file passed to ->open has just been allocated.
> 
> Perhaps I'm not following something here.
> 
> This behavior enables using poll/select to detect or wait for the
> external interrupt counter (which is per-device) to change.  The
> "has changed" status must be on a per-open basis, not a per-device
> basis, as seperate processes may read the counter at different times.

Sorry, the whole behaviour is complety fine.  I just don't thing the name
and calling convention of file_to_extint_device is optimal.  It should
take an struct inode * and be called just to_extint_device or someting.
The above would become

	struct extint_device *ed = to_extint_device(filp->f_dentry->d_inode);

-
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/