From mboxrd@z Thu Jan  1 00:00:00 1970
From: rgroner@rtd.com (Rob Groner)
Date: Wed, 30 Mar 2016 11:51:50 -0400
Subject: Did PCI/IRQ allocation change significantly after 4.2 kernel?
In-Reply-To: <20160330005717.GA19184@kroah.com>
References: <20160329144244.GA24067@kroah.com>
	<20160329144314.GA24237@kroah.com>
	<1459265269.2010.7.camel@rtd-VirtualBox>
	<20160329154353.GA26108@kroah.com>
	<1459276021.2010.19.camel@rtd-VirtualBox>
	<20160329183851.GA9186@kroah.com>
	<1459278706.2010.24.camel@rtd-VirtualBox>
	<20160329191840.GA30945@kroah.com> <20160329192244.GA31666@kroah.com>
	<1459282282.2010.37.camel@rtd-VirtualBox>
	<20160330005717.GA19184@kroah.com>
Message-ID: <1459353110.2118.11.camel@rtd-VirtualBox>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

On Tue, 2016-03-29 at 17:57 -0700, Greg KH wrote:
> On Tue, Mar 29, 2016 at 04:11:22PM -0400, Rob Groner wrote:
> > Your first glance is probably correct.  The driver handles reads and
> > writes to registers via IOCTLs from the user library, as well as
> > interrupts and DMA.  There are probably two main reasons the driver is
> > structured like that: 1) All "new" drivers here are essentially tailored
> > versions of previous drivers that have been around for a while, so this
> > wasn't built-from-scratch new.  While it means that any poor design
> > decisions made early on are perpetuated, it also means to us that we
> > aren't re-inventing the wheel and we're mostly using a driver that has
> > proven to work for quite a while.  2) The library code for the board is
> > shared (internally) with our Windows driver package, and in some cases
> > DOS too.  Since Windows uses IOCTLs, we can essentially share the exact
> > same library files, and only the IOCTL call itself is OS-specific.
> > 
> > I know #1 is not a good reason and I'd be happy to work towards
> > re-writing the driver in a more correct way, but probably not if it
> > would cause us to have to split into a Linux and Windows library
> > versions.  Keeping the library common at this point has saved us a lot
> > of development time.
> 
> I understand the need for userspace libraries to be "unified", but you
> might be able to get away with no kernel driver at all, just use the UIO
> driver for your device and then read/write the memory mapped area of
> your card directly from your library/application.  That will have the
> benifit of making your Linux implementation faster as well :)

I'm looking at the UIO API for the first time, and I'm beginning to
understand it, and I *do* see the benefits of it.  Instead of working to
get this driver accepted in the kernel, I think I am going to instead
make a UIO implementation my pet project.

>>From what I've learned so far, I'll still need a kernel driver for the
interrupt handler, just much smaller.  One thing I haven't been able to
find out for sure, however, is if DMA is possible through a UIO
implementation.  Not seeing any mention of it on kernel.org isn't
encouraging.

> > I absolutely appreciate the feedback on driver design...I've never
> > really received any and all I know about Linux drivers is what I read
> > online, read in LDD, and what was in the drivers that existed when I
> > came to work here.  Is the current form of the driver just "bad" or
> > "intolerably bad"?
> 
> It's not "bad", but there is room for improvement to make it smaller and
> more "flexible" (i.e. no static list of devices, use the pci driver
> model better, etc.)  You should also make sure you are properly checking
> your ioctl calls to ensure you don't have any security issues hiding
> here, that wouldn't be good for your users.  I think you are doing this
> correctly in dm35418_validate_pci_access(), but it would be good to
> verify this again to be sure, your ioctl structures are a bit "odd" in
> that they try to be generic so it's hard to really tell what you are
> passing around.
> 

That is good input.  I'll take another look at how I scrutinize what is
being sent in IOCTLs.  I'm guessing that it would probably be better if
I used read() and write() instead of a READ IOCTL and WRITE IOCTL,
correct?

BTW, git bisect continues.  It says about 10 more tries....

Rob