On Tue, Mar 29, 2016 at 03:11:46PM -0400, Rob Groner wrote: > On Tue, 2016-03-29 at 11:38 -0700, Greg KH wrote: > > 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: > > > > > > 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. > > > > Has that always been true? I'm pretty sure that code came from a driver > that was around the 2.6.35 era, or earlier. I will happily remove that > IRQ check, though. I'm always interested in less code to maintain. > Don't I still need to keep ahold of the IRQ number to call free_irq() > when the resources are released, though? Yes you do. > > > > 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. > > > That is encouraging and persuasive. I will make submitting the driver > one of my pet projects. I need to put a new coat of paint on it before > I submit it for consideration, though. Do I post to this mailing list > when I'm ready? Or is there a more pertinent one? I can take it "as-is" through the staging tree (drivers/staging/) which lets the community start to clean it up now, if you don't have the time to do the work now. Then you can work within the kernel development process to polish up the remaining issues before we merge it out of the staging tree into the "real" portion of the kernel. If you are interested in this, just let me know. thanks, greg k-h _______________________________________________ Kernelnewbies mailing list Kernelnewbies@xxxxxxxxxxxxxxxxx http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
