Re: pci_scan, tulip, 3c59x, multiple detections and others
Petr Vandrovec Ing. VTEI (VANDROVE@vc.cvut.cz)
Tue, 28 Sep 1999 17:53:04 MET-1
On 28 Sep 99 at 16:12, Alan Cox wrote:
> > Using physbase instead of ioaddr is important part of change... But adding
> > this to all Donalds drivers does not look like intelligent solution to me...
> > pci_scan now does check_{mem_,}region, so why it could not do
> > request_{mem_,}region too? When we can add five lines into pci's scan.c
> > and remove 8 lines from each network driver, benefit looks clear to me.
> You need to remember to free it - at that point you do need to cover physical
> addresses properly
Same way as you have to remember to iounmap it now...
Now ethernet drivers do request_mem_region with completely meaningless address
from another address space than /proc/iomem covers. I saw report with this
request_mem_region failing to register ioremapped memory at 0xd2xxxxxx
because of BIOS assigned physical range d0000000-d7ffffff to another device.
You are correct that currently Donald does not store device physical
address anywhere and stores virtual address in base_address item instead.
It is also wrong idea; report from ifconfig that I have eth0 at
0xd2544000 is completely meaningless and useless as after rmmod & insmod
it lives on different address... But it is another bug.
So what is correct solution from your point of view? Remove ioremap()
from drivers/pci/scan.c and move it to all ethernet drivers or add
request_resource to drivers/pci/scan.c and remove it from all ethernet
drivers?
From my point of view it should
(1) store device physical address into netdevice's base_address. This
ensures backward compatibility and useful data exported to userspace.
Reporting kernel virtual address to userspace is useless.
(2) store device virtual address into netdevice's priv field, if device
does not use it priv structure. Maybe specific vbase_addr field is better,
network device drivers can probably provide better comments on this (fbdev
has base & vbase... base is exported to userspace & passed to allocation/
deallocation functions, vbase is really used)
(3) drivers/pci/scan.c passes physical address to driver.
Driver can create virtual mapping by ioremap if it needs it.
(4) it is driver task to determine resource type (by checking
pci_tbl[chip_idx].pci_flags), call appropriate request_{mem,}region
and ioremap. If probe function fails, it must iounmap virtual
mapping and release registered regions.
(5) on driver unload it's driver's task to release regions and
iounmap virtual mappings.
Step (3) is really annoying, I see alternate solution:
(3) drivers/pci/scan.c passes virtual address to driver. Region passed
to driver is registered and virtual mapping is build for region.
(4) it is driver task to determine resource type and choose
readb/inb depending on it. If driver needs physical address,
it can find it by doing
pdev->resource[(pci_tbl[chip_idx].pci_flags >> 4) & 7].start
If probe function fails, it must NOT passed virtual mapping
and must NOT release passed region.
(5) on driver unload, it is driver's task to release regions and
iounmap virtual mappings.
I do not know, what is better, but first approach looks more consistent
to me. But it forces all drivers to do same things: they must determine
region type, do appropriate request region, build virtual mapping by
ioremap. And driver which fails to register its region causes big troubles
to itself and/or to system (multiple driver instance for same piece
of hardware, eventually running out of static resources (eth drivers...)).
Best regards,
Petr Vandrovec
vandrove@vc.cvut.cz
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/