All of lore.kernel.org
 help / color / mirror / Atom feed
From: greg@kroah.com (Greg KH)
To: kernelnewbies@lists.kernelnewbies.org
Subject: Did PCI/IRQ allocation change significantly after 4.2 kernel?
Date: Tue, 29 Mar 2016 11:38:51 -0700	[thread overview]
Message-ID: <20160329183851.GA9186@kroah.com> (raw)
In-Reply-To: <1459276021.2010.19.camel@rtd-VirtualBox>

On Tue, Mar 29, 2016 at 02:27:01PM -0400, Rob Groner wrote:
> On Tue, 2016-03-29 at 08:43 -0700, Greg KH wrote:
> > On Tue, Mar 29, 2016 at 11:27:49AM -0400, Rob Groner wrote:
> > > x86 only (we don't support other platforms).  An example of the code
> > > (this problem appears to be affecting all of our drivers so far) is on
> > > our website.
> > > 
> > > http://www.rtd.com/software/DM/DM35x18/DM35418_Linux_v03.00.00.tar.gz
> > > 
> > > If you end up looking at the driver code, I apologize in advance for all
> > > of the obvious errors you might find.  It's a little intimidating having
> > > one of the authors of LDD look at your code... I feel like I didn't
> > > study hard enough for this.  
> > 
> > Heh, no worries.  One question, why are you saving off the pci irq
> > before you call request_irq()?  You shouldn't care what the number is,
> > and with MSI and other fun, you might get a "different" irq number than
> > the main PCI device was because nothing was really assigned to it just
> > yet.
> > 
> 
> Can I invoke my 5th amendment rights here?  
> 
> Actually, I don't really know.  Most of that driver is inherited from
> other drivers we had before it so a lot of code is "legacy" for lack of
> a better term.
> 
> The driver appears to care about what the IRQ number because it uses it
> in several other places in the driver: to compare to the incoming IRQ in
> the interrupt handler, and to use when the "free_irq" call is required. 
> If we shouldn't care what the IRQ is then that means we don't need it
> for those things?  Or are you saying we should just keep a pointer to
> the pci_dev and reference that IRQ value instead of saving our own?

Hm, maybe this is ok, it just seems odd that you check the irq number in
the handler, that shouldn't be needed at all as the core will not call
you unless the irq you have signed up for has been triggered.

> > Also, why not submit this for inclusion in the main kernel tree?  That
> > will make your ongoing maintenance of the code much easier.
> 
> I had considered that since this driver currently supports 5 of our
> boards (that's better than most of our drivers).  It would be a nice
> thing to say it is supported in the kernel.  But I'm not sure how that
> will make maintenance easier.  I had a small view into the patch
> submitting process earlier this year, and it didn't seem easy...Is it
> different if I'm patching my own driver?

You will get other people fixing your bugs and for any api changes, they
will be made automatically.  And, odds are, your driver will get a lot
smaller, there seems to be things in there that aren't needed.  And less
code means less bugs and easier to maintain overtime.

It shouldn't be hard to merge patches for a driver you maintain, if so
then the development process is at fault, and let me know what's going
on and I'll work to help fix it.

thanks,

greg k-h

  reply	other threads:[~2016-03-29 18:38 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 [this message]
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
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=20160329183851.GA9186@kroah.com \
    --to=greg@kroah.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.