[SLOF] [PATCH] libusb: Fix compiler warnings with GCC 9 in the USB code

Thomas Huth thuth at redhat.com
Fri Sep 20 19:06:43 AEST 2019


GCC 9 emits a lot of compiler warnings in the USB code like this:

[...]
usb-ohci.c: In function ‘ohci_get_last_frame’:
usb-ohci.c:972:20: warning: taking address of packed member of ‘struct ohci_regs’
 may result in an unaligned pointer value [-Waddress-of-packed-member]
  972 |  return read_reg32(&regs->fm_num);
      |                    ^~~~~~~~~~~~~
	[CC]	usb-ehci.o
usb-ehci.c: In function ‘ehci_hub_check_ports’:
usb-ehci.c:63:25: warning: taking address of packed member of ‘struct ehci_cap_regs’
 may result in an unaligned pointer value [-Waddress-of-packed-member]
   63 |  num_ports = read_reg32(&ehcd->cap_regs->hcsparams) & HCS_NPORTS_MASK;
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~
usb-ehci.c:66:23: warning: taking address of packed member of ‘struct ehci_op_regs’
 may result in an unaligned pointer value [-Waddress-of-packed-member]
   66 |   portsc = read_reg32(&ehcd->op_regs->portsc[i]);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~
[...]
usb-xhci.c:227:20: warning: taking address of packed member of ‘struct xhci_op_regs’
 may result in an unaligned pointer value [-Waddress-of-packed-member]
  227 |  xhci_wait_for_cnr(&op->usbsts);
      |                    ^~~~~~~~~~~
usb-xhci.c: In function ‘xhci_poll_event’:
usb-xhci.c:309:14: warning: taking address of packed member of ‘struct xhci_int_regs’
 may result in an unaligned pointer value [-Waddress-of-packed-member]
  309 |  write_reg64(&xhcd->run_regs->irs[0].erdp, val);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
[...]

Most of the structures are naturally aligned, so we can simply drop the
"packed" attribute and use a _Static_assert() instead to make sure that
there is no padding here.

The only struct that needed some more attention is struct ehci_cap_regs.
The portroute field is not aligned to a natural boundary here. But
looking at the EHCI spec, it seems like this field is not meant as
a full 64-bit value anyway, but rather an array of nibbles, so switch
it to an array instead, then we can also drop the "packed" attribute
here.

Signed-off-by: Thomas Huth <thuth at redhat.com>
---
 lib/libusb/usb-ehci.c |  6 +++++-
 lib/libusb/usb-ehci.h |  8 +++++---
 lib/libusb/usb-ohci.h |  3 ++-
 lib/libusb/usb-xhci.h | 25 ++++++++++++++++++-------
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/lib/libusb/usb-ehci.c b/lib/libusb/usb-ehci.c
index 60af9e1..299e9da 100644
--- a/lib/libusb/usb-ehci.c
+++ b/lib/libusb/usb-ehci.c
@@ -38,7 +38,11 @@ static void dump_ehci_regs(struct ehci_hcd *ehcd)
 	dprintf("\n - HCIVERSION          %04X", read_reg16(&cap_regs->hciversion));
 	dprintf("\n - HCSPARAMS           %08X", read_reg32(&cap_regs->hcsparams));
 	dprintf("\n - HCCPARAMS           %08X", read_reg32(&cap_regs->hccparams));
-	dprintf("\n - HCSP_PORTROUTE      %016llX", read_reg64(&cap_regs->portroute));
+	dprintf("\n - HCSP_PORTROUTE      %02X %02X %02X %02X %02X %02X %02X %02X",
+	        read_reg8(&cap_regs->portroute[0]), read_reg8(&cap_regs->portroute[1]),
+	        read_reg8(&cap_regs->portroute[2]), read_reg8(&cap_regs->portroute[3]),
+	        read_reg8(&cap_regs->portroute[4]), read_reg8(&cap_regs->portroute[5]),
+	        read_reg8(&cap_regs->portroute[6]), read_reg8(&cap_regs->portroute[7]));
 	dprintf("\n");
 
 	dprintf("\n - USBCMD              %08X", read_reg32(&op_regs->usbcmd));
diff --git a/lib/libusb/usb-ehci.h b/lib/libusb/usb-ehci.h
index 2955a9c..a834e95 100644
--- a/lib/libusb/usb-ehci.h
+++ b/lib/libusb/usb-ehci.h
@@ -28,8 +28,9 @@ struct ehci_cap_regs {
 	uint16_t hciversion;
 	uint32_t hcsparams;
 	uint32_t hccparams;
-	uint64_t portroute;
-} __attribute__ ((packed));
+	uint8_t  portroute[8];
+};
+_Static_assert(sizeof(struct ehci_cap_regs) == 20, "padding in ehci_cap_regs");
 
 struct ehci_op_regs {
 	uint32_t usbcmd;
@@ -42,7 +43,8 @@ struct ehci_op_regs {
 	uint32_t reserved[9];
 	uint32_t configflag;
 	uint32_t portsc[0];
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct ehci_op_regs) == 68, "padding in ehci_op_regs");
 
 struct ehci_framelist {
 	uint32_t fl_ptr[FL_SIZE];
diff --git a/lib/libusb/usb-ohci.h b/lib/libusb/usb-ohci.h
index f4535fd..1f38047 100644
--- a/lib/libusb/usb-ohci.h
+++ b/lib/libusb/usb-ohci.h
@@ -47,7 +47,8 @@ struct ohci_regs {
 	uint32_t rh_desc_b;
 	uint32_t rh_status;
 	uint32_t rh_ps[5];
-} __attribute__((packed));
+};
+_Static_assert(sizeof(struct ohci_regs) == 104, "padding in ohci_regs");
 
 #define EDA_FADDR(x)     ((x & 0x7F))
 #define EDA_EP(x)        ((x & 0x0F) << 7)
diff --git a/lib/libusb/usb-xhci.h b/lib/libusb/usb-xhci.h
index 02b382b..07c3957 100644
--- a/lib/libusb/usb-xhci.h
+++ b/lib/libusb/usb-xhci.h
@@ -37,7 +37,8 @@ struct xhci_cap_regs {
 #define XHCI_HCCPARAMS_XECP(x)  ((x & 0xFFFF0000) >> 16)
 	uint32_t dboff;
 	uint32_t rtsoff;
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_cap_regs) == 28, "padding in xhci_cap_regs");
 
 /* USB 3.0: Section 7 and 7.2 */
 #define XHCI_XECP_CAP_ID(x)     ((x & 0xF))
@@ -90,7 +91,8 @@ struct xhci_port_regs {
 	uint32_t portpmsc;
 	uint32_t portli;
 	uint32_t reserved;
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_port_regs) == 16, "padding in xhci_port_regs");
 
 struct port_state {
 	bool    PP;
@@ -132,7 +134,10 @@ struct xhci_op_regs {
 	/* USB Port register set */
 #define XHCI_PORT_MAX 256
 	struct xhci_port_regs prs[XHCI_PORT_MAX];
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_op_regs) == 0x400 +
+               XHCI_PORT_MAX * sizeof(struct xhci_port_regs),
+               "padding in xhci_op_regs");
 
 /*
  * 5.5.2  Interrupter Register Set
@@ -148,7 +153,8 @@ struct xhci_int_regs {
 #define XHCI_ERST_ADDR_MASK (~(0x3FUL))
 	uint64_t erdp;
 #define XHCI_ERDP_MASK      (~(0xFUL))
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_int_regs) == 32, "padding in xhci_int_regs");
 
 /* 5.5 Host Controller Runtime Registers */
 struct xhci_run_regs {
@@ -156,12 +162,16 @@ struct xhci_run_regs {
 	uint8_t reserved[28];
 #define XHCI_IRS_MAX 1024
 	struct xhci_int_regs irs[XHCI_IRS_MAX];
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_run_regs) == 32 +
+               XHCI_IRS_MAX * sizeof(struct xhci_int_regs),
+               "padding in xhci_run_regs");
 
 /* 5.6 Doorbell Registers*/
 struct xhci_db_regs {
 	uint32_t db[256];
-}  __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_db_regs) == 1024, "padding in xhci_db_regs");
 
 #define COMP_SUCCESS         1
 
@@ -259,7 +269,8 @@ struct xhci_erst_entry {
 	uint64_t addr;
 	uint32_t size;
 	uint32_t reserved;
-} __attribute__ ((packed));
+};
+_Static_assert(sizeof(struct xhci_erst_entry) == 16, "padding in xhci_erst_entry");
 
 struct xhci_erst {
 	struct xhci_erst_entry *entries;
-- 
2.18.1



More information about the SLOF mailing list