Date: Fri, 18 Jul 1997 20:25:58 +0200 From: Stefan Esser <se@FreeBSD.ORG> To: Simon Shapiro <Shimon@i-connect.net> Cc: freebsd-hackers@FreeBSD.ORG, Stefan Esser <se@FreeBSD.ORG> Subject: Re: pcireg.h lost children... ? Message-ID: <19970718202558.63332@mi.uni-koeln.de> In-Reply-To: <XFMail.970717212105.Shimon@i-Connect.Net>; from Simon Shapiro on Thu, Jul 17, 1997 at 04:35:22PM -0700 References: <19970716214222.61218@mi.uni-koeln.de> <XFMail.970717212105.Shimon@i-Connect.Net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jul 17, Simon Shapiro <Shimon@i-Connect.Net> wrote:
[ ... new macro names for PCI classes and config register bits ... ]
> It did, for a while, break compatability...
Well, I'm sorry for that.
But I did not expect drivers to actually use those names ...
And you were warned: I left no doubt in my commit messages,
that the new PCI code was a independent implementation :)
> > I did not expect drivers to actually care for the class code
> > or command register values of a device, since higher level
> > code should take care of things ...
> >
> > What are you using those values for ?
>
> In pci/dpt_pci.c:
>
> dpt_pci_probe()
> ....
>
> if ((dpt_id = (type & 0xffff0000) >> 16) == DPT_DEVICE_ID) {
> /* This one appears to belong to us, but what is it? */
> class = pci_conf_read(tag, PCI_CLASS_REG);
> if (((class & PCI_CLASS_MASK) == PCI_CLASS_MASS_STORAGE) &&
> ((class & PCI_SUBCLASS_MASK) == PCI_SUBCLASS_MASS_STORAGE_SCSI) ) {
> /* It is a SCSI storage device. How do talk to it? */
> command = pci_conf_read(tag, PCI_COMMAND_STATUS_REG);
> if ( ((command & PCI_COMMAND_MEM_ENABLE) == 0)
> && ((command & PCI_COMMAND_IO_ENABLE) == 0) ) {
> printf("DPT: Cannot map the controller as memory, nor as I/O :-(\n");
> return(NULL);
> }
> } else {
> printf("DPT: Device is not Mass Storage, nor SCSI controller :-(\n");
> return(NULL);
> }
Hmmm, why don't you probe for known vendor and device IDs ?
One of the changes to the PCI code, that might follow (I have
not completely made up my mind) is, that the probe will just
associate a driver with a list of vendor/device IDs, and there
will no longer be a xxx_probe() function in the drivers.
One reason is that I want to be able to assign drivers to IDs
(chip or cards IDs) without recompilation.
NCR put a similar probe into their SDMS BIOS (3.xx.xx), where
they used the vendor ID and class code, but changed the code to
check for specific decide IDs in 4.xx.xx.
The old approach caused trouble to people with NCR SCSI cards
based on new chips, which came with their own 4.xx BIOS, but
had the (inappropriate) 3.xx BIOS trying to initialize them.
> command = pci_conf_read(tag, PCI_COMMAND_STATUS_REG);
> if ( (command & PCI_COMMAND_MASTER_ENABLE) == 0 ) {
> printf("DPT: BUSMASTER disabled :-(\n");
> return (NULL);
> }
I've never heard of a PCI BIOS, that failed to enable the
bus-master functionality, but did set up the chip correctly,
else. I really can't imagine a system that would trigger
this condition!
> And in dpt_pci_attach()
> ....
> io_base = 0;
> vaddr = 0;
> paddr = 0;
> data = pci_conf_read(config_id, PCI_BASEADR0);
>
> command = pci_conf_read(config_id, PCI_COMMAND_STATUS_REG);
> if ( ((command & PCI_COMMAND_MEM_ENABLE) == 0)
> || (pci_map_mem(config_id, PCI_BASEADR0, &vaddr, &paddr) == 0) ) {
> /* Either not memory mappable or mapping failed. Try I/O mapping */
> if ((command & PCI_COMMAND_IO_ENABLE) == 0
> || (pci_map_port(config_id, PCI_BASEADR0, &io_base) == 0) ) {
> free(dpt, M_DEVBUF);
> printf ("dpt%d: Failed to map memory or I/O registers :-(\n", unit);
> return;
> }
> }
>
> Makes sense?
The other driver's don't test for the enable bits, and
just rely on the error indication returned by the call
to pci_map_XXX().
You should not check those values from within any driver,
IMHO, but instead just try to map the regions you need,
and perform an error exit if mandatory mappings failed.
BTW: There appears to be a buglet in your code: Every
base address register may only hold either a memory or
a port address, and the address type is hard-wired into
the chip!
This means, that the use of base addr reg 0 for both the
call to pci_map_mem() and pci_map_port() must be wrong.
(I assume the letter is never tried, since the prior
always succeeds.)
The PCI BIOS needs the type bits (low order bits of the
address base registers) to be R/O in order to assign valid
addresses. If both memory and port accesses are supported,
then one register will have the two low order bits wired
to 01, and another one will have low order 4 bits of 0000
(or less likely some other even number).
The PCI BIOS uses the low order bits to decide whether
some currently unused port or memory region is reserved
for that chip, and then puts the base into the register.
You know from the data sheet, which register will hold
a port and which one will hold a memory address, if they
can be assigned.
BTW: Your test of the memory and port enable bits is not
sufficient, if the devices are behind a PCI to PCI bridge,
as Justin Gibbs pointed out to me. (And I'll modify the
PCI code to correctly return an error indication to the
pci_map_xxx() call, in such a case.)
Regards, STefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19970718202558.63332>