* [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues
@ 2019-11-12 21:26 Simon Goldschmidt
2019-11-12 21:26 ` [U-Boot] [PATCH 2/2] usb: dwc2: " Simon Goldschmidt
2019-11-21 19:36 ` [U-Boot] [PATCH 1/2] usb: composite: " Heinrich Schuchardt
0 siblings, 2 replies; 4+ messages in thread
From: Simon Goldschmidt @ 2019-11-12 21:26 UTC (permalink / raw)
To: u-boot
Since upgrading to gcc9, warnings are issued:
"taking address of packed member of ‘...’ may result in an unaligned
pointer value"
Fix this by converting two functions to use unaligned access since packed
structures may be on an unaligned address, depending on USB hardware.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
drivers/usb/gadget/composite.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 618a7d5016..cfc9512caa 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -12,8 +12,16 @@
#define USB_BUFSIZ 4096
+/* Helper type for accessing packed u16 pointers */
+typedef struct { __le16 val; } __packed __le16_packed;
+
static struct usb_composite_driver *composite;
+static inline void le16_add_cpu_packed(__le16_packed *var, u16 val)
+{
+ var->val = cpu_to_le16(le16_to_cpu(var->val) + val);
+}
+
/**
* usb_add_function() - add a function to a configuration
* @config: the configuration
@@ -480,20 +488,20 @@ done:
* the host side.
*/
-static void collect_langs(struct usb_gadget_strings **sp, __le16 *buf)
+static void collect_langs(struct usb_gadget_strings **sp, void *buf)
{
const struct usb_gadget_strings *s;
u16 language;
- __le16 *tmp;
+ __le16_packed *tmp;
while (*sp) {
s = *sp;
language = cpu_to_le16(s->language);
- for (tmp = buf; *tmp && tmp < &buf[126]; tmp++) {
- if (*tmp == language)
+ for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
+ if (tmp->val == language)
goto repeat;
}
- *tmp++ = language;
+ tmp->val = language;
repeat:
sp++;
}
@@ -705,7 +713,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
*/
usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
bos->bNumDeviceCaps++;
- le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
+ le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
+ USB_DT_USB_EXT_CAP_SIZE);
usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
@@ -721,7 +730,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
ss_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
bos->bNumDeviceCaps++;
- le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE);
+ le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
+ USB_DT_USB_SS_CAP_SIZE);
ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH 2/2] usb: dwc2: fix possible alignment issues
2019-11-12 21:26 [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
@ 2019-11-12 21:26 ` Simon Goldschmidt
2019-11-21 19:36 ` [U-Boot] [PATCH 1/2] usb: composite: " Heinrich Schuchardt
1 sibling, 0 replies; 4+ messages in thread
From: Simon Goldschmidt @ 2019-11-12 21:26 UTC (permalink / raw)
To: u-boot
Since upgrading to gcc9, warnings are issued:
"taking address of packed member of ‘...’ may result in an unaligned
pointer value"
Fix this by converting dwc2_fifo_read to use unaligned access since packed
structures may be on an unaligned address, depending on USB hardware.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
index 7eb632d3b1..dba221dad0 100644
--- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
+++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
@@ -731,7 +731,7 @@ static int write_fifo_ep0(struct dwc2_ep *ep, struct dwc2_request *req)
return 0;
}
-static int dwc2_fifo_read(struct dwc2_ep *ep, u32 *cp, int max)
+static int dwc2_fifo_read(struct dwc2_ep *ep, void *cp, int max)
{
invalidate_dcache_range((unsigned long)cp, (unsigned long)cp +
ROUND(max, CONFIG_SYS_CACHELINE_SIZE));
@@ -1285,7 +1285,7 @@ static void dwc2_ep0_setup(struct dwc2_udc *dev)
nuke(ep, -EPROTO);
/* read control req from fifo (8 bytes) */
- dwc2_fifo_read(ep, (u32 *)usb_ctrl, 8);
+ dwc2_fifo_read(ep, usb_ctrl, 8);
debug_cond(DEBUG_SETUP != 0,
"%s: bRequestType = 0x%x(%s), bRequest = 0x%x"
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues
2019-11-12 21:26 [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
2019-11-12 21:26 ` [U-Boot] [PATCH 2/2] usb: dwc2: " Simon Goldschmidt
@ 2019-11-21 19:36 ` Heinrich Schuchardt
2019-11-21 21:12 ` Simon Goldschmidt
1 sibling, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2019-11-21 19:36 UTC (permalink / raw)
To: u-boot
On 11/12/19 10:26 PM, Simon Goldschmidt wrote:
> Since upgrading to gcc9, warnings are issued:
> "taking address of packed member of ‘...’ may result in an unaligned
> pointer value"
>
> Fix this by converting two functions to use unaligned access since packed
> structures may be on an unaligned address, depending on USB hardware.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
>
> drivers/usb/gadget/composite.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 618a7d5016..cfc9512caa 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -12,8 +12,16 @@
>
> #define USB_BUFSIZ 4096
>
> +/* Helper type for accessing packed u16 pointers */
> +typedef struct { __le16 val; } __packed __le16_packed;
> +
> static struct usb_composite_driver *composite;
>
> +static inline void le16_add_cpu_packed(__le16_packed *var, u16 val)
> +{
> + var->val = cpu_to_le16(le16_to_cpu(var->val) + val);
> +}
> +
> /**
> * usb_add_function() - add a function to a configuration
> * @config: the configuration
> @@ -480,20 +488,20 @@ done:
> * the host side.
> */
>
> -static void collect_langs(struct usb_gadget_strings **sp, __le16 *buf)
> +static void collect_langs(struct usb_gadget_strings **sp, void *buf)
With this patch and GCC 9.2.1 I get:
In file included from drivers/usb/gadget/g_dnl.c:24:
drivers/usb/gadget/composite.c: In function ‘collect_langs’:
drivers/usb/gadget/composite.c:500:41: error: dereferencing ‘void *’
pointer [-Werror]
500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
| ^
drivers/usb/gadget/composite.c:500:35: error: comparison of distinct
pointer types lacks a cast [-Werror]
500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
void does not have defined size and so cannot be subscripted.
Best regards
Heinrich
> {
> const struct usb_gadget_strings *s;
> u16 language;
> - __le16 *tmp;
> + __le16_packed *tmp;
>
> while (*sp) {
> s = *sp;
> language = cpu_to_le16(s->language);
> - for (tmp = buf; *tmp && tmp < &buf[126]; tmp++) {
> - if (*tmp == language)
> + for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
> + if (tmp->val == language)
> goto repeat;
> }
> - *tmp++ = language;
> + tmp->val = language;
> repeat:
> sp++;
> }
> @@ -705,7 +713,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
> */
> usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> bos->bNumDeviceCaps++;
> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
> + USB_DT_USB_EXT_CAP_SIZE);
> usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> @@ -721,7 +730,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
>
> ss_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> bos->bNumDeviceCaps++;
> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE);
> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
> + USB_DT_USB_SS_CAP_SIZE);
> ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
> ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues
2019-11-21 19:36 ` [U-Boot] [PATCH 1/2] usb: composite: " Heinrich Schuchardt
@ 2019-11-21 21:12 ` Simon Goldschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Simon Goldschmidt @ 2019-11-21 21:12 UTC (permalink / raw)
To: u-boot
Am 21.11.2019 um 20:36 schrieb Heinrich Schuchardt:
> On 11/12/19 10:26 PM, Simon Goldschmidt wrote:
>> Since upgrading to gcc9, warnings are issued:
>> "taking address of packed member of ‘...’ may result in an unaligned
>> pointer value"
>>
>> Fix this by converting two functions to use unaligned access since packed
>> structures may be on an unaligned address, depending on USB hardware.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>>
>> drivers/usb/gadget/composite.c | 24 +++++++++++++++++-------
>> 1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> index 618a7d5016..cfc9512caa 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -12,8 +12,16 @@
>>
>> #define USB_BUFSIZ 4096
>>
>> +/* Helper type for accessing packed u16 pointers */
>> +typedef struct { __le16 val; } __packed __le16_packed;
>> +
>> static struct usb_composite_driver *composite;
>>
>> +static inline void le16_add_cpu_packed(__le16_packed *var, u16 val)
>> +{
>> + var->val = cpu_to_le16(le16_to_cpu(var->val) + val);
>> +}
>> +
>> /**
>> * usb_add_function() - add a function to a configuration
>> * @config: the configuration
>> @@ -480,20 +488,20 @@ done:
>> * the host side.
>> */
>>
>> -static void collect_langs(struct usb_gadget_strings **sp, __le16 *buf)
>> +static void collect_langs(struct usb_gadget_strings **sp, void *buf)
>
> With this patch and GCC 9.2.1 I get:
>
> In file included from drivers/usb/gadget/g_dnl.c:24:
> drivers/usb/gadget/composite.c: In function ‘collect_langs’:
> drivers/usb/gadget/composite.c:500:41: error: dereferencing ‘void *’
> pointer [-Werror]
> 500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
> | ^
> drivers/usb/gadget/composite.c:500:35: error: comparison of distinct
> pointer types lacks a cast [-Werror]
> 500 | for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
>
> void does not have defined size and so cannot be subscripted.
Right, thanks for testing. I hate it that we don't have -Werror enabled,
as this just slipped through with all the output on the screen.
I'll send v2 fixing that.
Regards,
Simon
>
> Best regards
>
> Heinrich
>
>> {
>> const struct usb_gadget_strings *s;
>> u16 language;
>> - __le16 *tmp;
>> + __le16_packed *tmp;
>>
>> while (*sp) {
>> s = *sp;
>> language = cpu_to_le16(s->language);
>> - for (tmp = buf; *tmp && tmp < &buf[126]; tmp++) {
>> - if (*tmp == language)
>> + for (tmp = buf; tmp->val && tmp < &buf[126]; tmp++) {
>> + if (tmp->val == language)
>> goto repeat;
>> }
>> - *tmp++ = language;
>> + tmp->val = language;
>> repeat:
>> sp++;
>> }
>> @@ -705,7 +713,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
>> */
>> usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>> bos->bNumDeviceCaps++;
>> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
>> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
>> + USB_DT_USB_EXT_CAP_SIZE);
>> usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
>> usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>> usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
>> @@ -721,7 +730,8 @@ static int bos_desc(struct usb_composite_dev *cdev)
>>
>> ss_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>> bos->bNumDeviceCaps++;
>> - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE);
>> + le16_add_cpu_packed((__le16_packed *)&bos->wTotalLength,
>> + USB_DT_USB_SS_CAP_SIZE);
>> ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
>> ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>> ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-21 21:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-12 21:26 [U-Boot] [PATCH 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
2019-11-12 21:26 ` [U-Boot] [PATCH 2/2] usb: dwc2: " Simon Goldschmidt
2019-11-21 19:36 ` [U-Boot] [PATCH 1/2] usb: composite: " Heinrich Schuchardt
2019-11-21 21:12 ` Simon Goldschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox