[Skiboot] [RFC PATCH 09/23] core/pcie-slot: Remove wait hack

[Frederic Barrat]

Le 03/04/2019 à 11:09, Oliver O'Halloran a écrit :
> Now that all the users of slot->ops.set_power_state() have been converted
> to expect either OPAL_SUCCESS or a wait time we cna remove the synchrnous
> wait hack.
> 
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
>   core/pcie-slot.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/core/pcie-slot.c b/core/pcie-slot.c
> index 213740a683c7..b368e5496fab 100644
> --- a/core/pcie-slot.c
> +++ b/core/pcie-slot.c
> @@ -206,6 +206,9 @@ static int64_t pcie_slot_set_attention_state(struct pci_slot *slot,
>   	return OPAL_SUCCESS;
>   }
>   
> +#define PCIE_SLOT_POWER_IDLE 0 /* Ready to accept another power control cmd */
> +#define PCIE_SLOT_POWER_WAIT 1 /* Waiting for a previous power control cmd  */
> +
>   static int64_t pcie_slot_set_power_state_ext(struct pci_slot *slot, uint8_t val,
>   					     bool surprise_check)
>   {
> @@ -230,6 +233,11 @@ static int64_t pcie_slot_set_power_state_ext(struct pci_slot *slot, uint8_t val,
>   		return OPAL_SUCCESS;
>   	}

I think there's small problem here: in the first call to this function, 
we set power_ctl_state to wait and power_state to the desired value. The 
function is called multiple times. On the next call, we'll exit early 
with OPAL_SUCCESS because the slot power_state is already at the desired 
value. So we leave power_ctl_state at the wrong value. And the next 
power state change, then... more problems.

   Fred

> +	if (slot->power_ctl_state == PCIE_SLOT_POWER_WAIT) {
> +		slot->power_ctl_state = PCIE_SLOT_POWER_IDLE;
> +		return OPAL_SUCCESS;
> +	}
> +
>   	/*
>   	 * Suprise hotpluggable slots need to be handled with care since
>   	 * many systems do not implement the presence detect side-band
> @@ -256,6 +264,9 @@ static int64_t pcie_slot_set_power_state_ext(struct pci_slot *slot, uint8_t val,
>   		 */
>   	}
>   
> +
> +	slot->power_ctl_state = PCIE_SLOT_POWER_WAIT;
> +
>   	slot->power_state = val;
>   	ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>   	pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, &state);
> @@ -275,13 +286,7 @@ static int64_t pcie_slot_set_power_state_ext(struct pci_slot *slot, uint8_t val,
>   
>   	pci_cfg_write16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, state);
>   
> -	/*
> -	 * HACK: wait a bit for the link to come up. Drop this once we've
> -	 * converted users of set_power_state to use the new interface
> -	 */
> -	time_wait_ms(10);
> -
> -	return OPAL_SUCCESS;
> +	return 10;
>   }
>   
>   static int64_t pcie_slot_set_power_state(struct pci_slot *slot, uint8_t val)
>