On Wed, 20 Apr 2011 14:31:33 -0600 Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > -struct pci_cap_saved_state { > - struct hlist_node next; > +struct pci_cap_saved { > char cap_nr; > + unsigned int size; > u32 data[0]; > }; > > +struct pci_cap_saved_state { > + struct hlist_node next; > + struct pci_cap_saved saved; > +}; > + > struct pcie_link_state; > struct pci_vpd; > struct pci_sriov; > @@ -366,7 +371,7 @@ static inline struct pci_cap_saved_state *pci_find_saved_cap( > struct hlist_node *pos; > > hlist_for_each_entry(tmp, pos, &pci_dev->saved_cap_space, next) { > - if (tmp->cap_nr == cap) > + if (tmp->saved.cap_nr == cap) > return tmp; > } > return NULL; Looks pretty good in general. But I think the naming makes it harder to read than it ought to be. So we have a pci_cap_saved_state, which implies capability info, and that's fine. But pci_cap_saved doesn't communicate much; maybe pci_cap_data or pci_cap_saved_data would be better? Thanks, -- Jesse Barnes, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html