Red Hat Bugzilla – Bug 175351
displaced enumerated value knackers parport_pc
Last modified: 2015-01-04 17:23:38 EST
From Bugzilla Helper: User-Agent: Mozilla/5.0 (compatible; MSIE 6.0; Windows; U; AIIEEEE!; Win98; Windows 98; en-US; Gecko masquerading as IE; should it matter?; rv:1.8b) Gecko/20050217 Description of problem: There is a misplaced enumerated value in parport_pc.c which causes faulty matching of vendor/device pci entries. Version-Release number of selected component (if applicable): kernel-2.6.14-1.1644_FC4 How reproducible: Always Steps to Reproduce: 1.read drivers/parport/parport_pc.c 2. 3. Actual Results: "How on earth could and enumerated value get out of whack", you will say. Then you will say, "Why on earth doesn't the code check that enumerated values match required displacement into table so that this sort of bug just cannot happen?". Expected Results: You shouldn't have to say anything. Additional info:
Created attachment 122071 [details] Puts parport_pc mobility_pp enum in right position
Created attachment 122072 [details] Patch to correct mobility_pp ordering problem Please discard previous patch because there is also another table entry that must also be reordered. Now that I have discovered another table ordering problem it would have probably been just as easy to adjust parport_pc_pci_tbl[]. Never mind, this patch obsoletes the previous one and seems to work ok. Only affects people trying to use syba, titan, avlab, aks, and netmos cards.
whats actually the problem here? I can't figure out why moving that enum to a different value actually fixes anything. From the diff, it seems you're just moving it from the 43rd member to the 33rd member. What am I missing here ? (You should probably post this upstream too to linux-kernel@vger.kernel.org)
(In reply to comment #3) > > From the diff, it seems you're just moving it from the 43rd member to the 33rd > member. What am I missing here ? > You have to read the code to understand. Look at parport_pc_pci_probe() and study the variable 'i' and how it is used. parport_pc_pci_tbl[].driver_data is enum type, and that enum is also used as an index into table cards[]. So cards[] must match ordering of enum. Actually my patch is wrong, because the printk (approx line 2960) should be using id->vendor instead of indexing into parport_pc_pci_tbl[] with driver_data. But cards[] *must* have correct ordering.
I follow what you're saying with the table & enum having to be in sync, but what I'm failing to see is how they're out of sync. mobility_pp looks to be in the right place both before, and after your diff as you're moving its entry in both places. This really should be taken upstream, where people more familiar with this driver can comment.
My changes did cure the problem. I know this because I made the changes, recompiled the modules, and tested them! Without the patch kernel printk's would show wrong vendor:device ids. It is all a bit more complicated than that anyhow. My original patch wasn't the best. In actual fact the tables should be constructed using [enum]={...} entries rather than the existing "/*enum*/{...}" entries. There is also a bug where _pci_tbl[enum].vendor is used instead of id->vendor etc. There is also bug where class of board is not checked (so that parport_serial never gets a chance). There are also changes needed to handle quirks for timedia serial/parallel cards which have wrong pci class. Surely you must have encountered these problems with your higher quality RHEL product? Or do you still get RHEL bugs fixed upstream?
if its an issue in RHEL, no-one reported it, and actually yes, we do try to get things fixed upstream first, so that they get coverage testing before the patches land in RHEL.