All of lore.kernel.org
 help / color / mirror / Atom feed
From: rgroner@rtd.com (Rob Groner)
To: kernelnewbies@lists.kernelnewbies.org
Subject: Did PCI/IRQ allocation change significantly after 4.2 kernel?
Date: Wed, 30 Mar 2016 11:51:50 -0400	[thread overview]
Message-ID: <1459353110.2118.11.camel@rtd-VirtualBox> (raw)
In-Reply-To: <20160330005717.GA19184@kroah.com>

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

  reply	other threads:[~2016-03-30 15:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29 14:15 Did PCI/IRQ allocation change significantly after 4.2 kernel? Rob Groner
2016-03-29 14:42 ` Greg KH
2016-03-29 14:43   ` Greg KH
2016-03-29 15:27     ` Rob Groner
2016-03-29 15:43       ` Greg KH
2016-03-29 18:27         ` Rob Groner
2016-03-29 18:38           ` Greg KH
2016-03-29 19:11             ` Rob Groner
2016-03-29 19:18               ` Greg KH
2016-03-29 19:22                 ` Greg KH
2016-03-29 20:11                   ` Rob Groner
2016-03-30  0:57                     ` Greg KH
2016-03-30 15:51                       ` Rob Groner [this message]
2016-04-02 22:26                         ` Greg KH
2016-03-30 21:24                       ` Rob Groner
2016-03-30 21:52                         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1459353110.2118.11.camel@rtd-VirtualBox \
    --to=rgroner@rtd.com \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.