[U-Boot] [PATCH 2/5] pci: option for configurable delay between pci reset and pci bus scan

[Detlev Zundel <dzu@denx.de>]

Hi Anatolij,

> 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.

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: dzu at denx.de