[Skiboot] [PATCH v11 13/23] core/pci: Support PCI slot
Gavin Shan
gwshan at linux.vnet.ibm.com
Tue May 31 16:23:52 AEST 2016
On Tue, May 31, 2016 at 02:35:21PM +1000, Russell Currey wrote:
>On Fri, 2016-05-20 at 16:32 +1000, Gavin Shan wrote:
>> Every PCIE bridge port or PHB is expected to be bound with PCI slot
>> , to which various PCI slot's functionalities are attached (e.g. power,
>> link, reset). This supports PCI slot:
>>
>> * PCI slot is reprsented by "struct pci_slot".
>> * "struct pci_slot_ops" represents the functions supported on the
>> PCI slot. It's initialized by PCI slot core at the beginning and
>> allowed to be overrided by platform partially or completely.
>> * On PCI hot plugging event, the PCI devices behind the slot are
>> enumarated. Device sub-tree is populated and sent to OS by OPAL
>> message.
>> * On PCI hot unplugging event, the PCI devices behind the slot are
>> destroyed. Device sub-tree is removed and the slot is powered off.
>>
>> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
>
>This patch is huge and I don't particularly think it can be split up much, so
>apologies if I miss anything. My comments are pretty minor. I think there's
>still a decent amount of room for refactoring, but nothing significant enough
>that it would need to be changed before this series is merged.
>
Thanks for looking into it, I will respin for next revision after you finish
the review until I will be told I don't have to do it. Replies included to
answer your questions. All cleanup/refactoring you suggested will be included
in next revision.
>> ---
>> core/Makefile.inc | 4 +-
>> core/pci-slot.c | 203 +++++++++++++++++++++
>> core/pci.c | 77 +++++---
>> core/pcie-slot.c | 452
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> include/opal-api.h | 10 +
>> include/pci-slot.h | 254 ++++++++++++++++++++++++++
>> include/pci.h | 66 ++-----
>> include/platform.h | 1 +
>> platforms/ibm-fsp/lxvpd.c | 1 +
>> 9 files changed, 988 insertions(+), 80 deletions(-)
>> create mode 100644 core/pci-slot.c
>> create mode 100644 core/pcie-slot.c
>> create mode 100644 include/pci-slot.h
>>
>> diff --git a/core/Makefile.inc b/core/Makefile.inc
>> index 5af0d7c..13b287c 100644
>> --- a/core/Makefile.inc
>> +++ b/core/Makefile.inc
>> @@ -3,8 +3,8 @@
>> SUBDIRS += core
>> CORE_OBJS = relocate.o console.o stack.o init.o chip.o mem_region.o
>> CORE_OBJS += malloc.o lock.o cpu.o utils.o fdt.o opal.o interrupts.o
>> -CORE_OBJS += timebase.o opal-msg.o pci.o pci-opal.o fast-reboot.o
>> -CORE_OBJS += device.o exceptions.o trace.o affinity.o vpd.o
>> +CORE_OBJS += timebase.o opal-msg.o pci.o pci-slot.o pcie-slot.o pci-opal.o
>> +CORE_OBJS += fast-reboot.o device.o exceptions.o trace.o affinity.o vpd.o
>> CORE_OBJS += hostservices.o platform.o nvram.o nvram-format.o hmi.o
>> CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o
>> CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
>> diff --git a/core/pci-slot.c b/core/pci-slot.c
>> new file mode 100644
>> index 0000000..f1416a8
>> --- /dev/null
>> +++ b/core/pci-slot.c
>> @@ -0,0 +1,203 @@
>> +/* Copyright 2013-2016 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <skiboot.h>
>> +#include <opal-msg.h>
>> +#include <pci-cfg.h>
>> +#include <pci.h>
>> +#include <pci-slot.h>
>> +
>> +/* Debugging options */
>> +#define PCI_SLOT_PREFIX "PCI-SLOT-%016llx "
>> +#define PCI_SLOT_DBG(s, fmt, a...) \
>> + prlog(PR_DEBUG, PCI_SLOT_PREFIX fmt, (s)->id, ##a)
>> +
>> +static void pci_slot_prepare_link_change(struct pci_slot *slot, bool up)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t aercap, mask;
>> +
>> + /*
>> + * Mask the link down and receiver error before the link becomes
>> + * down. Otherwise, unmask the errors when the link is up.
>> + */
>> + if (pci_has_cap(pd, PCIECAP_ID_AER, true)) {
>> + aercap = pci_cap(pd, PCIECAP_ID_AER, true);
>> +
>> + /* Link down error */
>> + pci_cfg_read32(phb, pd->bdfn, aercap + PCIECAP_AER_UE_MASK,
>> + &mask);
>> + if (up)
>> + mask &= ~PCIECAP_AER_UE_MASK_SURPRISE_DOWN;
>> + else
>> + mask |= PCIECAP_AER_UE_MASK_SURPRISE_DOWN;
>> + pci_cfg_write32(phb, pd->bdfn, aercap + PCIECAP_AER_UE_MASK,
>> + mask);
>> +
>> + /* Receiver error */
>> + pci_cfg_read32(phb, pd->bdfn, aercap + PCIECAP_AER_CE_MASK,
>> + &mask);
>> + if (up)
>> + mask &= ~PCIECAP_AER_CE_RECVR_ERR;
>> + else
>> + mask |= PCIECAP_AER_CE_RECVR_ERR;
>> + pci_cfg_write32(phb, pd->bdfn, aercap + PCIECAP_AER_CE_MASK,
>> + mask);
>> + }
>> +
>> + /*
>> + * We're coming back from reset. We need restore bus ranges
>> + * and reinitialize the affected bridges and devices.
>> + */
>> + if (up) {
>> + pci_restore_bridge_buses(phb, pd);
>> + if (phb->ops->device_init)
>> + pci_walk_dev(phb, pd, phb->ops->device_init, NULL);
>> + }
>> +}
>> +
>> +static int64_t pci_slot_sm_poll(struct pci_slot *slot)
>
>What's "sm" refer to here?
>
It means "state machine".
>> +{
>> + uint64_t now = mftb();
>> + int64_t ret;
>> +
>> + /* Return timeout value if we have */
>
>Could be phrased better. "Return remaining timeout if we're still waiting"
>maybe?
>
>> + if (slot->delay_tgt_tb &&
>> + tb_compare(now, slot->delay_tgt_tb) == TB_ABEFOREB)
>> + return slot->delay_tgt_tb - now;
>> +
>> + slot->delay_tgt_tb = 0;
>> + switch (slot->state & PCI_SLOT_STATE_MASK) {
>> + case PCI_SLOT_STATE_LINK:
>> + ret = slot->ops.poll_link(slot);
>> + break;
>> + case PCI_SLOT_STATE_HRESET:
>> + ret = slot->ops.hreset(slot);
>> + break;
>> + case PCI_SLOT_STATE_FRESET:
>> + ret = slot->ops.freset(slot);
>> + break;
>> + case PCI_SLOT_STATE_PFRESET:
>> + ret = slot->ops.pfreset(slot);
>> + break;
>> + case PCI_SLOT_STATE_CRESET:
>> + ret = slot->ops.creset(slot);
>> + break;
>> + default:
>> + prlog(PR_ERR, PCI_SLOT_PREFIX
>> + "Invalid state %08x\n", slot->id, slot->state);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_HARDWARE;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +void pci_slot_add_properties(struct pci_slot *slot,
>> + struct dt_node *np)
>
>Function name reads as if you're populating the pci_slot rather than the
>dt_node. Perhaps pci_slot_add_dt_properties()?
>
>> +{
>> + /* Bail without device node */
>> + if (!np)
>> + return;
>> +
>> + dt_add_property_cells(np, "ibm,reset-by-firmware", 1);
>> + dt_add_property_cells(np, "ibm,slot-pluggable", slot->pluggable);
>> + dt_add_property_cells(np, "ibm,slot-power-ctl", slot->power_ctl);
>> + dt_add_property_cells(np, "ibm,slot-power-led-ctlled",
>> + slot->power_led_ctl);
>> + dt_add_property_cells(np, "ibm,slot-attn-led", slot->attn_led_ctl);
>> + dt_add_property_cells(np, "ibm,slot-connector-type",
>> + slot->connector_type);
>> + dt_add_property_cells(np, "ibm,slot-card-desc", slot->card_desc);
>> + dt_add_property_cells(np, "ibm,slot-card-mech", slot->card_mech);
>> + dt_add_property_cells(np, "ibm,slot-wired-lanes", slot->wired_lanes);
>> +
>> + if (slot->ops.add_properties)
>> + slot->ops.add_properties(slot, np);
>> +}
>> +
>> +struct pci_slot *pci_slot_alloc(struct phb *phb,
>> + struct pci_device *pd)
>> +{
>> + struct pci_slot *slot = NULL;
>> +
>> + /*
>> + * The function can be used to allocate either PHB slot or normal
>> + * one. For both cases, the @phb should be always valid.
>> + */
>> + if (!phb)
>> + return NULL;
>> +
>> + /*
>> + * When @pd is NULL, we're going to create a PHB slot. Otherwise,
>> + * a normal slot will be created. Check if the specified slot has
>> + * been existing or not.
>> + */
>
>"Check if the specified slot already exists or not"
>
>> + slot = pd ? pd->slot : phb->slot;
>> + if (slot) {
>> + prlog(PR_ERR, PCI_SLOT_PREFIX "Already existing\n", slot-
>> >id);
>
>"already exists"
>
>> + return slot;
>> + }
>> +
>> + /* Allocate memory chunk */
>> + slot = zalloc(sizeof(struct pci_slot));
>> + if (!slot) {
>> + prlog(PR_ERR, "%s: Out of memory\n", __func__);
>> + return NULL;
>> + }
>> +
>> + /* The polling function shouldn't be overrided by individual platform
>> */
>
>"The polling function sholdn't be overridden by individual platforms"
>
>> + slot->phb = phb;
>> + slot->pd = pd;
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + slot->power_state = PCI_SLOT_POWER_ON;
>> + slot->ops.poll = pci_slot_sm_poll;
>> + slot->ops.prepare_link_change = pci_slot_prepare_link_change;
>> + if (!pd) {
>> + slot->id = PCI_PHB_SLOT_ID(phb);
>> + phb->slot = slot;
>> + } else {
>> + slot->id = PCI_SLOT_ID(phb, pd->bdfn);
>> + pd->slot = slot;
>> + }
>> +
>> + return slot;
>> +}
>> +
>> +struct pci_slot *pci_slot_find(uint64_t id)
>> +{
>> + struct phb *phb;
>> + struct pci_device *pd;
>> + struct pci_slot *slot;
>> + uint64_t index;
>> + uint16_t bdfn;
>> +
>> + index = PCI_SLOT_PHB_INDEX(id);
>> + phb = pci_get_phb(index);
>> +
>> + /* PHB slot */
>> + if (!(id & PCI_SLOT_ID_PREFIX)) {
>> + slot = phb ? phb->slot : NULL;
>> + return slot;
>> + }
>> +
>> + /* Normal PCI slot */
>> + bdfn = PCI_SLOT_BDFN(id);
>> + pd = phb ? pci_find_dev(phb, bdfn) : NULL;
>> + slot = pd ? pd->slot : NULL;
>> + return slot;
>> +}
>> diff --git a/core/pci.c b/core/pci.c
>> index 91e45f3..5172698 100644
>> --- a/core/pci.c
>> +++ b/core/pci.c
>> @@ -18,6 +18,7 @@
>> #include <cpu.h>
>> #include <pci.h>
>> #include <pci-cfg.h>
>> +#include <pci-slot.h>
>> #include <timebase.h>
>> #include <device.h>
>> #include <fsp.h>
>> @@ -452,11 +453,33 @@ static void pci_cleanup_bridge(struct phb *phb, struct
>> pci_device *pd)
>> pci_cfg_write16(phb, pd->bdfn, PCI_CFG_CMD, cmd);
>> }
>>
>> +/* Remove all subordinate PCI devices leading from the indicated
>> + * PCI bus. It's used to remove all PCI devices behind one PCI
>> + * slot at unplugging time
>> + */
>> +void pci_remove_bus(struct phb *phb, struct list_head *list)
>> +{
>> + struct pci_device *pd, *tmp;
>> +
>> + list_for_each_safe(list, pd, tmp, link) {
>> + pci_remove_bus(phb, &pd->children);
>> +
>> + /* Release device node and PCI slot */
>> + if (pd->dn)
>> + dt_free(pd->dn);
>> + if (pd->slot)
>> + free(pd->slot);
>> +
>> + /* Remove from parent list and release itself */
>> + list_del(&pd->link);
>> + free(pd);
>> + }
>> +}
>>
>> -/* pci_scan - Perform a recursive scan of the bus at bus_number
>> - * populating the list passed as an argument. This also
>> - * performs the bus numbering, so it returns the largest
>> - * bus number that was assigned.
>> +/* Perform a recursive scan of the bus at bus_number populating
>> + * the list passed as an argument. This also performs the bus
>> + * numbering, so it returns the largest bus number that was
>> + * assigned.
>> *
>> * Note: Eventually this might want to access some VPD information
>> * in order to know what slots to scan and what not etc..
>> @@ -466,9 +489,9 @@ static void pci_cleanup_bridge(struct phb *phb, struct
>> pci_device *pd)
>> * XXX NOTE: We might also want to setup the PCIe MPS/MRSS properly
>> * here as Linux may or may not do it
>> */
>> -static uint8_t pci_scan(struct phb *phb, uint8_t bus, uint8_t max_bus,
>> - struct list_head *list, struct pci_device *parent,
>> - bool scan_downstream)
>> +uint8_t pci_scan_bus(struct phb *phb, uint8_t bus, uint8_t max_bus,
>> + struct list_head *list, struct pci_device *parent,
>> + bool scan_downstream)
>> {
>> struct pci_device *pd = NULL;
>> uint8_t dev, fn, next_bus, max_sub, save_max;
>> @@ -585,8 +608,8 @@ static uint8_t pci_scan(struct phb *phb, uint8_t bus,
>> uint8_t max_bus,
>>
>> /* Perform recursive scan */
>> if (do_scan) {
>> - max_sub = pci_scan(phb, next_bus, max_bus,
>> - &pd->children, pd, true);
>> + max_sub = pci_scan_bus(phb, next_bus, max_bus,
>> + &pd->children, pd, true);
>> } else if (!use_max) {
>> /* XXX Empty bridge... we leave room for hotplug
>> * slots etc.. but we should be smarter at figuring
>> @@ -794,7 +817,7 @@ static void pci_scan_phb(void *data)
>> /* Scan root port and downstream ports if applicable */
>> PCIDBG(phb, 0, "Scanning (upstream%s)...\n",
>> has_link ? "+downsteam" : " only");
>> - pci_scan(phb, 0, 0xff, &phb->devices, NULL, has_link);
>> + pci_scan_bus(phb, 0, 0xff, &phb->devices, NULL, has_link);
>>
>> /* Configure MPS (Max Payload Size) for PCIe domain */
>> pci_walk_dev(phb, NULL, pci_get_mps, &mps);
>> @@ -1320,12 +1343,12 @@ static void pci_print_summary_line(struct phb *phb,
>> struct pci_device *pd,
>> rev_class & 0xff, rev_class >> 8, cname, slotstr);
>> }
>>
>> -
>> -static void pci_add_one_node(struct phb *phb, struct pci_device *pd,
>> - struct dt_node *parent_node,
>> - struct pci_lsi_state *lstate, uint8_t swizzle)
>> +static void pci_add_one_device_node(struct phb *phb,
>> + struct pci_device *pd,
>> + struct dt_node *parent_node,
>> + struct pci_lsi_state *lstate,
>> + uint8_t swizzle)
>> {
>> - struct pci_device *child;
>> struct dt_node *np;
>> const char *cname;
>> #define MAX_NAME 256
>> @@ -1440,14 +1463,14 @@ static void pci_add_one_node(struct phb *phb, struct
>> pci_device *pd,
>> * Instead add a ranges property that explicitly translates 1:1.
>> */
>> dt_add_property(np, "ranges", ranges_direct, sizeof(ranges_direct));
>> -
>> - list_for_each(&pd->children, child, link)
>> - pci_add_one_node(phb, child, np, lstate, swizzle);
>> }
>>
>> -static void pci_add_nodes(struct phb *phb)
>> +void pci_add_device_nodes(struct phb *phb,
>> + struct list_head *list,
>> + struct dt_node *parent_node,
>> + struct pci_lsi_state *lstate,
>> + uint8_t swizzle)
>> {
>> - struct pci_lsi_state *lstate = &phb->lstate;
>> struct pci_device *pd;
>>
>> /* If the PHB has its own slot info, add them */
>> @@ -1455,8 +1478,15 @@ static void pci_add_nodes(struct phb *phb)
>> pci_add_slot_properties(phb, phb->slot_info, NULL);
>>
>> /* Add all child devices */
>> - list_for_each(&phb->devices, pd, link)
>> - pci_add_one_node(phb, pd, phb->dt_node, lstate, 0);
>> + list_for_each(list, pd, link) {
>> + pci_add_one_device_node(phb, pd, parent_node,
>> + lstate, swizzle);
>> + if (list_empty(&pd->children))
>> + continue;
>> +
>> + pci_add_device_nodes(phb, &pd->children,
>> + pd->dn, lstate, swizzle);
>> + }
>> }
>>
>> static void __pci_reset(struct list_head *list)
>> @@ -1537,7 +1567,8 @@ void pci_init_slots(void)
>> for (i = 0; i < ARRAY_SIZE(phbs); i++) {
>> if (!phbs[i])
>> continue;
>> - pci_add_nodes(phbs[i]);
>> + pci_add_device_nodes(phbs[i], &phbs[i]->devices,
>> + phbs[i]->dt_node, &phbs[i]->lstate, 0);
>> }
>>
>> /* PHB final fixup */
>> diff --git a/core/pcie-slot.c b/core/pcie-slot.c
>> new file mode 100644
>> index 0000000..4783dbe
>> --- /dev/null
>> +++ b/core/pcie-slot.c
>> @@ -0,0 +1,452 @@
>> +/* Copyright 2013-2016 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <skiboot.h>
>> +#include <opal-msg.h>
>> +#include <pci-cfg.h>
>> +#include <pci.h>
>> +#include <pci-slot.h>
>> +
>> +/* Debugging options */
>> +#define PCIE_SLOT_PREFIX "PCIE-SLOT-%016llx "
>> +#define PCIE_SLOT_DBG(s, fmt, a...) \
>> + prlog(PR_DEBUG, PCIE_SLOT_PREFIX fmt, (s)->id, ##a)
>> +
>> +static int64_t pcie_slot_get_presence_state(struct pci_slot *slot, uint8_t
>> *val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + uint16_t state;
>> +
>> + /* The presence is always on if it's switch upstream port */
>
> /* The presence is always on if it's a switch upstream port */
>
>> + if (pd->dev_type == PCIE_TYPE_SWITCH_UPPORT) {
>> + *val = OPAL_PCI_SLOT_PRESENT;
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /*
>> + * The presence is always on if switch downstream port doesn't
>
> * The presence is always on if a switch downstream port doesn't
>
>> + * support slot capability according to PCIE spec.
>> + */
>> + if (pd->dev_type == PCIE_TYPE_SWITCH_DNPORT &&
>> + !(slot->slot_cap & PCICAP_EXP_CAP_SLOT)) {
>> + *val = OPAL_PCI_SLOT_PRESENT;
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Retrieve presence status */
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTSTAT, &state);
>> + if (state & PCICAP_EXP_SLOTSTAT_PDETECTST)
>> + *val = OPAL_PCI_SLOT_PRESENT;
>> + else
>> + *val = OPAL_PCI_SLOT_EMPTY;
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_get_link_state(struct pci_slot *slot,
>> + uint8_t *val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + int16_t state;
>> +
>> + /*
>> + * The link behind switch upstream port is always on
>> + * since it doesn't have valid link indicator.
>
> * since it doesn't have a valid link indicator.
>
>> + */
>> + if (pd->dev_type == PCIE_TYPE_SWITCH_UPPORT) {
>> + *val = 1;
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Retrieve link width */
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_LSTAT, &state);
>> + if (state & PCICAP_EXP_LSTAT_DLLL_ACT)
>> + *val = ((state & PCICAP_EXP_LSTAT_WIDTH) >> 4);
>> + else
>> + *val = 0;
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_get_power_state(struct pci_slot *slot __unused,
>> + uint8_t *val)
>> +{
>> + /* The power is always on if no functionality is supported */
>> + if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL))
>> + *val = PCI_SLOT_POWER_ON;
>> + else
>> + *val = slot->power_state;
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_get_attention_state(struct pci_slot *slot,
>> + uint8_t *val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + uint16_t state;
>> +
>> + /* Attention is off if the capability is missed */
>
>Should probably be "missing" instead of "missed"
>
>> + if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_ATTNI)) {
>> + *val = 0;
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Retrieve attention state */
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, &state);
>> + state = (state & PCICAP_EXP_SLOTCTL_ATTNI) >> 6;
>> + switch (state) {
>> + case PCIE_INDIC_ON:
>> + *val = PCI_SLOT_ATTN_LED_ON;
>> + break;
>> + case PCIE_INDIC_BLINK:
>> + *val = PCI_SLOT_ATTN_LED_BLINK;
>> + break;
>> + case PCIE_INDIC_OFF:
>> + default:
>> + *val = PCI_SLOT_ATTN_LED_OFF;
>> + }
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_get_latch_state(struct pci_slot *slot,
>> + uint8_t *val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + uint16_t state;
>> +
>> + /* Latch is off if MRL sensor doesn't exist */
>> + if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_MRLSENS)) {
>> + *val = 0;
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Retrieve MRL sensor state */
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTSTAT, &state);
>> + if (state & PCICAP_EXP_SLOTSTAT_MRLSENSST)
>> + *val = 1;
>> + else
>> + *val = 0;
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_set_attention_state(struct pci_slot *slot,
>> + uint8_t val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + uint16_t state;
>> +
>> + /* Drop the request if functionality doesn't exist */
>> + if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_ATTNI))
>> + return OPAL_SUCCESS;
>> +
>> + /* Update with the requested state */
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, &state);
>> + state &= ~PCICAP_EXP_SLOTCTL_ATTNI;
>> + switch (val) {
>> + case PCI_SLOT_ATTN_LED_ON:
>> + state |= (PCIE_INDIC_ON << 6);
>> + break;
>> + case PCI_SLOT_ATTN_LED_BLINK:
>> + state |= (PCIE_INDIC_BLINK << 6);
>> + break;
>> + case PCI_SLOT_ATTN_LED_OFF:
>> + state |= (PCIE_INDIC_OFF << 6);
>> + break;
>> + default:
>> + prlog(PR_ERR, PCIE_SLOT_PREFIX
>> + "Invalid attention state (0x%x)\n", slot->id, val);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + pci_cfg_write16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, state);
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static int64_t pcie_slot_set_power_state(struct pci_slot *slot, uint8_t val)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap;
>> + uint16_t state;
>> +
>> + /* Drop the request if functionality doesn't exist */
>> + if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL))
>> + return OPAL_SUCCESS;
>> +
>> + if (slot->power_state == val)
>> + return OPAL_SUCCESS;
>> +
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_SPOWER_START);
>> + 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);
>> + state &= ~(PCICAP_EXP_SLOTCTL_PWRCTLR | PCICAP_EXP_SLOTCTL_PWRI);
>> + switch (val) {
>> + case PCI_SLOT_POWER_OFF:
>> + state |= (PCICAP_EXP_SLOTCTL_PWRCTLR | (PCIE_INDIC_OFF <<
>> 8));
>> + break;
>> + case PCI_SLOT_POWER_ON:
>> + state |= (PCIE_INDIC_ON << 8);
>> + break;
>> + default:
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + prlog(PR_ERR, PCIE_SLOT_PREFIX
>> + "Invalid power state (0x%x)\n", slot->id, val);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + pci_cfg_write16(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCTL, state);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_SPOWER_DONE);
>> +
>> + return OPAL_ASYNC_COMPLETION;
>> +}
>> +
>> +static int64_t pcie_slot_sm_poll_link(struct pci_slot *slot)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint32_t ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + uint16_t val;
>> + uint8_t presence = 0;
>> +
>> + switch (slot->state) {
>> + case PCI_SLOT_STATE_LINK_START_POLL:
>> + PCIE_SLOT_DBG(slot, "LINK: Start polling\n");
>> +
>> + /* Link is down for ever without devices attached */
>> + if (slot->ops.get_presence_state)
>> + slot->ops.get_presence_state(slot, &presence);
>> + if (!presence) {
>> + PCIE_SLOT_DBG(slot, "LINK: No adapter, end
>> polling\n");
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Enable the link without check */
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_LCTL, &val);
>> + val &= ~PCICAP_EXP_LCTL_LINK_DIS;
>> + pci_cfg_write16(phb, pd->bdfn, ecap + PCICAP_EXP_LCTL, val);
>> +
>> + /*
>> + * If the link change report isn't supported, we expect
>> + * the link is up and stabilized after one second.
>> + */
>> + if (!(slot->link_cap & PCICAP_EXP_LCAP_DL_ACT_REP)) {
>> + pci_slot_set_state(slot,
>> + PCI_SLOT_STATE_LINK_DELAY_FINALIZE
>> D);
>> + return pci_slot_set_sm_timeout(slot, secs_to_tb(1));
>> + }
>> +
>> + /*
>> + * Poll the link state if link state change report is
>> + * supported on the link.
>> + */
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_LINK_POLLING);
>> + slot->retries = 250;
>
>This retry count should probably be a #define somewhere.
>
I was considering it, but gave up for two reasons: (1) There are too many
constants needed for retry times and the interval; (2) The retry times
and intervals are meaningful to this (local) function only, I guess it's
fine to use numbers.
>> + return pci_slot_set_sm_timeout(slot, msecs_to_tb(20));
>> + case PCI_SLOT_STATE_LINK_DELAY_FINALIZED:
>> + PCIE_SLOT_DBG(slot, "LINK: No link report, end polling\n");
>> + if (slot->ops.prepare_link_change)
>> + slot->ops.prepare_link_change(slot, true);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_SUCCESS;
>> + case PCI_SLOT_STATE_LINK_POLLING:
>> + pci_cfg_read16(phb, pd->bdfn, ecap + PCICAP_EXP_LSTAT, &val);
>> + if (val & PCICAP_EXP_LSTAT_DLLL_ACT) {
>> + PCIE_SLOT_DBG(slot, "LINK: Link is up, end
>> polling\n");
>> + if (slot->ops.prepare_link_change)
>> + slot->ops.prepare_link_change(slot, true);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + /* Check link state again until timeout */
>> + if (slot->retries-- == 0) {
>
>This seems a bit dodgy...
>
>> + prlog(PR_ERR, PCIE_SLOT_PREFIX
>> + "LINK: Timeout waiting for up (%04x)\n",
>> + slot->id, val);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_SUCCESS;
>> + }
>> +
>> + return pci_slot_set_sm_timeout(slot, msecs_to_tb(20));
>> + default:
>> + prlog(PR_ERR, PCIE_SLOT_PREFIX
>> + "Link: Unexpected slot state %08x\n",
>> + slot->id, slot->state);
>> + }
>> +
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_HARDWARE;
>> +}
>> +
>> +static void pcie_slot_reset(struct pci_slot *slot, bool assert)
>> +{
>> + struct phb *phb = slot->phb;
>> + struct pci_device *pd = slot->pd;
>> + uint16_t ctl;
>> +
>> + pci_cfg_read16(phb, pd->bdfn, PCI_CFG_BRCTL, &ctl);
>> + if (assert)
>> + ctl |= PCI_CFG_BRCTL_SECONDARY_RESET;
>> + else
>> + ctl &= ~PCI_CFG_BRCTL_SECONDARY_RESET;
>> + pci_cfg_write16(phb, pd->bdfn, PCI_CFG_BRCTL, ctl);
>> +}
>> +
>> +static int64_t pcie_slot_sm_hreset(struct pci_slot *slot)
>> +{
>> + switch (slot->state) {
>> + case PCI_SLOT_STATE_NORMAL:
>> + PCIE_SLOT_DBG(slot, "HRESET: Starts\n");
>> + if (slot->ops.prepare_link_change) {
>> + PCIE_SLOT_DBG(slot, "HRESET: Prepare for link
>> down\n");
>> + slot->ops.prepare_link_change(slot, false);
>> + }
>> + /* fall through */
>> + case PCI_SLOT_STATE_HRESET_START:
>> + PCIE_SLOT_DBG(slot, "HRESET: Assert\n");
>> + pcie_slot_reset(slot, true);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_HRESET_HOLD);
>> + return pci_slot_set_sm_timeout(slot, msecs_to_tb(250));
>> + case PCI_SLOT_STATE_HRESET_HOLD:
>> + PCIE_SLOT_DBG(slot, "HRESET: Deassert\n");
>> + pcie_slot_reset(slot, false);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_LINK_START_POLL);
>> + return pci_slot_set_sm_timeout(slot, msecs_to_tb(1800));
>> + default:
>> + PCIE_SLOT_DBG(slot, "HRESET: Unexpected slot state %08x\n",
>> + slot->state);
>> + }
>> +
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_HARDWARE;
>> +}
>> +
>> +/*
>> + * Usually, individual platforms need override the power
>> + * management methods for fundamental reset because the
>> + * hot reset is commonly shared.
>> + */
>
>Perhaps:
>
>/*
> * Usually, individual platforms need to override the power
> * management methods for fundamental reset because the
> * hot reset method is commonly shared.
> */
>
>> +static int64_t pcie_slot_sm_freset(struct pci_slot *slot)
>> +{
>> + uint8_t power_state = PCI_SLOT_POWER_ON;
>> +
>> + switch (slot->state) {
>> + case PCI_SLOT_STATE_NORMAL:
>> + PCIE_SLOT_DBG(slot, "FRESET: Starts\n");
>> + if (slot->ops.prepare_link_change)
>> + slot->ops.prepare_link_change(slot, false);
>> +
>> + /* Retrieve power state */
>> + if (slot->ops.get_power_state) {
>> + PCIE_SLOT_DBG(slot, "FRESET: Retrieve power
>> state\n");
>> + slot->ops.get_power_state(slot, &power_state);
>> + }
>> +
>> + /* In power on state, power it off */
>> + if (power_state == PCI_SLOT_POWER_ON &&
>> + slot->ops.set_power_state) {
>> + PCIE_SLOT_DBG(slot, "FRESET: Power is on, turn
>> off\n");
>> + slot->ops.set_power_state(slot, PCI_SLOT_POWER_OFF);
>> + pci_slot_set_state(slot,
>> + PCI_SLOT_STATE_FRESET_POWER_OFF);
>> + return pci_slot_set_sm_timeout(slot,
>> msecs_to_tb(50));
>> + }
>> + /* No power state change, fall through */
>> + case PCI_SLOT_STATE_FRESET_POWER_OFF:
>> + PCIE_SLOT_DBG(slot, "FRESET: Power is off, turn on\n");
>> + if (slot->ops.set_power_state)
>> + slot->ops.set_power_state(slot, PCI_SLOT_POWER_ON);
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_HRESET_START);
>> + return pci_slot_set_sm_timeout(slot, msecs_to_tb(50));
>> + default:
>> + prlog(PR_ERR, PCIE_SLOT_PREFIX
>> + "FRESET: Unexpected slot state %08x\n",
>> + slot->id, slot->state);
>> + }
>> +
>> + pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL);
>> + return OPAL_HARDWARE;
>> +}
>> +
>> +struct pci_slot *pcie_slot_create(struct phb *phb, struct pci_device *pd)
>> +{
>> + struct pci_slot *slot;
>> + uint32_t ecap;
>> +
>> + /* Allocate PCI slot */
>> + slot = pci_slot_alloc(phb, pd);
>> + if (!slot)
>> + return NULL;
>> +
>> + /* Cache the link and slot capabilities */
>> + if (pd) {
>> + ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
>> + pci_cfg_read32(phb, pd->bdfn, ecap + PCICAP_EXP_LCAP,
>> + &slot->link_cap);
>> + pci_cfg_read32(phb, pd->bdfn, ecap + PCICAP_EXP_SLOTCAP,
>> + &slot->slot_cap);
>> + }
>> +
>> + /* Slot info */
>
>Should probably change this comment to something else so as not to confuse
>things with the pci_slot_info struct
>
>> + if ((slot->slot_cap & PCICAP_EXP_SLOTCAP_HPLUG_SURP) &&
>> + (slot->slot_cap & PCICAP_EXP_SLOTCAP_HPLUG_CAP))
>> + slot->pluggable = 1;
>> + if (slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL)
>> + slot->power_ctl = 1;
>> + if (slot->slot_cap & PCICAP_EXP_SLOTCAP_PWRI)
>> + slot->power_led_ctl = PCI_SLOT_PWR_LED_CTL_KERNEL;
>> + if (slot->slot_cap & PCICAP_EXP_SLOTCAP_ATTNI)
>> + slot->attn_led_ctl = PCI_SLOT_ATTN_LED_CTL_KERNEL;
>> + slot->wired_lanes = ((slot->link_cap & PCICAP_EXP_LCAP_MAXWDTH) >>
>> 4);
>> + /* Standard slot operations */
>> + slot->ops.get_presence_state = pcie_slot_get_presence_state;
>> + slot->ops.get_link_state = pcie_slot_get_link_state;
>> + slot->ops.get_power_state = pcie_slot_get_power_state;
>> + slot->ops.get_attention_state = pcie_slot_get_attention_state;
>> + slot->ops.get_latch_state = pcie_slot_get_latch_state;
>> + slot->ops.set_power_state = pcie_slot_set_power_state;
>> + slot->ops.set_attention_state = pcie_slot_set_attention_state;
>> +
>> + /*
>> + * SM based reset stuff. The poll function is always
>> + * unified for all cases.
>> + */
>
>Probably a good place to expand on what SM is.
>
>> + slot->ops.poll_link = pcie_slot_sm_poll_link;
>> + slot->ops.hreset = pcie_slot_sm_hreset;
>> + slot->ops.freset = pcie_slot_sm_freset;
>> + slot->ops.pfreset = NULL;
>> +
>> + return slot;
>> +}
>> diff --git a/include/opal-api.h b/include/opal-api.h
>> index 41af0e5..a81d870 100644
>> --- a/include/opal-api.h
>> +++ b/include/opal-api.h
>> @@ -376,6 +376,16 @@ enum OpalPciMaskAction {
>> OPAL_MASK_ERROR_TYPE = 1
>> };
>>
>> +enum OpalPciSlotPresentenceState {
>
>Should be "OpalPciSlotPresenceState" or even just "OpalPciSlotPresence".
>
>> + OPAL_PCI_SLOT_EMPTY = 0,
>> + OPAL_PCI_SLOT_PRESENT = 1
>> +};
>> +
>> +enum OpalPciSlotPowerState {
>> + OPAL_PCI_SLOT_POWER_OFF = 0,
>> + OPAL_PCI_SLOT_POWER_ON = 1
>> +};
>> +
>> enum OpalSlotLedType {
>> OPAL_SLOT_LED_TYPE_ID = 0, /* IDENTIFY LED */
>> OPAL_SLOT_LED_TYPE_FAULT = 1, /* FAULT LED */
>
>In v10 you defined a new opal_msg_type called OPAL_MSG_PCI_HOTPLUG, is there a
>reason that went away?
>
The message used by PCI hotplug exclusively has been dropped in this revision.
Instead, we reuse OPAL_MSG_ASYNC_COMP and note that the individual message is
identified by token.
>> diff --git a/include/pci-slot.h b/include/pci-slot.h
>> new file mode 100644
>> index 0000000..cc64b0e
>> --- /dev/null
>> +++ b/include/pci-slot.h
>> @@ -0,0 +1,254 @@
>> +/* Copyright 2013-2016 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#ifndef __PCI_SLOT_H
>> +#define __PCI_SLOT_H
>> +
>> +#include <opal.h>
>> +#include <device.h>
>> +#include <timebase.h>
>> +#include <timer.h>
>> +#include <ccan/list/list.h>
>> +
>> +/*
>> + * PCI Slot Info: Wired Lane Values
>> + *
>> + * Values 0 to 6 match slot map 1005. In case of *any* change here
>> + * make sure to keep the lxvpd.c parsing code in sync *and* the
>> + * corresponding label strings in pci.c
>> + */
>> +#define PCI_SLOT_WIRED_LANES_UNKNOWN 0x00
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X1 0x01
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X2 0x02
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X4 0x03
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X8 0x04
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X16 0x05
>> +#define PCI_SLOT_WIRED_LANES_PCIE_X32 0x06
>> +#define PCI_SLOT_WIRED_LANES_PCIX_32 0x07
>> +#define PCI_SLOT_WIRED_LANES_PCIX_64 0x08
>> +
>> +/* PCI Slot Info: Bus Clock Values */
>> +#define PCI_SLOT_BUS_CLK_RESERVED 0x00
>> +#define PCI_SLOT_BUS_CLK_GEN_1 0x01
>> +#define PCI_SLOT_BUS_CLK_GEN_2 0x02
>> +#define PCI_SLOT_BUS_CLK_GEN_3 0x03
>> +
>> +/* PCI Slot Info: Connector Type Values */
>> +#define PCI_SLOT_CONNECTOR_PCIE_EMBED 0x00
>> +#define PCI_SLOT_CONNECTOR_PCIE_X1 0x01
>> +#define PCI_SLOT_CONNECTOR_PCIE_X2 0x02
>> +#define PCI_SLOT_CONNECTOR_PCIE_X4 0x03
>> +#define PCI_SLOT_CONNECTOR_PCIE_X8 0x04
>> +#define PCI_SLOT_CONNECTOR_PCIE_X16 0x05
>> +#define PCI_SLOT_CONNECTOR_PCIE_NS 0x0E /* Non-Standard */
>> +
>> +/* PCI Slot Info: Card Description Values */
>> +#define PCI_SLOT_DESC_NON_STANDARD 0x00 /* Embed/Non-
>> Standard */
>> +#define PCI_SLOT_DESC_PCIE_FH_FL 0x00 /* Full Height, Full
>> Length */
>> +#define PCI_SLOT_DESC_PCIE_FH_HL 0x01 /* Full Height, Half
>> Length */
>> +#define PCI_SLOT_DESC_PCIE_HH_FL 0x02 /* Half Height, Full
>> Length */
>> +#define PCI_SLOT_DESC_PCIE_HH_HL 0x03 /* Half Height, Half
>> Length */
>> +
>> +/* PCI Slot Info: Mechanicals Values */
>> +#define PCI_SLOT_MECH_NONE 0x00
>> +#define PCI_SLOT_MECH_RIGHT 0x01
>> +#define PCI_SLOT_MECH_LEFT 0x02
>> +#define PCI_SLOT_MECH_RIGHT_LEFT 0x03
>> +
>> +/* PCI Slot Info: Power LED Control Values */
>> +#define PCI_SLOT_PWR_LED_CTL_NONE 0x00 /* No Control */
>> +#define PCI_SLOT_PWR_LED_CTL_FSP 0x01 /* FSP Controlled */
>> +#define PCI_SLOT_PWR_LED_CTL_KERNEL 0x02 /* Kernel Controlled
>> */
>> +
>> +/* PCI Slot Info: ATTN LED Control Values */
>> +#define PCI_SLOT_ATTN_LED_CTL_NONE 0x00 /* No Control */
>> +#define PCI_SLOT_ATTN_LED_CTL_FSP 0x01 /* FSP Controlled */
>> +#define PCI_SLOT_ATTN_LED_CTL_KERNEL 0x02 /* Kernel Controlled
>> */
>> +
>> +/* Attention LED */
>> +#define PCI_SLOT_ATTN_LED_OFF 0
>> +#define PCI_SLOT_ATTN_LED_ON 1
>> +#define PCI_SLOT_ATTN_LED_BLINK 2
>> +
>> +/* Power state */
>> +#define PCI_SLOT_POWER_OFF 0
>> +#define PCI_SLOT_POWER_ON 1
>> +
>> +/*
>> + * We have hard and soft reset for slot. Hard reset requires
>> + * power-off and then power-on, but soft reset only resets
>> + * secondary bus.
>> + */
>> +struct pci_slot;
>> +struct pci_slot_ops {
>> + /* For slot management */
>> + int64_t (*get_presence_state)(struct pci_slot *slot, uint8_t *val);
>> + int64_t (*get_link_state)(struct pci_slot *slot, uint8_t *val);
>> + int64_t (*get_power_state)(struct pci_slot *slot, uint8_t *val);
>> + int64_t (*get_attention_state)(struct pci_slot *slot, uint8_t *val);
>> + int64_t (*get_latch_state)(struct pci_slot *slot, uint8_t *val);
>> + int64_t (*set_power_state)(struct pci_slot *slot, uint8_t val);
>> + int64_t (*set_attention_state)(struct pci_slot *slot, uint8_t val);
>> +
>> + /* SM based functions for reset */
>> + void (*prepare_link_change)(struct pci_slot *slot, bool is_up);
>> + int64_t (*poll_link)(struct pci_slot *slot);
>> + int64_t (*creset)(struct pci_slot *slot);
>> + int64_t (*freset)(struct pci_slot *slot);
>> + int64_t (*pfreset)(struct pci_slot *slot);
>> + int64_t (*hreset)(struct pci_slot *slot);
>> + int64_t (*poll)(struct pci_slot *slot);
>> +
>> + /* Auxillary functions */
>> + void (*add_properties)(struct pci_slot *slot, struct dt_node *np);
>> +};
>> +
>> +/*
>> + * The PCI slot state is split up into base and number. With this
>> + * design, the individual platforms can introduce their own PCI
>> + * slot states with addition to the base. Eventually, the base
>> + * state can be recognized by PCI slot core.
>> + */
>> +#define PCI_SLOT_STATE_MASK 0xFFFFFF00
>> +#define PCI_SLOT_STATE_NORMAL 0x00000000
>> +#define PCI_SLOT_STATE_LINK 0x00000100
>> +#define PCI_SLOT_STATE_LINK_START_POLL 0x00000101
>> +#define PCI_SLOT_STATE_LINK_DELAY_FINALIZED 0x00000102
>> +#define PCI_SLOT_STATE_LINK_POLLING 0x00000103
>> +#define PCI_SLOT_STATE_HRESET 0x00000200
>> +#define PCI_SLOT_STATE_HRESET_START 0x00000201
>> +#define PCI_SLOT_STATE_HRESET_HOLD 0x00000202
>> +#define PCI_SLOT_STATE_FRESET 0x00000300
>> +#define PCI_SLOT_STATE_FRESET_POWER_OFF 0x00000301
>> +#define PCI_SLOT_STATE_PFRESET 0x00000400
>> +#define PCI_SLOT_STATE_PFRESET_START 0x00000401
>> +#define PCI_SLOT_STATE_CRESET 0x00000500
>> +#define PCI_SLOT_STATE_CRESET_START 0x00000501
>> +#define PCI_SLOT_STATE_GPOWER 0x00000600
>> +#define PCI_SLOT_STATE_GPOWER_START 0x00000601
>> +#define PCI_SLOT_STATE_SPOWER 0x00000700
>> +#define PCI_SLOT_STATE_SPOWER_START 0x00000701
>> +#define PCI_SLOT_STATE_SPOWER_DONE 0x00000702
>> +#define PCI_SLOT_STATE_GPRESENCE 0x00000800
>> +#define PCI_SLOT_STATE_GPRESENCE_START 0x00000801
>> +
>> +
>> +struct pci_slot {
>> + uint32_t flags;
>> +#define PCI_SLOT_FLAG_BOOTUP 0x1
>> +
>> + struct phb *phb;
>> + struct pci_device *pd;
>> +
>> + /* Identifier */
>> + uint64_t id;
>> + struct timer timer;
>> + uint64_t async_token;
>> + uint8_t power_state;
>> +
>> + /* Slot information */
>> + uint8_t pluggable;
>> + uint8_t power_ctl;
>> + uint8_t power_led_ctl;
>> + uint8_t attn_led_ctl;
>> + uint8_t connector_type;
>> + uint8_t card_desc;
>> + uint8_t card_mech;
>> + uint8_t wired_lanes;
>> +
>> + /* State machine */
>> + uint32_t state;
>> + uint32_t retry_state;
>> + uint32_t link_cap;
>> + uint32_t slot_cap;
>> + uint64_t delay_tgt_tb;
>
>"delay_tgt_tb"? Unlike the other values, I don't think this one is particularly
>self-explanatory. It'd be good to have a comment here given it's not especially
>clear what it is when it's set, either.
>
>> + uint64_t retries;
>> + struct pci_slot_ops ops;
>> + void *data;
>> +};
>> +
>> +#define PCI_SLOT_ID_PREFIX 0x8000000000000000
>> +#define PCI_SLOT_ID(phb, bdfn) \
>> + (PCI_SLOT_ID_PREFIX | ((uint64_t)(bdfn) << 16) | (phb)->opal_id)
>> +#define PCI_PHB_SLOT_ID(phb) ((phb)->opal_id)
>> +#define PCI_SLOT_PHB_INDEX(id) ((id) & 0xfffful)
>> +#define PCI_SLOT_BDFN(id) (((id) >> 16) & 0xfffful)
>> +
>> +static inline uint32_t pci_slot_add_flags(struct pci_slot *slot,
>> + uint32_t flags)
>> +{
>> + uint32_t old = 0;
>> +
>> + if (slot) {
>> + old = slot->flags;
>> + slot->flags |= flags;
>> + }
>> +
>> + return old;
>> +}
>> +
>> +static inline bool pci_slot_has_flags(struct pci_slot *slot,
>> + uint32_t flags)
>> +{
>> + if (!slot)
>> + return false;
>> +
>> + if ((slot->flags & flags) == flags)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static inline uint32_t pci_slot_remove_flags(struct pci_slot *slot,
>> + uint32_t flags)
>> +{
>> + uint32_t old = 0;
>> +
>> + if (slot) {
>> + old = slot->flags;
>> + slot->flags &= ~flags;
>> + }
>> +
>> + return old;
>> +}
>> +
>> +static inline void pci_slot_set_state(struct pci_slot *slot, uint32_t state)
>> +{
>> + if (slot)
>> + slot->state = state;
>> +}
>> +
>> +static inline uint64_t pci_slot_set_sm_timeout(struct pci_slot *slot,
>> + uint64_t dur)
>> +{
>> + uint64_t target, now = mftb();
>> +
>> + target = now + dur;
>> + if (target == 0)
>> + target++;
>> + slot->delay_tgt_tb = target;
>
>What's going on here?
>
It's to tell the polling function how many ticks (count of decrementer)
it needs to wait. Nothing will executed/changed in the PCI slot's state
machine until the delay is expired. Note we still have "retry" mechanism,
meaning the delay could possibly repeat.
>> +
>> + return dur;
>> +}
>> +
>> +extern struct pci_slot *pci_slot_alloc(struct phb *phb,
>> + struct pci_device *pd);
>> +extern struct pci_slot *pcie_slot_create(struct phb *phb,
>> + struct pci_device *pd);
>> +extern void pci_slot_add_properties(struct pci_slot *slot,
>> + struct dt_node *np);
>> +extern struct pci_slot *pci_slot_find(uint64_t id);
>> +#endif /* __PCI_SLOT_H */
>> diff --git a/include/pci.h b/include/pci.h
>> index 3481ed4..11d6126 100644
>> --- a/include/pci.h
>> +++ b/include/pci.h
>> @@ -22,60 +22,6 @@
>> #include <lock.h>
>> #include <ccan/list/list.h>
>>
>> -/* PCI Slot Info: Wired Lane Values
>> - *
>> - * Values 0 to 6 match slot map 1005. In case of *any* change here
>> - * make sure to keep the lxvpd.c parsing code in sync *and* the
>> - * corresponding label strings in pci.c
>> - */
>> -#define PCI_SLOT_WIRED_LANES_UNKNOWN 0x00
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X1 0x01
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X2 0x02
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X4 0x03
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X8 0x04
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X16 0x05
>> -#define PCI_SLOT_WIRED_LANES_PCIE_X32 0x06
>> -#define PCI_SLOT_WIRED_LANES_PCIX_32 0x07
>> -#define PCI_SLOT_WIRED_LANES_PCIX_64 0x08
>> -
>> -/* PCI Slot Info: Bus Clock Values */
>> -#define PCI_SLOT_BUS_CLK_RESERVED 0x00
>> -#define PCI_SLOT_BUS_CLK_GEN_1 0x01
>> -#define PCI_SLOT_BUS_CLK_GEN_2 0x02
>> -#define PCI_SLOT_BUS_CLK_GEN_3 0x03
>> -
>> -/* PCI Slot Info: Connector Type Values */
>> -#define PCI_SLOT_CONNECTOR_PCIE_EMBED 0x00
>> -#define PCI_SLOT_CONNECTOR_PCIE_X1 0x01
>> -#define PCI_SLOT_CONNECTOR_PCIE_X2 0x02
>> -#define PCI_SLOT_CONNECTOR_PCIE_X4 0x03
>> -#define PCI_SLOT_CONNECTOR_PCIE_X8 0x04
>> -#define PCI_SLOT_CONNECTOR_PCIE_X16 0x05
>> -#define PCI_SLOT_CONNECTOR_PCIE_NS 0x0E /* Non-Standard */
>> -
>> -/* PCI Slot Info: Card Description Values */
>> -#define PCI_SLOT_DESC_NON_STANDARD 0x00 /* Embed/Non-Standard Connector
>> */
>> -#define PCI_SLOT_DESC_PCIE_FH_FL 0x00 /* Full Height, Full Length */
>> -#define PCI_SLOT_DESC_PCIE_FH_HL 0x01 /* Full Height, Half Length */
>> -#define PCI_SLOT_DESC_PCIE_HH_FL 0x02 /* Half Height, Full Length */
>> -#define PCI_SLOT_DESC_PCIE_HH_HL 0x03 /* Half Height, Half Length */
>> -
>> -/* PCI Slot Info: Mechanicals Values */
>> -#define PCI_SLOT_MECH_NONE 0x00
>> -#define PCI_SLOT_MECH_RIGHT 0x01
>> -#define PCI_SLOT_MECH_LEFT 0x02
>> -#define PCI_SLOT_MECH_RIGHT_LEFT 0x03
>> -
>> -/* PCI Slot Info: Power LED Control Values */
>> -#define PCI_SLOT_PWR_LED_CTL_NONE 0x00 /* No Control */
>> -#define PCI_SLOT_PWR_LED_CTL_FSP 0x01 /* FSP Controlled */
>> -#define PCI_SLOT_PWR_LED_CTL_KERNEL 0x02 /* Kernel Controlled */
>> -
>> -/* PCI Slot Info: ATTN LED Control Values */
>> -#define PCI_SLOT_ATTN_LED_CTL_NONE 0x00 /* No Control */
>> -#define PCI_SLOT_ATTN_LED_CTL_FSP 0x01 /* FSP Controlled */
>> -#define PCI_SLOT_ATTN_LED_CTL_KERNEL 0x02 /* Kernel Controlled */
>> -
>> /* PCI Slot Entry Information */
>> struct pci_slot_info {
>> char label[16];
>> @@ -151,6 +97,7 @@ struct pci_device {
>> struct list_head pcrf;
>>
>> struct dt_node *dn;
>> + struct pci_slot *slot;
>> struct pci_slot_info *slot_info;
>> struct pci_device *parent;
>> struct list_head children;
>> @@ -468,6 +415,7 @@ struct phb {
>> uint32_t mps;
>>
>> /* PCI-X only slot info, for PCI-E this is in the RC bridge */
>> + struct pci_slot *slot;
>> struct pci_slot_info *slot_info;
>
>In both the pci_device and phb structs, I don't see why we need separate
>pci_slot and pci_slot_info structs; logically, they should be merged together.
>This is something that can be changed after this series is merged, I think.
>
>Do you see any reason why we should have to separate structs here?
>
"struct pci_slot_info" will be removed in last patches included in
this series, I can't remove it at this point and build error will
be raised because of it.
>>
>> /* Base location code used to generate the children one */
>> @@ -525,7 +473,15 @@ static inline int64_t pci_cfg_write32(struct phb *phb,
>> uint32_t bdfn,
>> }
>>
>> /* Utilities */
>> -
>> +extern void pci_remove_bus(struct phb *phb, struct list_head *list);
>> +extern uint8_t pci_scan_bus(struct phb *phb, uint8_t bus, uint8_t max_bus,
>> + struct list_head *list, struct pci_device
>> *parent,
>> + bool scan_downstream);
>> +extern void pci_add_device_nodes(struct phb *phb,
>> + struct list_head *list,
>> + struct dt_node *parent_node,
>> + struct pci_lsi_state *lstate,
>> + uint8_t swizzle);
>> extern int64_t pci_find_cap(struct phb *phb, uint16_t bdfn, uint8_t cap);
>> extern int64_t pci_find_ecap(struct phb *phb, uint16_t bdfn, uint16_t cap,
>> uint8_t *version);
>> diff --git a/include/platform.h b/include/platform.h
>> index f1bdc30..d07994f 100644
>> --- a/include/platform.h
>> +++ b/include/platform.h
>> @@ -20,6 +20,7 @@
>> /* Some fwd declarations for types used further down */
>> struct phb;
>> struct pci_device;
>> +struct pci_slot;
>> struct errorlog;
>>
>> enum resource_id {
>> diff --git a/platforms/ibm-fsp/lxvpd.c b/platforms/ibm-fsp/lxvpd.c
>> index 90c9e09..542c49a 100644
>> --- a/platforms/ibm-fsp/lxvpd.c
>> +++ b/platforms/ibm-fsp/lxvpd.c
>> @@ -24,6 +24,7 @@
>> #include <vpd.h>
>> #include <pci.h>
>> #include <pci-cfg.h>
>> +#include <pci-slot.h>
>>
>> #include "lxvpd.h"
>>
Thanks,
Gavin
More information about the Skiboot
mailing list