Hi Detlev,
On Fri, 27 May 2011 17:26:24 +0200 Detlev Zundel dzu@denx.de wrote: ...
PCI cards might need some time after reset to respond. On some boards (mpc5200 or mpc8260 based) the PCI bus reset is deasserted at pci_board_init() time, so we can not use available "pcidelay" option for waiting before pci bus scan here. Add an option to delay bus scan by setting "pci_scan_delay" environment variable.
Hm, I'm not sure I understand the situation, so please correct me. We have a "pcidelay" variable, which is used to wait before pci_board_init() (I'm not counting the semantically different usage in the esd boards). This does not fit your need, so you define pci_scan_delay which is used _after_ pci_init_board(), correct?
yes, this is correct.
If this is correct, then why don't you keep your new delay also in the pci_init() function so that the delays are easily visible on code inspection? But wait, if this is only needed for this very board, then why don't we put the delay into digsys pci_init_board? Actually I think this is the best way, as on this board we always need the delay as PCI is not hotplug.
The reason for not keeping new delay in pci_init() is: pci_init_board() starts scanning the bus (calls pci_hose_scan()), so when pci_init_board() returns, it is too late, the scanning is already completed.
digsy's pci_init_board() just calls pci_mpc5xxx_init(), when the latter returns, the scanning is completed, too. PCI reset is de-asserted in pci_mpc5xxx_init(), so I thought about putting the delay there, but similar situation is also on mpc8260 based boards, pci_mpc8250_init() de-asserts PCI reset and waits on some boards (on MPC8266ADS 1 sec). So the problem is not only digsy specific. The needed time after reset before config cycles could be up to 1 sec, depending on the card. The pci spec 2.2 allows this.
I think that it would be good to run arch specific pci init not from the pci_board_init(), but from pci_init(). Then we can add delay code in the board specific way. This would reduce the code duplication, too. Currently we have the same pci_init_board() for many 5200 boards, except for mvbc_p and mvsmr boards.
Apart from that, having two variables "pcidelay" and "pci_scan_delay" we would need good documentation to explain their usage - the names do not help (me) much ;)
Sure. If there is an agreement to solve the problem as proposed in the patch, I'll add the documentation in the next patch version. Maybe someone have a better idea, lets wait a bit for other comments. Actually I don't like the name of the variable, it is somehow misleading. Any better name?
Thanks, Anatolij