Re: PCI bus numbering and orphaned devices

On Mon, Jun 09, 2003 at 10:46:21PM -0600, M. Warner Losh wrote:
> In message: <20030609210919.33379_at_hydrogen.funkthat.com>
>             John-Mark Gurney <gurney_j_at_efn.org> writes:
> : > > +#ifdef __sparc64__
> : > > +		/*
> : > > +		 * XXX - some sparc hardware has valid hardware when the
> : > > +		 * function 0 doesn't probe.  Scan all functions.
> : > > +		 */
> : > > +		pcifunchigh = PCI_FUNCMAX;
> : > > +#else
> : > >  		pcifunchigh = 0;
> : > > +#endif
> : > >  		for (f = 0; f <= pcifunchigh; f++) {
> : > >  			dinfo = pci_read_device(pcib, busno, s, f, dinfo_size);
> : > >  			if (dinfo != NULL) {
> : > 
> : > This is problematic as it ignores the fact about single function
> : > devices which may react to all function numbers.
> : 
> : Wouldn't this happen with the current logic? since if function 0 is
> : found, it scans the rest...  (Might be getting confused with SCSI
> : buses).

No - it checks the mfdev flag before increasing pcifunchigh.

> Actually, there's no reason not to scan the hardware.  we likely
> should be checking the multi function status differently than we are
> right now.  We also shouldn't be rejecting based on the vendor id.
> while that provides a convenient way to chek to see if it really isn't
> there, a better sanity check would be to check the header type to see
> if it a one we know about (0, 1 or 2).  If so, then we know if the
> device is there.  that might be a better hueristic to see if we need
> to scan everything.
> 
> : Actually, I was thinking that we could check to see if the next word
> : is not -1.  The chip responds to the rest of the registers, but just
> : doesn't respond to the DEVVENDOR (first word).
> 
> since header type is a required field, this likely is a better way to
> go.  maybe keep the test against -1 for adding it as a child, but
> don't assume nothing is there unless the header type is bogus.
> 
> : I'm also thinking of adding support code to the pci bus to let the
> : userland add a new device node to be probed.  It shouldn't be too hard,
> : but would be help in these cases.
> 
> I'd rather tht we fix the pci probe code to do the right thing.
> Kludges like this tend to live for a long time because nobody bothers
> to fix them correctly...
> 
> I'm thinking that the loop should be more like:
> 
> 		pcifunchigh = 0;
> 		f = 0;
> 		hdrtype	= REG(PCIR_HEADERTYPE, 1);
> 		if (hdrtype & 0x7f > 2)
> 			continue;
> 		if (hdrtype & 0x80)
s/0x80/PCIM_MFDEV/
Maybe we should add a PCIM_REGLAYOUT as well.

> 			pcifunchigh = PCI_FUNCMAX;
> 		for (f = 0; f <= pcifunchigh; f++) {
> 			dinfo = pci_read_device(pcib, busno, s, f, dinfo_size);
> 			if (dinfo != NULL)
> 				pci_add_child(dev, dinfo);
> 		}
> 
> might be better code (REG likely needs to be correctly defined for
> this context).

This needs to be tested on that given hardware.
I don't know if REG will work as expected because it asks function 0,
which is disabled.

> this is based on my limited understanding that function 0 shouldn't be
> attached and function 1 should...

-- 
B.Walter                   BWCT                http://www.bwct.de
ticso_at_bwct.de                                  info_at_bwct.de