Messages in this thread | | | Date | Thu, 18 Jun 2009 21:43:07 +0400 | Subject | Re: staging driver s626 clashes with philips SAA7146 media/dvb based cards | From | Manu Abraham <> |
| |
Hi Herton,
On Thu, Jun 18, 2009 at 9:23 PM, Herton Ronaldo Krzesinski<herton@mandriva.com.br> wrote: > Em Quinta-feira 18 Junho 2009, às 04:28:04, Frank Mori Hess escreveu: >> On Wednesday 17 June 2009, Herton Ronaldo Krzesinski wrote: >> > > >> > > Your patch breaks configuration of the board unless the bus and slot >> > > are explicitly specified. Just make a minimal patch that replaces >> > > pci_get_device with pci_get_subsys and fixes the problem that was >> > > reported. >> > >> > Hmm that's not what the patch does, it doesn't break configuration, >> > keeps the same logic as before (I was wrong in my last email replying to >> > myself), check it, if it->options[0] and it->options[1] isn't specified, >> > the pdev is valid so the for loop exits (see !pdev check). >> >> Your right. However, it also turns a loop over pci devices into a loop >> over pci ids, which appears to break the case of multiple s626 boards >> where the bus/slot of the second s626 board is specified. If you're not >> willing to provide a minimal patch that just fixes the reported problem, >> just say so. It would have been less effort for me to do it myself than >> analyze what your changes are breaking. > > That's part of reviewing process, I just wanted to enhance it in case more pci > ids are added in the future along with the switch to pci_get_subsys, I don't > know why you act like that, you don't want the code to be enhanced? Other > comedi driver loop over ids, for example gsc_hpdi > > And indeed what you say it's true, there is a bug in the patch now that you > checked it out properly, it has a problem with this multiple s626 case, fixed > that now. Pointing out the problem instead just saying it's broken helps :) > > I also removed the obligation to add sub ids and re-factored the patch, that > was too much and not good, just a simpler loop over pci id array is used now, > and a comment was added in pci id list telling people that they should add > subvendor:subdevice ids for boards with vendor:device == 0x1131:0x7146, > in case new boards with this id appear.
Since there is valid subsystem information: why do you have to loop over the ID's manually in the driver ?
Can't the ID's be set into the PCI ID table and a normal pci_probe be used instead ? Not that i am familiar with the comedi stuff though ...
We use the probe method directly (rather than pci_get_ ) for most of the multimedia drivers. As an example this is what i do: http://jusst.de/hg/saa716x/file/59dd985d4473/linux/drivers/media/dvb/saa716x/saa716x_budget.c
Maybe it helps you in some way.
Regards, Manu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |