Hi Anatolij, > Hi Detlev, > > On Fri, 27 May 2011 17:26:24 +0200 > Detlev Zundel <d...@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.
Ah, thanks for shedding some light on this. Now I see how you arrived at the solution you propose. > 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. Yes, I have also noticed the massive code duplicatin here. But as I obviously didn't even understand the problem I didn't ponder changing it ;) >> 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? Sorry, no idea. If we are stuck stuck with "pcidelay" (which I think we are), then it is hard to come up with a differentiating name. So good documentation will have to make up for the lack of good names ;) Cheers Detlev -- Old mathematicians never die; they just lose some of their functions. -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
