[Skiboot] [PATCH v2 4/9] core/pci: Update PCI topology after power change

[Andrew Donnellan]

On 13/10/16 12:16, Gavin Shan wrote:
> When OPAL_SUCCESS is returned from slot->ops.set_power_state(),
> we need update the PCI toplogy accordingly. This scenario can
> happen when builtin power control functionality is ignored to
> accomodate PCI surprise hotplug or not supported at all by the
> hardware.

I think I get what this is doing, but correct me if I'm wrong, it took 
me a bit of work... if set_power_state() returns OPAL_ASYNC_COMPLETION, 
then that means we're using normal power control functionality and the 
topology will be updated in set_power_timer(). If set_power_state() 
returns OPAL_SUCCESS on the other hand, then that means the slot doesn't 
have power control capability and as such we should update the topology 
immediately.

Perhaps a small comment in the code would be helpful, but that's not too 
big a deal.

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

> ---
>  core/pci-opal.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/core/pci-opal.c b/core/pci-opal.c
> index ba7a261..7ba64f5 100644
> --- a/core/pci-opal.c
> +++ b/core/pci-opal.c
> @@ -805,8 +805,8 @@ static int64_t opal_pci_set_power_state(uint64_t async_token,
>  			return OPAL_PARAMETER;
>
>  		pci_remove_bus(phb, &pd->children);
> -		rc = OPAL_SUCCESS;
> -		break;
> +		phb_unlock(phb);
> +		return OPAL_SUCCESS;
>  	case OPAL_PCI_SLOT_ONLINE:
>  		if (!pd)
>  			return OPAL_PARAMETER;
> @@ -814,19 +814,29 @@ static int64_t opal_pci_set_power_state(uint64_t async_token,
>  			     &pd->children, pd, true);
>  		pci_add_device_nodes(phb, &pd->children, pd->dn,
>  				     &phb->lstate, 0);
> -		rc = OPAL_SUCCESS;
> -		break;
> +		phb_unlock(phb);
> +		return OPAL_SUCCESS;
>  	default:
>  		rc = OPAL_PARAMETER;
>  	}
>
> -	phb_unlock(phb);
>  	if (rc == OPAL_ASYNC_COMPLETION) {
>  		slot->retries = 500;
>  		init_timer(&slot->timer, set_power_timer, slot);
>  		schedule_timer(&slot->timer, msecs_to_tb(10));
> +	} else if (rc == OPAL_SUCCESS) {
> +		if (*state == OPAL_PCI_SLOT_POWER_OFF) {
> +			pci_remove_bus(phb, &pd->children);
> +		} else {
> +			slot->ops.prepare_link_change(slot, true);
> +			pci_scan_bus(phb, pd->secondary_bus,
> +				pd->subordinate_bus, &pd->children, pd, true);
> +			pci_add_device_nodes(phb, &pd->children, pd->dn,
> +				&phb->lstate, 0);
> +		}
>  	}
>
> +	phb_unlock(phb);
>  	return rc;
>  }
>  opal_call(OPAL_PCI_SET_POWER_STATE, opal_pci_set_power_state, 3);
>

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