* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
@ 2019-11-21 21:15 Simon Goldschmidt
2019-11-21 21:15 ` [U-Boot] [PATCH v2 2/2] usb: dwc2: " Simon Goldschmidt
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Simon Goldschmidt @ 2019-11-21 21:15 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>
---
Changes in v2:
- fix compiler warning "dereferencing ‘void *'"
drivers/usb/gadget/composite.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 618a7d5016..c98a444245 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,21 @@ 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;
+ __le16_packed *end = (buf + 252);
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 < end; tmp++) {
+ if (tmp->val == language)
goto repeat;
}
- *tmp++ = language;
+ tmp->val = language;
repeat:
sp++;
}
@@ -705,7 +714,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 +731,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] 12+ messages in thread
* [U-Boot] [PATCH v2 2/2] usb: dwc2: fix possible alignment issues
2019-11-21 21:15 [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
@ 2019-11-21 21:15 ` Simon Goldschmidt
2019-11-21 22:39 ` Lukasz Majewski
2019-11-21 22:38 ` [U-Boot] [PATCH v2 1/2] usb: composite: " Lukasz Majewski
2019-11-22 0:25 ` Marek Vasut
2 siblings, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2019-11-21 21:15 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>
---
Changes in v2: None
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] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-21 21:15 [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
2019-11-21 21:15 ` [U-Boot] [PATCH v2 2/2] usb: dwc2: " Simon Goldschmidt
@ 2019-11-21 22:38 ` Lukasz Majewski
2019-11-22 0:25 ` Marek Vasut
2 siblings, 0 replies; 12+ messages in thread
From: Lukasz Majewski @ 2019-11-21 22:38 UTC (permalink / raw)
To: u-boot
On Thu, 21 Nov 2019 22:15:22 +0100
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> 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>
> ---
>
> Changes in v2:
> - fix compiler warning "dereferencing ‘void *'"
>
> drivers/usb/gadget/composite.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c
> b/drivers/usb/gadget/composite.c index 618a7d5016..c98a444245 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,21 @@ 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;
> + __le16_packed *end = (buf + 252);
>
> 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 < end; tmp++) {
> + if (tmp->val == language)
> goto repeat;
> }
> - *tmp++ = language;
> + tmp->val = language;
> repeat:
> sp++;
> }
> @@ -705,7 +714,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 +731,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;
Acked-by: Lukasz Majewski <lukma@denx.de>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191121/28c4c32c/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/2] usb: dwc2: fix possible alignment issues
2019-11-21 21:15 ` [U-Boot] [PATCH v2 2/2] usb: dwc2: " Simon Goldschmidt
@ 2019-11-21 22:39 ` Lukasz Majewski
0 siblings, 0 replies; 12+ messages in thread
From: Lukasz Majewski @ 2019-11-21 22:39 UTC (permalink / raw)
To: u-boot
On Thu, 21 Nov 2019 22:15:23 +0100
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> 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 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>
> ---
>
> Changes in v2: None
>
> 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"
Acked-by: Lukasz Majewski <lukma@denx.de>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191121/a089ca0f/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-21 21:15 [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
2019-11-21 21:15 ` [U-Boot] [PATCH v2 2/2] usb: dwc2: " Simon Goldschmidt
2019-11-21 22:38 ` [U-Boot] [PATCH v2 1/2] usb: composite: " Lukasz Majewski
@ 2019-11-22 0:25 ` Marek Vasut
2019-11-22 6:50 ` Heinrich Schuchardt
2 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2019-11-22 0:25 UTC (permalink / raw)
To: u-boot
On 11/21/19 10:15 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>
Applied both, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 0:25 ` Marek Vasut
@ 2019-11-22 6:50 ` Heinrich Schuchardt
2019-11-22 7:47 ` Simon Goldschmidt
0 siblings, 1 reply; 12+ messages in thread
From: Heinrich Schuchardt @ 2019-11-22 6:50 UTC (permalink / raw)
To: u-boot
On 11/22/19 1:25 AM, Marek Vasut wrote:
> On 11/21/19 10:15 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>
>
> Applied both, thanks.
>
With these two patches applied to origin/master GCC 9.2.1 does not
produce warnings anymore but for tbs2910_defconfig:
u-boot.imx exceeds file size limit:
limit: 0x5fc00 bytes
actual: 0x60c00 bytes
excess: 0x1000 bytes
make: *** [Makefile:1159: u-boot.imx] Error 1
make: *** Deleting file 'u-boot.imx'
So irrespective of my patches for the USB keyboard we need to address
the size limit on TBS2910.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 6:50 ` Heinrich Schuchardt
@ 2019-11-22 7:47 ` Simon Goldschmidt
2019-11-22 11:58 ` Heinrich Schuchardt
0 siblings, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2019-11-22 7:47 UTC (permalink / raw)
To: u-boot
On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/22/19 1:25 AM, Marek Vasut wrote:
> > On 11/21/19 10:15 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>
> >
> > Applied both, thanks.
> >
>
> With these two patches applied to origin/master GCC 9.2.1 does not
> produce warnings anymore but for tbs2910_defconfig:
>
> u-boot.imx exceeds file size limit:
> limit: 0x5fc00 bytes
> actual: 0x60c00 bytes
> excess: 0x1000 bytes
> make: *** [Makefile:1159: u-boot.imx] Error 1
> make: *** Deleting file 'u-boot.imx'
>
> So irrespective of my patches for the USB keyboard we need to address
> the size limit on TBS2910.
Is that due to my cleanup patches? Can you compare the size by compiling
without them? That should work if you leave away the -Werror switch.
Regards,
Simon
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 7:47 ` Simon Goldschmidt
@ 2019-11-22 11:58 ` Heinrich Schuchardt
2019-11-22 12:14 ` Marek Vasut
0 siblings, 1 reply; 12+ messages in thread
From: Heinrich Schuchardt @ 2019-11-22 11:58 UTC (permalink / raw)
To: u-boot
On 11/22/19 8:47 AM, Simon Goldschmidt wrote:
> On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 11/22/19 1:25 AM, Marek Vasut wrote:
>>> On 11/21/19 10:15 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>
>>>
>>> Applied both, thanks.
>>>
>>
>> With these two patches applied to origin/master GCC 9.2.1 does not
>> produce warnings anymore but for tbs2910_defconfig:
>>
>> u-boot.imx exceeds file size limit:
>> limit: 0x5fc00 bytes
>> actual: 0x60c00 bytes
>> excess: 0x1000 bytes
>> make: *** [Makefile:1159: u-boot.imx] Error 1
>> make: *** Deleting file 'u-boot.imx'
>>
>> So irrespective of my patches for the USB keyboard we need to address
>> the size limit on TBS2910.
>
> Is that due to my cleanup patches? Can you compare the size by compiling
> without them? That should work if you leave away the -Werror switch.
>
> Regards,
> Simon
GCC 9.2.1 without your patches but with -Wno-address-of-packed-member:
u-boot.imx exceeds file size limit:
limit: 0x5fc00 bytes
actual: 0x60c00 bytes
excess: 0x1000 bytes
Best regards
Heinrich
>
>>
>> Best regards
>>
>> Heinrich
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 11:58 ` Heinrich Schuchardt
@ 2019-11-22 12:14 ` Marek Vasut
2019-11-22 17:32 ` Heinrich Schuchardt
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2019-11-22 12:14 UTC (permalink / raw)
To: u-boot
On 11/22/19 12:58 PM, Heinrich Schuchardt wrote:
> On 11/22/19 8:47 AM, Simon Goldschmidt wrote:
>> On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt
>> <xypron.glpk@gmx.de> wrote:
>>>
>>> On 11/22/19 1:25 AM, Marek Vasut wrote:
>>>> On 11/21/19 10:15 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>
>>>>
>>>> Applied both, thanks.
>>>>
>>>
>>> With these two patches applied to origin/master GCC 9.2.1 does not
>>> produce warnings anymore but for tbs2910_defconfig:
>>>
>>> u-boot.imx exceeds file size limit:
>>> limit: 0x5fc00 bytes
>>> actual: 0x60c00 bytes
>>> excess: 0x1000 bytes
>>> make: *** [Makefile:1159: u-boot.imx] Error 1
>>> make: *** Deleting file 'u-boot.imx'
>>>
>>> So irrespective of my patches for the USB keyboard we need to address
>>> the size limit on TBS2910.
>>
>> Is that due to my cleanup patches? Can you compare the size by compiling
>> without them? That should work if you leave away the -Werror switch.
>>
>> Regards,
>> Simon
>
> GCC 9.2.1 without your patches but with -Wno-address-of-packed-member:
>
> u-boot.imx exceeds file size limit:
> limit: 0x5fc00 bytes
> actual: 0x60c00 bytes
> excess: 0x1000 bytes
I see, so you have additional options added to the build which trigger
the size issue. It would be nice to mention that up front. Do you use
any other extra options ?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 12:14 ` Marek Vasut
@ 2019-11-22 17:32 ` Heinrich Schuchardt
2019-11-22 18:10 ` Tom Rini
2019-11-22 18:28 ` Marek Vasut
0 siblings, 2 replies; 12+ messages in thread
From: Heinrich Schuchardt @ 2019-11-22 17:32 UTC (permalink / raw)
To: u-boot
On 11/22/19 1:14 PM, Marek Vasut wrote:
> On 11/22/19 12:58 PM, Heinrich Schuchardt wrote:
>> On 11/22/19 8:47 AM, Simon Goldschmidt wrote:
>>> On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt
>>> <xypron.glpk@gmx.de> wrote:
>>>>
>>>> On 11/22/19 1:25 AM, Marek Vasut wrote:
>>>>> On 11/21/19 10:15 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>
>>>>>
>>>>> Applied both, thanks.
>>>>>
>>>>
>>>> With these two patches applied to origin/master GCC 9.2.1 does not
>>>> produce warnings anymore but for tbs2910_defconfig:
>>>>
>>>> u-boot.imx exceeds file size limit:
>>>> limit: 0x5fc00 bytes
>>>> actual: 0x60c00 bytes
>>>> excess: 0x1000 bytes
>>>> make: *** [Makefile:1159: u-boot.imx] Error 1
>>>> make: *** Deleting file 'u-boot.imx'
>>>>
>>>> So irrespective of my patches for the USB keyboard we need to address
>>>> the size limit on TBS2910.
>>>
>>> Is that due to my cleanup patches? Can you compare the size by compiling
>>> without them? That should work if you leave away the -Werror switch.
>>>
>>> Regards,
>>> Simon
>>
>> GCC 9.2.1 without your patches but with -Wno-address-of-packed-member:
>>
>> u-boot.imx exceeds file size limit:
>> limit: 0x5fc00 bytes
>> actual: 0x60c00 bytes
>> excess: 0x1000 bytes
>
> I see, so you have additional options added to the build which trigger
> the size issue. It would be nice to mention that up front. Do you use
> any other extra options ?
>
Dear Marek,
Simon asked me to determine if origin/master exceeds the u-boot.imx size
limit when compiled without his patches. The only way to do so is to
suppress the build warnings.
-Wno-address-of-packed-member is the only option I added to
origin/master. This option suppresses the build error that we get
without Simon's patches.
The only other difference between Gitlab CI and my setup is that I use
GCC 9.2.1 (arm-linux-gnueabihf-gcc on Debian Bullseye).
These are the sizes of u-boot.bin:
390080 bytes with Simon's patches
390080 bytes with Simon's patches + -Wno-address-of-packed-member
390064 bytes origin/master + -Wno-address-of-packed-member
386248 bytes with Simon's patches + CONFIG_REGEX=n
390024 bytes with Simon patches +
"usb: kbd: simplify coding for arrow keys"
390440 bytes with Simon patches + all my USB keyboard patches
So I will add a CONFIG option to my "usb: kbd: implement special keys"
patch to avoid the code increase.
With CONFIG_REGEX=n the image fits into the current board limit in all
cases.
Soeren could you, please, evaluate if this configuration works with your
board. It should only be relevant if multiple network interfaces are
supported by U-Boot.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 17:32 ` Heinrich Schuchardt
@ 2019-11-22 18:10 ` Tom Rini
2019-11-22 18:28 ` Marek Vasut
1 sibling, 0 replies; 12+ messages in thread
From: Tom Rini @ 2019-11-22 18:10 UTC (permalink / raw)
To: u-boot
On Fri, Nov 22, 2019 at 06:32:13PM +0100, Heinrich Schuchardt wrote:
> On 11/22/19 1:14 PM, Marek Vasut wrote:
> > On 11/22/19 12:58 PM, Heinrich Schuchardt wrote:
> > > On 11/22/19 8:47 AM, Simon Goldschmidt wrote:
> > > > On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt
> > > > <xypron.glpk@gmx.de> wrote:
> > > > >
> > > > > On 11/22/19 1:25 AM, Marek Vasut wrote:
> > > > > > On 11/21/19 10:15 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>
> > > > > >
> > > > > > Applied both, thanks.
> > > > > >
> > > > >
> > > > > With these two patches applied to origin/master GCC 9.2.1 does not
> > > > > produce warnings anymore but for tbs2910_defconfig:
> > > > >
> > > > > u-boot.imx exceeds file size limit:
> > > > > limit: 0x5fc00 bytes
> > > > > actual: 0x60c00 bytes
> > > > > excess: 0x1000 bytes
> > > > > make: *** [Makefile:1159: u-boot.imx] Error 1
> > > > > make: *** Deleting file 'u-boot.imx'
> > > > >
> > > > > So irrespective of my patches for the USB keyboard we need to address
> > > > > the size limit on TBS2910.
> > > >
> > > > Is that due to my cleanup patches? Can you compare the size by compiling
> > > > without them? That should work if you leave away the -Werror switch.
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > GCC 9.2.1 without your patches but with -Wno-address-of-packed-member:
> > >
> > > u-boot.imx exceeds file size limit:
> > > limit: 0x5fc00 bytes
> > > actual: 0x60c00 bytes
> > > excess: 0x1000 bytes
> >
> > I see, so you have additional options added to the build which trigger
> > the size issue. It would be nice to mention that up front. Do you use
> > any other extra options ?
> >
>
> Dear Marek,
>
> Simon asked me to determine if origin/master exceeds the u-boot.imx size
> limit when compiled without his patches. The only way to do so is to
> suppress the build warnings.
>
> -Wno-address-of-packed-member is the only option I added to
> origin/master. This option suppresses the build error that we get
> without Simon's patches.
For the record, with gcc 7.2, these types of fixes result in:
u-boot: add: 0/0, grow: 2/0 bytes: 36/0 (36)
function old new delta
collect_langs 76 100 +24
composite_setup 2440 2452 +12
spl-u-boot-spl: add: 0/0, grow: 2/0 bytes: 36/0 (36)
function old new delta
collect_langs 76 100 +24
composite_setup 2440 2452 +12
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191122/cb0ea46b/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues
2019-11-22 17:32 ` Heinrich Schuchardt
2019-11-22 18:10 ` Tom Rini
@ 2019-11-22 18:28 ` Marek Vasut
1 sibling, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2019-11-22 18:28 UTC (permalink / raw)
To: u-boot
On 11/22/19 6:32 PM, Heinrich Schuchardt wrote:
> On 11/22/19 1:14 PM, Marek Vasut wrote:
>> On 11/22/19 12:58 PM, Heinrich Schuchardt wrote:
>>> On 11/22/19 8:47 AM, Simon Goldschmidt wrote:
>>>> On Fri, Nov 22, 2019 at 7:50 AM Heinrich Schuchardt
>>>> <xypron.glpk@gmx.de> wrote:
>>>>>
>>>>> On 11/22/19 1:25 AM, Marek Vasut wrote:
>>>>>> On 11/21/19 10:15 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>
>>>>>>
>>>>>> Applied both, thanks.
>>>>>>
>>>>>
>>>>> With these two patches applied to origin/master GCC 9.2.1 does not
>>>>> produce warnings anymore but for tbs2910_defconfig:
>>>>>
>>>>> u-boot.imx exceeds file size limit:
>>>>> limit: 0x5fc00 bytes
>>>>> actual: 0x60c00 bytes
>>>>> excess: 0x1000 bytes
>>>>> make: *** [Makefile:1159: u-boot.imx] Error 1
>>>>> make: *** Deleting file 'u-boot.imx'
>>>>>
>>>>> So irrespective of my patches for the USB keyboard we need to address
>>>>> the size limit on TBS2910.
>>>>
>>>> Is that due to my cleanup patches? Can you compare the size by
>>>> compiling
>>>> without them? That should work if you leave away the -Werror switch.
>>>>
>>>> Regards,
>>>> Simon
>>>
>>> GCC 9.2.1 without your patches but with -Wno-address-of-packed-member:
>>>
>>> u-boot.imx exceeds file size limit:
>>> limit: 0x5fc00 bytes
>>> actual: 0x60c00 bytes
>>> excess: 0x1000 bytes
>>
>> I see, so you have additional options added to the build which trigger
>> the size issue. It would be nice to mention that up front. Do you use
>> any other extra options ?
>>
>
> Dear Marek,
Hi,
> Simon asked me to determine if origin/master exceeds the u-boot.imx size
> limit when compiled without his patches. The only way to do so is to
> suppress the build warnings.
>
> -Wno-address-of-packed-member is the only option I added to
> origin/master. This option suppresses the build error that we get
> without Simon's patches.
But you somehow got this -Werror into the build too, right ?
> The only other difference between Gitlab CI and my setup is that I use
> GCC 9.2.1 (arm-linux-gnueabihf-gcc on Debian Bullseye).
That's what I use too.
> These are the sizes of u-boot.bin:
>
> 390080 bytes with Simon's patches
> 390080 bytes with Simon's patches + -Wno-address-of-packed-member
> 390064 bytes origin/master + -Wno-address-of-packed-member
> 386248 bytes with Simon's patches + CONFIG_REGEX=n
> 390024 bytes with Simon patches +
> "usb: kbd: simplify coding for arrow keys"
> 390440 bytes with Simon patches + all my USB keyboard patches
>
> So I will add a CONFIG option to my "usb: kbd: implement special keys"
> patch to avoid the code increase.
There might be another option. How about improving the encoding of these
escape sequences? There seem to be a lot of duplication, e.g. the
leading '\e' is in all of them. So is the '[' . Maybe deduplicating
those could help ?
I did a quick try by putting all the sequences into a table, then
removed the \e and sent it explicitly with usb_kbd_put_queue(), and got
~100 bytes saved. And then each of those strings has trailing '\0',
which I don't think is needed either, so that might be another few bytes
saved.
$ arm-linux-gnueabi-readelf -s u-boot | sort -nk 3 | grep usb_kbd
can help tracking down these bloat hotspots.
> With CONFIG_REGEX=n the image fits into the current board limit in all
> cases.
>
> Soeren could you, please, evaluate if this configuration works with your
> board. It should only be relevant if multiple network interfaces are
> supported by U-Boot.
CONFIG_REGEX is used by setexpr to handle regexes on U-Boot command
line. Hence, removing CONFIG_REGEX would degrade the board functionality
and that is what I don't want to see.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-22 18:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-21 21:15 [U-Boot] [PATCH v2 1/2] usb: composite: fix possible alignment issues Simon Goldschmidt
2019-11-21 21:15 ` [U-Boot] [PATCH v2 2/2] usb: dwc2: " Simon Goldschmidt
2019-11-21 22:39 ` Lukasz Majewski
2019-11-21 22:38 ` [U-Boot] [PATCH v2 1/2] usb: composite: " Lukasz Majewski
2019-11-22 0:25 ` Marek Vasut
2019-11-22 6:50 ` Heinrich Schuchardt
2019-11-22 7:47 ` Simon Goldschmidt
2019-11-22 11:58 ` Heinrich Schuchardt
2019-11-22 12:14 ` Marek Vasut
2019-11-22 17:32 ` Heinrich Schuchardt
2019-11-22 18:10 ` Tom Rini
2019-11-22 18:28 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.