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