[Skiboot] [PATCH v10 16/17] core/opal: Use PCI slot and new APIs for slot states

Alistair Popple alistair at popple.id.au
Fri May 6 11:37:12 AEST 2016


Gavin,

<snip>

> >
> >As we discussed offline there needs to be much better separation of the
> >platform specific parts and the generic parts of setting the power state.
> >
> >At the moment the calls to update device tree, etc. are all in the platform
> >specific code but they should really appear here as it will apply to all platforms
> >that eventually support hotplug. The platform specific code should only deal with
> >platform dependent functions such as actually removing power from the slots which
> >is completely different on say an FSP system versus a non-FSP system.
> >
> 
> Thanks for review. Yeah, it's absolutely making sense. I will have the change
> after collecting more comments.

Yeah, no problem. I'm going to get stuck into the review of the kernel/skiboot
ABI next week so we can hopefully get it right in time for the kernel stuff
to go upstream in the next cycle.

Regards,

Alistair

> Thanks,
> Gavin
> 
> >> +	phb_unlock(phb);
> >> +
> >> +	if (rc > 0) {
> >> +		rc = tb_to_msecs(rc);
> >> +		if (rc == 0)
> >> +			rc = 1;
> >> +	}
> >> +
> >> +	return rc;
> >> +}
> >> +opal_call(OPAL_PCI_SET_POWER_STATE, opal_pci_set_power_state, 2);
> >> +
> >> +static int64_t opal_pci_poll2(uint64_t id, uint64_t data)
> >> +{
> >> +	struct pci_slot *slot = pci_slot_find(id);
> >> +	struct phb *phb = slot ? slot->phb : NULL;
> >> +	uint8_t *state = (uint8_t *)data;
> >> +	int64_t rc;
> >> +
> >> +	if (!slot || !phb)
> >> +		return OPAL_PARAMETER;
> >> +	if (!slot->ops.poll)
> >> +		return OPAL_UNSUPPORTED;
> >> +
> >> +	phb_lock(phb);
> >> +	rc = slot->ops.poll(slot, state);
> >> +	phb_unlock(phb);
> >> +
> >> +	/* Return milliseconds for caller to sleep: round up */
> >> +	if (rc > 0) {
> >> +		rc = tb_to_msecs(rc);
> >> +		if (rc == 0)
> >> +			rc = 1;
> >> +	}
> >> +
> >> +	return rc;
> >> +}
> >> +opal_call(OPAL_PCI_POLL2, opal_pci_poll2, 2);
> >> +
> >>  static int64_t opal_pci_set_phb_tce_memory(uint64_t phb_id,
> >>  					   uint64_t tce_mem_addr,
> >>  					   uint64_t tce_mem_size)
> >> diff --git a/include/opal-api.h b/include/opal-api.h
> >> index 50de6d1..d7b5a1e 100644
> >> --- a/include/opal-api.h
> >> +++ b/include/opal-api.h
> >> @@ -164,7 +164,11 @@
> >>  #define OPAL_CEC_REBOOT2			116
> >>  #define OPAL_CONSOLE_FLUSH			117
> >>  #define OPAL_GET_DEVICE_TREE			118
> >> -#define OPAL_LAST				118
> >> +#define OPAL_PCI_GET_PRESENCE_STATE		119
> >> +#define OPAL_PCI_GET_POWER_STATE		120
> >> +#define OPAL_PCI_SET_POWER_STATE		121
> >> +#define OPAL_PCI_POLL2				122
> >> +#define OPAL_LAST				122
> >>  
> >>  /* Device tree flags */
> >>  
> >> diff --git a/include/pci.h b/include/pci.h
> >> index 1326a84..048ef0d 100644
> >> --- a/include/pci.h
> >> +++ b/include/pci.h
> >> @@ -293,78 +293,6 @@ struct phb_ops {
> >>  	 */
> >>  	int64_t (*pci_msi_eoi)(struct phb *phb, uint32_t hwirq);
> >>  
> >> -	/*
> >> -	 * Slot control
> >> -	 */
> >> -
> >> -	/* presence_detect - Check for a present device
> >> -	 *
> >> -	 * Immediate return of:
> >> -	 *
> >> -	 * OPAL_SHPC_DEV_NOT_PRESENT = 0,
> >> -	 * OPAL_SHPC_DEV_PRESENT = 1
> >> -	 *
> >> -	 * or a negative OPAL error code
> >> -	 */
> >> -	int64_t (*presence_detect)(struct phb *phb);
> >> -
> >> -	/* link_state - Check link state
> >> -	 *
> >> -	 * Immediate return of:
> >> -	 *
> >> -	 * OPAL_SHPC_LINK_DOWN = 0,
> >> -	 * OPAL_SHPC_LINK_UP_x1 = 1,
> >> -	 * OPAL_SHPC_LINK_UP_x2 = 2,
> >> -	 * OPAL_SHPC_LINK_UP_x4 = 4,
> >> -	 * OPAL_SHPC_LINK_UP_x8 = 8,
> >> -	 * OPAL_SHPC_LINK_UP_x16 = 16,
> >> -	 * OPAL_SHPC_LINK_UP_x32 = 32
> >> -	 *
> >> -	 * or a negative OPAL error code
> >> -	 */
> >> -	int64_t (*link_state)(struct phb *phb);
> >> -
> >> -	/* power_state - Check slot power state
> >> -	 *
> >> -	 * Immediate return of:
> >> -	 *
> >> -	 * OPAL_SLOT_POWER_OFF = 0,
> >> -	 * OPAL_SLOT_POWER_ON = 1,
> >> -	 *
> >> -	 * or a negative OPAL error code
> >> -	 */
> >> -	int64_t (*power_state)(struct phb *phb);
> >> -
> >> -	/* slot_power_off - Start slot power off sequence
> >> -	 *
> >> -	 * Asynchronous function, returns a positive delay
> >> -	 * or a negative error code
> >> -	 */
> >> -	int64_t (*slot_power_off)(struct phb *phb);
> >> -
> >> -	/* slot_power_on - Start slot power on sequence
> >> -	 *
> >> -	 * Asynchronous function, returns a positive delay
> >> -	 * or a negative error code.
> >> -	 */
> >> -	int64_t (*slot_power_on)(struct phb *phb);
> >> -
> >> -	/* PHB power off and on after complete init */
> >> -	int64_t (*complete_reset)(struct phb *phb, uint8_t assert);
> >> -
> >> -	/* hot_reset - Hot Reset sequence */
> >> -	int64_t (*hot_reset)(struct phb *phb);
> >> -
> >> -	/* Fundamental reset */
> >> -	int64_t (*fundamental_reset)(struct phb *phb);
> >> -
> >> -	/* poll - Poll and advance asynchronous operations
> >> -	 *
> >> -	 * Returns a positive delay, 0 for success or a
> >> -	 * negative OPAL error code
> >> -	 */
> >> -	int64_t (*poll)(struct phb *phb);
> >> -
> >>  	/* Put phb in capi mode or pcie mode */
> >>  	int64_t (*set_capi_mode)(struct phb *phb, uint64_t mode, uint64_t pe_number);
> >>  
> >> 
> >
> 



More information about the Skiboot mailing list