Message ID | 1354838564-6033-1-git-send-email-colin.king@canonical.com |
---|---|
State | Accepted |
Headers | show |
On Fri, Dec 7, 2012 at 8:02 AM, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > Since we are mapping the pcie_capabiliy struct 1-to-1 onto real > data we should give it a packed attribute to ensure GCC does not > stuff in extra padding. > > I took the liberty of making this a typedef to make it a little > simpler than having packed structs everywhere. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/pci/aspm/aspm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c > index e6d91ca..1a6cc27 100644 > --- a/src/pci/aspm/aspm.c > +++ b/src/pci/aspm/aspm.c > @@ -61,7 +61,7 @@ struct pci_device { > /* PCI Express Capability Structure is defined in Section 7.8 > * of PCI Express®i Base Specification Revision 2.0 > */ > -struct pcie_capability { > +typedef struct { > uint8_t pcie_cap_id; > uint8_t next_cap_point; > uint16_t pcie_cap_reg; > @@ -86,7 +86,7 @@ struct pcie_capability { > uint32_t slot_cap2; > uint16_t slot_contrl2; > uint16_t slot_status2; > -}; > +} __attribute__ ((packed)) pcie_capability; > > static int facp_get_aspm_control(fwts_framework *fw, int *aspm) > { > @@ -118,28 +118,28 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw, > struct pci_device *rp, > struct pci_device *dev) > { > - struct pcie_capability *rp_cap, *device_cap; > + pcie_capability *rp_cap, *device_cap; > uint8_t rp_aspm_cntrl, device_aspm_cntrl; > uint8_t next_cap; > int ret = FWTS_OK; > bool l0s_disabled = false, l1_disabled = false; > > next_cap = rp->config[FWTS_PCI_CAPABILITIES_POINTER]; > - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; > + rp_cap = (pcie_capability *) &rp->config[next_cap]; > while (rp_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { > if (rp_cap->next_cap_point == FWTS_PCI_LAST_ID) > break; > next_cap = rp_cap->next_cap_point; > - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; > + rp_cap = (pcie_capability *) &rp->config[next_cap]; > } > > next_cap = dev->config[FWTS_PCI_CAPABILITIES_POINTER]; > - device_cap = (struct pcie_capability *) &dev->config[next_cap]; > + device_cap = (pcie_capability *)&dev->config[next_cap]; > while (device_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { > if (device_cap->next_cap_point == FWTS_PCI_LAST_ID) > break; > next_cap = device_cap->next_cap_point; > - device_cap = (struct pcie_capability *) &dev->config[next_cap]; > + device_cap = (pcie_capability *)&dev->config[next_cap]; > } > > > -- > 1.8.0 > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
On 12/07/2012 08:02 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Since we are mapping the pcie_capabiliy struct 1-to-1 onto real > data we should give it a packed attribute to ensure GCC does not > stuff in extra padding. > > I took the liberty of making this a typedef to make it a little > simpler than having packed structs everywhere. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/pci/aspm/aspm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c > index e6d91ca..1a6cc27 100644 > --- a/src/pci/aspm/aspm.c > +++ b/src/pci/aspm/aspm.c > @@ -61,7 +61,7 @@ struct pci_device { > /* PCI Express Capability Structure is defined in Section 7.8 > * of PCI Express®i Base Specification Revision 2.0 > */ > -struct pcie_capability { > +typedef struct { > uint8_t pcie_cap_id; > uint8_t next_cap_point; > uint16_t pcie_cap_reg; > @@ -86,7 +86,7 @@ struct pcie_capability { > uint32_t slot_cap2; > uint16_t slot_contrl2; > uint16_t slot_status2; > -}; > +} __attribute__ ((packed)) pcie_capability; > > static int facp_get_aspm_control(fwts_framework *fw, int *aspm) > { > @@ -118,28 +118,28 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw, > struct pci_device *rp, > struct pci_device *dev) > { > - struct pcie_capability *rp_cap, *device_cap; > + pcie_capability *rp_cap, *device_cap; > uint8_t rp_aspm_cntrl, device_aspm_cntrl; > uint8_t next_cap; > int ret = FWTS_OK; > bool l0s_disabled = false, l1_disabled = false; > > next_cap = rp->config[FWTS_PCI_CAPABILITIES_POINTER]; > - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; > + rp_cap = (pcie_capability *) &rp->config[next_cap]; > while (rp_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { > if (rp_cap->next_cap_point == FWTS_PCI_LAST_ID) > break; > next_cap = rp_cap->next_cap_point; > - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; > + rp_cap = (pcie_capability *) &rp->config[next_cap]; > } > > next_cap = dev->config[FWTS_PCI_CAPABILITIES_POINTER]; > - device_cap = (struct pcie_capability *) &dev->config[next_cap]; > + device_cap = (pcie_capability *)&dev->config[next_cap]; > while (device_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { > if (device_cap->next_cap_point == FWTS_PCI_LAST_ID) > break; > next_cap = device_cap->next_cap_point; > - device_cap = (struct pcie_capability *) &dev->config[next_cap]; > + device_cap = (pcie_capability *)&dev->config[next_cap]; > } > > > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c index e6d91ca..1a6cc27 100644 --- a/src/pci/aspm/aspm.c +++ b/src/pci/aspm/aspm.c @@ -61,7 +61,7 @@ struct pci_device { /* PCI Express Capability Structure is defined in Section 7.8 * of PCI Express®i Base Specification Revision 2.0 */ -struct pcie_capability { +typedef struct { uint8_t pcie_cap_id; uint8_t next_cap_point; uint16_t pcie_cap_reg; @@ -86,7 +86,7 @@ struct pcie_capability { uint32_t slot_cap2; uint16_t slot_contrl2; uint16_t slot_status2; -}; +} __attribute__ ((packed)) pcie_capability; static int facp_get_aspm_control(fwts_framework *fw, int *aspm) { @@ -118,28 +118,28 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw, struct pci_device *rp, struct pci_device *dev) { - struct pcie_capability *rp_cap, *device_cap; + pcie_capability *rp_cap, *device_cap; uint8_t rp_aspm_cntrl, device_aspm_cntrl; uint8_t next_cap; int ret = FWTS_OK; bool l0s_disabled = false, l1_disabled = false; next_cap = rp->config[FWTS_PCI_CAPABILITIES_POINTER]; - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; + rp_cap = (pcie_capability *) &rp->config[next_cap]; while (rp_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { if (rp_cap->next_cap_point == FWTS_PCI_LAST_ID) break; next_cap = rp_cap->next_cap_point; - rp_cap = (struct pcie_capability *) &rp->config[next_cap]; + rp_cap = (pcie_capability *) &rp->config[next_cap]; } next_cap = dev->config[FWTS_PCI_CAPABILITIES_POINTER]; - device_cap = (struct pcie_capability *) &dev->config[next_cap]; + device_cap = (pcie_capability *)&dev->config[next_cap]; while (device_cap->pcie_cap_id != FWTS_PCI_EXPRESS_CAP_ID) { if (device_cap->next_cap_point == FWTS_PCI_LAST_ID) break; next_cap = device_cap->next_cap_point; - device_cap = (struct pcie_capability *) &dev->config[next_cap]; + device_cap = (pcie_capability *)&dev->config[next_cap]; }