Messages in this thread | | | Date | Sat, 24 May 2008 02:02:12 +0200 | From | "Leon Woestenberg" <> | Subject | Re: [PATCH 1/1] UIO: Add a write() function to enable/disable interrupts |
| |
Hello,
On Sat, May 24, 2008 at 12:43 AM, Hans J. Koch <hjk@linutronix.de> wrote: > On Fri, May 23, 2008 at 10:44:42PM +0200, Leon Woestenberg wrote: >> >> Shouldn't this be more future-proof, what if we need to abuse write() >> for something else in the future? > > We don't. I'm thinking about letting the function fail if irq_on is not > 0 or 1, just to stop any ideas of abusing write(). > We don't want to be future-proof?
With kernel UIO and userspace driver in seperate source repositories, expect serious API drift in the longer term. I.e. the UIO interface must be backwards and forwards proof IMHO.
> read() and write() only deal with irq handling, all data exchange with the > device is done through mapped memory. > *Currently*, read() and write() only deal with irq handling.
In the future you might want to add a second control. I cannot think of what that should be now, much like it was not foreseen a write() call was needed.
>> I would suggest a check for ppos to be 0 (zero) as well, just to be >> sure and future-proof and backwards-safe. > > write() is only for enabling/disabling irqs, there's only one possible > value of count, and we don't have a seek function. So why check ppos? > So that *if* we have a second write()able location (again, for something I cannot foresee now), you at least check that the userspace proper wants to enable/disable the interrupt.
AFAIK, POSIX pwrite() does not require a seek() implementation in the driver, but will come in with a different ppos.
Idea and patch looks fine, I just wanted to bring this up so that it is considered.
Regards, -- Leon
| |