[Skiboot] [PATCH 3/3] pci: Wait for CRS and switch link when restoring bus numbers

[Andrew Donnellan]

On 17/08/17 16:04, Russell Currey wrote:
> When a complete reset occurs, after the PHB recovers it propagates a
> reset down the wire to every device.  At the same time, skiboot talks to
> every device in order to restore the state of devices to what they were
> before the reset.
> 
> In some situations, such as devices that recovered slowly and/or were
> behind a switch, skiboot attempted to access config space of the device
> before the link was up and the device could respond.
> 
> Fix this by retrying CRS until the device responds correctly, and for
> devices behind a switch, making sure the switch has its link up first.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Signed-off-by: Russell Currey <ruscur at russell.cc>

This looks fairly sensible to me.

Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>

> @@ -1835,14 +1848,26 @@ static int __pci_restore_bridge_buses(struct phb *phb,
>   				      struct pci_device *pd,
>   				      void *data __unused)
>   {
> -	if (!pd->is_bridge) {
> -		uint32_t vdid;
> +	uint32_t vdid;
> +
> +	/* If the device is behind a switch, wait for the switch */
> +	if (!pd->is_vf && !(pd->bdfn & 7) && pd->parent != NULL &&
> +	    pd->parent->dev_type == PCIE_TYPE_SWITCH_DNPORT) {
> +		if (!pci_bridge_wait_link(phb, pd->parent, true)) {
> +			PCIERR(phb, pd->bdfn, "Timeout waiting for switch\n");
> +			return -1;
> +		}
> +	}
> +
> +	/* Wait for config space to stop returning CRS */
> +	if (!pci_wait_crs(phb, pd->bdfn, &vdid))
> +		return -1;

What does returning -1 do - it looks to me that this stops the 
pci_walk_dev() from descending any further, which seems correct, but I 
haven't used pci_walk_dev() very much...


-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited