[Skiboot] [PATCH 3/4] core/pci: Use cached vendor/device/class IDs on populating device node
Gavin Shan
gwshan at linux.vnet.ibm.com
Mon Jun 19 13:29:52 AEST 2017
On Thu, Jun 15, 2017 at 03:32:51PM +1000, Benjamin Herrenschmidt wrote:
>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.
>
Ok. I'll drop the part changing (class | revid), and keep the code
using the cached vdid.
Cheers,
Gavin
>> 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