[Skiboot] [PATCH 3/4] core/pci: Use cached vendor/device/class IDs on populating device node
Benjamin Herrenschmidt
benh at kernel.crashing.org
Thu Jun 15 15:32:51 AEST 2017
On Fri, 2017-04-07 at 10:16 +1000, Gavin Shan wrote:
> This uses the cached vendor/device/class IDs when populating the
> device node for the particular PCI device, no need to read from
> the hardware in order to save some CPU cycles.
I don't like the class change.
The class is 3 bytes, don't make it be class | revid, it's pointless,
we cache it already as class, so that doesn't need changing.
> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
> ---
> core/pci.c | 54 +++++++++++++++++++++++-------------------------------
> include/pci.h | 3 +++
> 2 files changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/core/pci.c b/core/pci.c
> index a93239b..7742454 100644
> --- a/core/pci.c
> +++ b/core/pci.c
> @@ -251,7 +251,6 @@ static struct pci_device *pci_scan_one(struct phb *phb, struct pci_device *paren
> pd->vdid = vdid;
> pci_cfg_read32(phb, bdfn, PCI_CFG_SUBSYS_VENDOR_ID, &pd->sub_vdid);
> pci_cfg_read32(phb, bdfn, PCI_CFG_REV_ID, &pd->class);
> - pd->class >>= 8;
>
> pd->parent = parent;
> list_head_init(&pd->pcrf);
> @@ -268,6 +267,14 @@ static struct pci_device *pci_scan_one(struct phb *phb, struct pci_device *paren
>
> pci_init_capabilities(phb, pd);
>
> + /*
> + * Quirk for IBM bridge bogus class on PCIe root complex.
> + * Without it, the PCI DN won't be created for its downstream
> + * devices in Linux.
> + */
> + if (!parent && pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false))
> + pd->class = (0x06040000 | (pd->class & 0xff));
> +
> /* If it's a bridge, sanitize the bus numbers to avoid forwarding
> *
> * This will help when walking down those bridges later on
> @@ -1331,16 +1338,12 @@ static void pci_add_loc_code(struct dt_node *np, struct pci_device *pd)
> }
>
> static void pci_print_summary_line(struct phb *phb, struct pci_device *pd,
> - struct dt_node *np, u32 rev_class,
> - const char *cname)
> + struct dt_node *np, const char *cname)
> {
> const char *label, *dtype, *s;
> - u32 vdid;
> #define MAX_SLOTSTR 32
> char slotstr[MAX_SLOTSTR + 1] = { 0, };
>
> - pci_cfg_read32(phb, pd->bdfn, 0, &vdid);
> -
> /* If it's a slot, it has a slot-label */
> label = dt_prop_get_def(np, "ibm,slot-label", NULL);
> if (label) {
> @@ -1379,14 +1382,15 @@ static void pci_print_summary_line(struct phb *phb, struct pci_device *pd,
> if (pd->is_bridge)
> PCINOTICE(phb, pd->bdfn,
> "[%s] %04x %04x R:%02x C:%06x B:%02x..%02x %s\n",
> - dtype, vdid & 0xffff, vdid >> 16,
> - rev_class & 0xff, rev_class >> 8, pd->secondary_bus,
> - pd->subordinate_bus, slotstr);
> + dtype, PCI_VENDOR_ID(pd->vdid), PCI_DEVICE_ID(pd->vdid),
> + PCI_CLASS_REV(pd->class), PCI_CLASS_CODE(pd->class),
> + pd->secondary_bus, pd->subordinate_bus, slotstr);
> else
> PCINOTICE(phb, pd->bdfn,
> "[%s] %04x %04x R:%02x C:%06x (%14s) %s\n",
> - dtype, vdid & 0xffff, vdid >> 16,
> - rev_class & 0xff, rev_class >> 8, cname, slotstr);
> + dtype, PCI_VENDOR_ID(pd->vdid), PCI_DEVICE_ID(pd->vdid),
> + PCI_CLASS_REV(pd->class), PCI_CLASS_CODE(pd->class),
> + cname, slotstr);
> }
>
> static void pci_add_one_device_node(struct phb *phb,
> @@ -1400,7 +1404,6 @@ static void pci_add_one_device_node(struct phb *phb,
> #define MAX_NAME 256
> char name[MAX_NAME];
> char compat[MAX_NAME];
> - uint32_t rev_class, vdid;
> uint32_t reg[5];
> uint8_t intpin;
> const uint32_t ranges_direct[] = {
> @@ -1411,19 +1414,8 @@ static void pci_add_one_device_node(struct phb *phb,
> 0x02000000, 0x0, 0x0,
> 0xf0000000, 0x0};
>
> - pci_cfg_read32(phb, pd->bdfn, 0, &vdid);
> - pci_cfg_read32(phb, pd->bdfn, PCI_CFG_REV_ID, &rev_class);
> pci_cfg_read8(phb, pd->bdfn, PCI_CFG_INT_PIN, &intpin);
> -
> - /*
> - * Quirk for IBM bridge bogus class on PCIe root complex.
> - * Without it, the PCI DN won't be created for its downstream
> - * devices in Linux.
> - */
> - if (pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false) &&
> - parent_node == phb->dt_node)
> - rev_class = (rev_class & 0xff) | 0x6040000;
> - cname = pci_class_name(rev_class >> 8);
> + cname = pci_class_name(PCI_CLASS_CODE(pd->class));
>
> if (pd->bdfn & 0x7)
> snprintf(name, MAX_NAME - 1, "%s@%x,%x",
> @@ -1436,17 +1428,17 @@ static void pci_add_one_device_node(struct phb *phb,
> /* XXX FIXME: make proper "compatible" properties */
> if (pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false)) {
> snprintf(compat, MAX_NAME, "pciex%x,%x",
> - vdid & 0xffff, vdid >> 16);
> + PCI_VENDOR_ID(pd->vdid), PCI_DEVICE_ID(pd->vdid));
> dt_add_property_cells(np, "ibm,pci-config-space-type", 1);
> } else {
> snprintf(compat, MAX_NAME, "pci%x,%x",
> - vdid & 0xffff, vdid >> 16);
> + PCI_VENDOR_ID(pd->vdid), PCI_DEVICE_ID(pd->vdid));
> dt_add_property_cells(np, "ibm,pci-config-space-type", 0);
> }
> - dt_add_property_cells(np, "class-code", rev_class >> 8);
> - dt_add_property_cells(np, "revision-id", rev_class & 0xff);
> - dt_add_property_cells(np, "vendor-id", vdid & 0xffff);
> - dt_add_property_cells(np, "device-id", vdid >> 16);
> + dt_add_property_cells(np, "class-code", PCI_CLASS_CODE(pd->class));
> + dt_add_property_cells(np, "revision-id", PCI_CLASS_REV(pd->class));
> + dt_add_property_cells(np, "vendor-id", PCI_VENDOR_ID(pd->vdid));
> + dt_add_property_cells(np, "device-id", PCI_DEVICE_ID(pd->vdid));
> if (intpin)
> dt_add_property_cells(np, "interrupts", intpin);
>
> @@ -1478,7 +1470,7 @@ static void pci_add_one_device_node(struct phb *phb,
> dt_add_property(np, "reg", reg, sizeof(reg));
>
> /* Print summary info about the device */
> - pci_print_summary_line(phb, pd, np, rev_class, cname);
> + pci_print_summary_line(phb, pd, np, cname);
> if (!pd->is_bridge)
> return;
>
> diff --git a/include/pci.h b/include/pci.h
> index 296fb8d..1c13545 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -75,6 +75,9 @@ struct pci_device {
> #define PCI_VENDOR_ID(x) ((x) & 0xFFFF)
> #define PCI_DEVICE_ID(x) ((x) >> 8)
> uint32_t class;
> +#define PCI_CLASS_REV(x) ((x) & 0xFF)
> +#define PCI_CLASS_CODE(x) ((x) >> 8)
> +#define PCI_CLASS_ID(x) ((x) >> 16)
> uint64_t cap_list;
> struct {
> uint32_t pos;
More information about the Skiboot
mailing list