Re: [PATCH 1/1] UIO: Add a write() function to enable/disable interrupts

["Hans J. Koch" <hjk@linutronix.de>]

Am Fri, 23 May 2008 13:20:50 +0100
schrieb "Tom Spink" <tspink@gmail.com>:

> 2008/5/23 Hans J. Koch <hjk@linutronix.de>:
> > Am Fri, 23 May 2008 13:00:17 +0100
> > schrieb "Tom Spink" <tspink@gmail.com>:
> >
> >> My initial idea was just a thought anyway, just to
> >> maintain a bit of extensibility if .write is ever needed for
> >> something else. :-)
> >
> > Hi Tom,
> > thanks for your contribution, but for me it's just the other way
> > round: I'm glad write() gets a defined purpose before people do
> > something stupid with it. It's good to remember that all data
> > exchange with the device has to be done through the mapped memory.
> > If this is not possible, the hardware is no candidate for a UIO
> > driver.
> >
> > BTW, I wait for the first UIO driver which abuses this write()
> > function to write many different values to trigger different
> > actions. I wonder if I should restrict write() to the value 0 and
> > 1...
> >
> > Thanks,
> > Hans
> 
> Hi Hans,
> 
> Thanks for your explanation.  Another thing, I noticed then, is that
> in your return statement, you blindly return the the value of
> irqcontrol if it's non-zero, and if it's zero, then the length of the
> data written.  However, if irqcontrol returns a value that's > 0, it
> could potentially confuse writers.  I guess it's up to the implementer
> of irqcontrol to ensure they stick to -EXXX and 0

True. And it's not even worth fixing this by checking retval<0, because
if irqcontrol returns values>0, it's broken anyway. It is supposed to
return zero if successful, and negative errors otherwise. This is
common practice in the kernel, and people frequently use if (ret)... to
detect errors without checking if ret is a legal negative error number.

Thanks,
Hans