Re: [PATCH] drivers: staging: Add driver for Amplicon PCIe215

[Greg Kroah-Hartman]

On Fri, Sep 01, 2017 at 09:58:52AM +0100, Piotr Gregor wrote:
> On Fri, Sep 01, 2017 at 08:57:50AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Aug 31, 2017 at 05:54:58PM +0100, Piotr Gregor wrote:
> > > This is a small and simple driver for handling of external
> > > interrupt signal asserted on pins of Amplicon's PCIe215 board.
> > > There is already a Comedi driver subsystem in kernel which handles
> > > that (and more) board, but that framework while offering more
> > > flexibility brings also additional complexity at the cost of being generic.
> > > 
> > > In some cases the simpler, more compact solution may be preferred.
> > 
> > Note, having different drivers for the same hardware platform in the
> > kernel tree is a mess, and generally discouraged.  I've handled this in
> > the past and we hated every minute of it.
> 
> Agree.
> 
> > 
> > What is wrong with the comedi interface instead?
> 
> For a user who needs only the interruption, this driver is preferred.
> The reason is that using Comedi framework requires knowledge of Comedi
> framework. There is no reason user should spend time learning additional
> software if what he needs can be achieved in a simpler way.
> Comedi allows you to do many things, but this means you must tell Comedi what
> is what you want. If you want to use Comedi for interrupt you end up learning
> how to configure Comedi so it can setup the driver, so that the driver will
> setup the board correctly. Once you know this, you have plenty of Comedi
> related code in your program and you are dependent on Comedi drivers
> and Comedi interface to them. It also means that after handling
> interrupt in kernel, the code control path visits plenty of Comedi
> related logic, before it uderstands what it is configured to do now and
> returns to your user space program.
> Using this driver means insmod followed by 2 ioctl to enable
> interrupt generation and set up enabled pins. I tried to make it return
> to user space as quickly as possible after the ISR is entered.

It's a tradeoff between a generic hardware interface layer (i.e. what an
operating system does), and having to write individual userspace
programs for each and every individual piece of hardware.

Hint, in the long run, you are better using the common interfaces, which
is why operating systems are important :)

> > And custom ioctls for a single hardware device is horrid, what's wrong
> > with using the uio interface for something like this?
> > 
> 
> You are right. This would be something TODO.

I suggest using UIO instead of this driver entirely, if you really want
low-level control where you wish to write individual userspace programs
for every different device.  That is the kernel interface for it, not a
misc driver with random ioctls :)

thanks,

greg k-h