On Tue, 2024-01-16 at 23:34 +0200, andy.shevchenko@xxxxxxxxx wrote: > Mon, Jan 15, 2024 at 03:46:17PM +0100, Philipp Stanner kirjoitti: > > The bit describing whether the PCI device is currently pinned is > > stored > > in the PCI devres struct. To clean up and simplify the pci-devres > > API, > > "PCI devres", 'pci-devres', ... > Shouldn't these (and across entire series) be consistent terms? > E.g., "PCI devres API". Yes, we should agree on a canonical term probably. Probably "PCI devres" is good. > > > it's better if this information is stored in the pci_dev struct, > > because > > pci_dev struct --> struct pci_dev > > > it allows for checking that device's pinned-status directly through > > the > > device struct. > > This will later permit simplifying pcim_enable_device(). > > > Move the 'pinned' boolean bit to struct pci_dev. > > ... > > > u8 pm_cap; /* PM capability offset */ > > unsigned int enabled:1; /* Whether this dev is > > enabled */ > > + unsigned int pinned:1; /* Whether this dev is > > pinned */ > > unsigned int imm_ready:1; /* Supports Immediate > > Readiness */ > > unsigned int pme_support:5; /* Bitmask of states from > > which PME# > > can be generated */ > > First of all, I think it's better to group PM stuff, like ACK > > u8 pm_cap; /* PM capability offset */ > unsigned int pme_support:5; /* Bitmask of states from > which PME# > can be generated */ > unsigned int imm_ready:1; /* Supports Immediate > Readiness */ > unsigned int enabled:1; /* Whether this dev is > enabled */ > unsigned int pinned:1; /* Whether this dev is pinned > */ > > Second, does this layout anyhow related to the HW layout? (For > example, > PME bits and their location in some HW register vs. these bitfields) > If so, but not sure, it might be good to preserve (to some extent) > the > order. As far as I know, one of the greatest weaknesses of C is that neither Ritchie nor the standard ever said anything about the fields' order. Hence, every field-order is purely implementation-defined and in no way portable. So I can't imagine a bitfield will ever directly mapp to a HW-layout? The fields order is purely aesthetic / for the human reader. P. >