On Mon, 6 Dec 2010 11:38:24 -0800 Bryce Harrington <[email protected]> wrote:
> On Mon, Dec 06, 2010 at 09:35:48AM -0800, Jesse Barnes wrote: > > On Wed, 1 Dec 2010 18:54:22 -0800 > > Bryce Harrington <[email protected]> wrote: > > > > > Jesse, > > > > > > I have a question about change 2bda5b73 that you did to libpciaccess > > > last year, to stop pci_device_get_bridge_buses() from looking at bus > > > data if the bridge data was not read. > > > > > > A ubuntu bug reporter says that this check is preventing his device from > > > getting detected properly - see > > > > > > https://bugs.launchpad.net/ubuntu/+source/libpciaccess/+bug/681207 > > > > > > Looking at the code I see that for case 0x04 there's already a check for > > > NULL, where it attempts to re-read the bridge info, so presumably there > > > are at least some situations where NULL bridge.pci is expected? > > > > > > What I'm wondering is if the check should be less strict in cases of > > > multi-function cards, or if somewhere else in the code needs updated to > > > read these devices properly. > > > > The multifunction check is probably ok, but there should still be a > > bridge above the device regardless, even if it's just a host bridge I > > think, so we should figure out why that's not getting set... > > In the discussion on 681207, it seems the portion of the data structure > being tested by this patch for null is always going to be null when it's > cast. So the original reporter finds this check always results in the > function exiting early, and suggests the patch should be reverted. What > are your thoughts? I remember adding that check for a reason; I ran into a case where it was necessary. Unfortunately either there wasn't a bug open for it or I didn't reference it in the commit, and I don't remember the details. Looking at the code later on, the check at the top certainly seems wrong; we should be filling in the bridge info later if needed. It looks like Darren's commit came before mine and actually conflicts, but we didn't catch it (I probably just did a blind rebase when I pushed my change), so it's probably safe to just revert mine. I'll do that now. Thanks, -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
