On Mon, Jun 09, 2003 at 10:46:21PM -0600, M. Warner Losh wrote:
> In message: <[EMAIL PROTECTED]>
> John-Mark Gurney <[EMAIL PROTECTED]> 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
[EMAIL PROTECTED] [EMAIL PROTECTED]
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
