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 _______________________________________________ Kernelnewbies mailing list Kernelnewbies@xxxxxxxxxxxxxxxxx http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
