Subject: Re: arch/mips/pci* stuff

From: Jun Sun <[email-masked]>
Date: Tue, 11 Sep 2001 12:19:05 -0700

Gleb,

Sorry to get back to this late.  I have been on a trip.

If I understand you correctly, your major argument is that "we do scan first,
so that we can do resource assignment more easily because we have got all the
dev structures and lists".

This unfortunately may not work, mainly because the PCI bridge may not be
setup properly so the scan results may be incomplete or invalid.

However, I do see your point.  Actually the most desirable result might be
that we do the resource assignement while we do the scan.

I think more and more people are realizing that bus number assignment and
fixup must be done separately and done at the beginning.  It is a more
complicated problem.  For example, in a multi-CPU cPCI system, you don't want
to re-assign bus numbers unless you are the first CPU to boot up.

pci_auto.c is not the ultimate solution.  But the nice thing is that it solves
the broken firmware problem and yet still works with pci_scan_bus() scheme.

My personal view is that we ultimately should do something like this:

1. do one pass scan for PCI bridges and do bus number assigment or fixup.

2. based on the above results, we should have a uniformed access to all
devices on all buses (such as read_pci_config(busno, ...)).

3. do another pass of pci scan to discover pci devs and assign proper
resources.

Note currently we have constraints that physical address space is the same as
PCI memory space and multiple PCI addr spaces are usually collapsed into a
single flat space.  We really should remove those constraints so that we can
utilize the full PCI space which is rather abundant for today's systems.

BTW, I think your pcibios_assign_resources(bus) function may have already been
implemented in the current pci code.  See pbus_assign_resources().  Those
functions are relatively new.  I have not got time to fully digest them yet.

Jun

"Gleb O. Raiko" wrote:
> 
> Jun Sun wrote:
> > Traditionally, people do the following for their PCI subsystem:
> >
> > 0. PCI BIOS or firmware goes through PCI bus and assign resources
> >
> > 1. PCI driver calls pci_scan_bus() to discover all the assigned resources,
> > including serveral hooks for various fixups.
> >
> > 2. optionally, if some PCI devices have not be assigned any device, either
> > because there is no PCI BIOS or firware did not do a good job, people would
> > then call pci_assign_unassigned_resources().
> >
> > The new pci code is invoked between step 0 and step 1.  It totally ignore the
> > current PCI resource assignment, and does a complete walk-through with new
> > assignments.  Then we move on with step 1, pci_scan_bus().
> >
> > Because we trust our own PCI assignment, we don't need to do step 2 anymore.
> >
> > A side benefit of the new code is to allow an easy support for multiple PCI
> > buses.
> >
> 
> OK. I understand what is done, bu still can't understand why it's done
> in that way.
> Let me explain my view on how to perform the task.
> 
> 0. Firmware on most MIPS boxes don't do 0. or don't do it well. My case
> is here.
> I guess, pci_auto.c means others are here too. Thus, we must assign
> resources properly, high level code (drivers/pci/*.c) doesn't do that.
> 
> 1. After pci_scan_bus is completed, all devices are discovered, both
> PBARs and _sizes_ of their windows in PCI spaces are known. Devices are
> _virtually_ mapped rather at wrong places, i.e. start and end of the
> resource for a PBAR contain garbage. However, at this point, no
> resources are registered. (So, mapping is _virtual_ or properly
> speaking, there is no mapping yet. Good. It's wrong anyway.) Resources
> just sit in pci_dev structures. Thus, we can fix them by choosing right
> placements.
> 
> I prefer to do it here, not earlier, because from my point of view
> devices behind bridges shall be placed first. The rationales are
> (A) the placement algorithm is simpler
> (B) by doing this, we allocate bigger windows first, which is good.
> 
> Well, it's possible to setup proper placements either in pcibios_fixups
> or in pcibios_fixup_bus, skipping bridges on primary bus and fixing them
> when pcibios_fixup_bus is called for the bus behind the bridge but it's
> much uglier in my taste.
> 
> The placement algorithm may be fast (just get next window) or best (try
> to find most suitable room). I prefer, the best one, because, there are
> less chances to exhaust PCI spaces.
> 
> Note, we don't need even touch PCI config space, we've got all we need
> from high level (driver/pci/pci.c). Really, all devices are available,
> their PBARs and sizes are known. (Well, in fact, we have to write proper
> address in PBARs, so have to touch.)
> 
> After proper placements are chosen and PBARs is initialized we may
> 
> 2. Just call pci_assign_unassigned_resources(). It automagically setups
> bridges and registers our resources. That's all.
> 
> Calling pci_assign_unassigned_resources is natural. Our firmware did bad
> job, so we must  call that function.
> 
> Anyway, if you don't call it, you have to provide its functionality
> yourself duplicating the code. Most important here is that you have to
> setup bridges, so the code for bridge initialization will be duplicated.
> 
> Note, we don't need to scan bus ourselves nor provide fake pci_dev
> structures. What we need is just travel over the list of busses and
> setup proper placements.
> 
> I think, described strategy handles multiple primary PCI buses too. At
> least, I don't see a reason why it shouldn't.
> 
> In summary, pcibios_init shall
> {
>         for each primary bus do
>                 pci_scan_bus(bus)
>                 pcibios_assign_resources(bus)
>         enddo
> }
> 
> pcibios_assign_resources(theBus)
> {
>         for each bus behind theBus do
>                 pcibios_assign_resources(bus)
>         enddo
>         for each device on theBus do
>                 pcibios_assign_resources(device)
>         enddo
>         pci_claim_resource(theBus)
> }
> 
> pcibios_assign_resources(device):
> {
>         for each resource in device
>                 get placement for resource
>                 change resource
>                 write resource->start to PBAR
>                 pci_claim_resource(resource)
>         enddo
> }
> 
> The board specific code shall provide
> . ranges of PCI spaces in terms of "system bus"
> . functions how to map a PCI address to corresponding kernel address and
> vise versa.
>   So, something like, pci_to_virt and virt_to_pci are needed.
> 
> Add some board specific callbacks to your taste. :-)
> 
> Roughly speaking, the code is here already, just call
> pciauto_assign_resources _after_ pci_scan_bus, call
> pci_assign_unassigned_resources, and change fast placement algorithm to
> the best one.
> 
> Any thoughts ?
> 
> Regards,
> Gleb.

---

From: "Gleb O. Raiko" <[email-masked]>
Date: Mon, 17 Sep 2001 15:21:16 +0400

Jun,

> dev structures and lists".

Yes. Also, "no code duplication".

> setup properly so the scan results may be incomplete or invalid.

You may control pci_scan_bridge by pcibios_assign_all_busses, just
return 0 from the latter and the code in pci_scan_bridge assigns all
numbers itself.

> to re-assign bus numbers unless you are the first CPU to boot up.

Thus, pcibios_assign_all_busses should just check CPU number.

> 3. do another pass of pci scan to discover pci devs and assign proper

You definitely can't mix device discovering and assignment of resources
in one pass a on a multi-board cPCI system.

While you may implement the algorithm above, I think it isn't necessary.
Between my previous mail and your reply, I just implemented pcibios_
calls for a mips board that has 2 PCI busses with a PCI bridge between.
Yes, BIOS doesn't initialize PCI properly. 

What I had to do is just call pci_scan_bus,
pci_assign_unassigned_resources, and provide callbacks for 

- pcibios_fixup_bus (sets bus->resource[] to board private resources
describing PCI spaces for root bus and to corresponding bridge resources
for child bus. For the latter case, I had to inherit values from parent
bus also.)

- pcibios_update_resource (just write supplied value to the
configuration register)

- pcibios_fixup_pbus_ranges (convert addresses on the system bus to
addresses on PCI bus)

I also implemented registration of resources in the global resource
tree, but it's not necessary.

The only problem I observed is that PCI code treats 0 as invalid
address. It's bad but non-fatal.

> functions are relatively new.  I have not got time to fully digest them yet.

I had. Basically, pci_assign_unassigned_resources will do the job if
pcibios_fixup_bus will provide places for bus->resources.

Regards,
Gleb.

---

From: Jun Sun <[email-masked]>
Date: Mon, 17 Sep 2001 15:37:33 -0700

> Yes. Also, "no code duplication".

Well, "code duplication" sometimes is not too bad when you avoid "code
bloating". :-)

> return 0 from the latter and the code in pci_scan_bridge assigns all

Do you mean that we call pci_scan_brige first before scaning other devices?

> in one pass a on a multi-board cPCI system.

I have not given enough thought on 3), but it is certainly desirable.

> pci_assign_unassigned_resources, and provide callbacks for

Like I said before, this is the old style of doing things.  There are many
drawbacks in this approach.  Among them, one is to require lots of knowledge
about PCI and how the following hookup functions are called.  Not every
porting engineer is willing to dive into that.  There are some other problems
too.

Jun

---

From: "Gleb O. Raiko" <[email-masked]>
Date: Wed, 19 Sep 2001 12:23:27 +0400

> Do you mean that we call pci_scan_brige first before scaning other devices?

Yes I do. Look at drivers/pci/pci.c. The code does

for each bus (by recursion)
	scan devices on this bus
	for each bridge on this bus
		scan bridge
		scan bus behind bridge

The flow is pci_do_scan_bus -> pci_scan_bridge -> pci_do_scan_bus

> I have not given enough thought on 3), but it is certainly desirable.

Well, your previous example works here. You perform scanning of devices
and assignments in one pass. You find new device unassigned by
firmware/another CPU in cPCI system, then you need find a room in a PCI
space. You can't do that, because you don't know yet what rooms
firmware/another CPU has allocated, so you don't know what rooms are
free.

Thus, you have to scan pci in two passes. On first pass, just scan
devices and collect allocated rooms in PCI spaces. On second pass,
assign unassigned devices.

Look how Dave Miller did this for ultras in 2.2 when common pci driver
didn't do that. You have to implement more or less the same modulo
ultras hw and OBP bugs. BTW, it's exactly what current drivers/pci code
does.

> porting engineer is willing to dive into that.  There are some other problems

Sorry, your reason doesn't convince me. I believe, a porting engineer
must know hardware and operating system internals very well irrespective
of what his wishes are.

Could you explain other problems, please ? 

Regards,
Gleb.

---

From: Jun Sun <[email-masked]>
Date: Wed, 19 Sep 2001 11:00:57 -0700

> firmware/another CPU has allocated, so you don't know what rooms are

You can if you throw away all the previous assignement done by BIOS, like
pci_auto.c approach.

> Could you explain other problems, please ?

I am not actively working on PCI right now.  A couple of problems I remembered
are related to the fixup mechnisms.  If you rely on BIOS assignment, than you
cannot do fixup on a per-PCI-device basis, then you have to do fixup based on
per-device/BIOS-combo basis.

I think a couple of days ago, there was a question about the restriction of
PCI memory space being the same as CPU physical address space.  Using pci_auto
allows you to control the PCI memory region you use and deal with the
restriction more easily.

BTW, your objection seems to stem from the objection against pci_auto.  But so
far I have not seen any good reasons for not using pci_auto.  Did I miss
anything?  If pci_auto does make porting easier, is there any good reason to
go to a more difficult route?

If you compare code size, PCI auto is much much smaller than the
pci_assign_unassigned_resources().  I really don't understand what is the
complain here.

Jun