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?
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?
Also, can you use 'git bisect' to find the problem commit? That might be the easiest way forward.
Umm...yes! As soon as I figure out what that means. I'm a git newbie (we use svn), but I shall learn as quickly as I can. I take it that it basically means "start with a kernel that worked and then start moving forward through commits until I find the one that broke", right?
Yes, but it uses a tree search, making it faster than just "one commit after each other" Read 'man git-bisect' for all of the details.
Gotcha! I'll get to work on that. Thanks. Rob