Dealing with PCI reset

[Paul Menzel]

Dear coreboot folks,

Several devices using the Intel 945 chipset copied code for PCI reset,
costing 200 ms of boot time.

```
        /* Force PCIRST# */
        pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, SBR);
        udelay(200 * 1000);
        pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, 0);
```

The change-set Ia37d9f0ecf5655531616edb20b53757d5d47b42f [1] removes
that code from the Lenovo X60.

That code was added for some crypto card on a Roda device.

My question is, if removing that code is fine, or if it should be left
in and be made configurable (Kconfig/NVRAM)?

Are there often cases where there are extensions card with problems,
that need such a PCI reset?

Thanks,

Paul

[1] https://review.coreboot.org/17703

[Timothy Pearson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/11/2016 03:48 PM, Paul Menzel via coreboot wrote:
> Dear coreboot folks,
>
> Several devices using the Intel 945 chipset copied code for PCI reset,
> costing 200 ms of boot time.
>         /* Force PCIRST# */
>         pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, SBR);
>         udelay(200 * 1000);
>         pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, 0);
>
> The change-set Ia37d9f0ecf5655531616edb20b53757d5d47b42f [1] removes
> that code from the Lenovo X60.
>
> That code was added for some crypto card on a Roda device.
>
> My question is, if removing that code is fine, or if it should be left
> in and be made configurable (Kconfig/NVRAM)?
>
> Are there often cases where there are extensions card with problems,
> that need such a PCI reset?

We have run into this issue on the KGPE-D16 and LSI SAS controllers.
However, in this case it's not so much the reset itself as it is the
time it takes for the card to start up and become ready for PCI scan.

We handled this with a devicetree.cb option to set a delay between reset
and PCI scan.  Perhaps something similar could be used in this instance?

- -- 
Timothy Pearson
Raptor Engineering
[phone] (direct line)
[phone] (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJYTcspAAoJEK+E3vEXDOFbbzQH/34G9MP0fpe5a83ifoXu20Dq
12GLduOIsknFdYoe45yBHwKCsejJPrc4cMYjv7cYMHVP1Ujlu9UoKVyMN4IiAGTK
i5fIvjyMsDh8XB4zngBoffhZUckvIwxiCvIojMhkUebRB04j+nYluZgpFNNHPo1B
vOZtAXP1Vst6qc82fid9U2b5gWOyTNgaTxCdf9FeKFGdIKoW14Gn3c1o3TJN88hf
CTsDAamKGs2ZPINbfX5VIrlQL+ft87I+Az5IpwJKY4Vy8JA4L5AgNHnaMrTYKyu2
GbGtNTORjGij5XSTrDWTZxBa0oLzpLkNzvCWZBqohSwIv1L5p2NOUtgTvRc7AS0=
=uoXv
-----END PGP SIGNATURE-----

[Paul Menzel]

Dear Timothy,

Thank you for your reply.

Am Sonntag, den 11.12.2016, 15:54 -0600 schrieb Timothy Pearson:
> > Several devices using the Intel 945 chipset copied code for PCI reset,
> > costing 200 ms of boot time.
> >          /* Force PCIRST# */
> >          pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, SBR);
> >          udelay(200 * 1000);
> >          pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, 0);
> >
> > The change-set Ia37d9f0ecf5655531616edb20b53757d5d47b42f [1] removes
> > that code from the Lenovo X60.
> >
> > That code was added for some crypto card on a Roda device.
> >
> > My question is, if removing that code is fine, or if it should be left
> > in and be made configurable (Kconfig/NVRAM)?
> >
> > Are there often cases where there are extensions card with problems,
> > that need such a PCI reset?
>
> We have run into this issue on the KGPE-D16 and LSI SAS controllers.
> However, in this case it's not so much the reset itself as it is the
> time it takes for the card to start up and become ready for PCI scan.
>
> We handled this with a devicetree.cb option to set a delay between reset
> and PCI scan.  Perhaps something similar could be used in this instance?

I guess you are referring to the following line in
`src/mainboard/asus/kgpe-d16/devicetree.cb`?

```
register "pcie_settling_time" = "1000000" # Allow PIKE to be detected / configured
```

This option is only available for that particular southbridge, AMD
SR5650, right?

In the Lenovo X60 case, it’d probably would be better to have it as a
run time option in NVRAM, so that it can be configured depending if
such a problematic card is used or not.

Would it be feasible to implement something like that in `src/device`,
so that all devices can use it?

Or is that overkill, and such pluggable(?), that means external, cards
are rare, and the code should just be removed for the Lenovo X60?

Thanks,

Paul

[Nico Huber]

Hi Paul,

sorry for the late reply.

On 11.12.2016 22:48, Paul Menzel via coreboot wrote:
> Dear coreboot folks,
>
> Several devices using the Intel 945 chipset copied code for PCI reset,
> costing 200 ms of boot time.
>         /* Force PCIRST# */
>         pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, SBR);
>         udelay(200 * 1000);
>         pci_write_config16(PCI_DEV(0, 0x1e, 0), BCTRL, 0);
>
> The change-set Ia37d9f0ecf5655531616edb20b53757d5d47b42f [1] removes
> that code from the Lenovo X60.
>
> That code was added for some crypto card on a Roda device.

According to Stepan, this was a security measure to make sure the card
is always reset on a reboot.
> My question is, if removing that code is fine, or if it should be left
> in and be made configurable (Kconfig/NVRAM)?

I don't think it's worth any hassle. We have our own branches locally
anyway, I can place it there.
> Are there often cases where there are extensions card with problems,
> that need such a PCI reset?

If Stepan is right, there were no functionality problems at all.

Nico
> Thanks,
>
> Paul
>
> [1] https://review.coreboot.org/17703