* [PATCH RESEND] usbtty: fix possible alignment issues
[not found] <CGME20200107052020epcas1p2a5809802832bcf3b8305c6be49e87c00@epcas1p2.samsung.com>
@ 2020-01-07 5:25 ` Seung-Woo Kim
2020-01-07 13:22 ` Tom Rini
0 siblings, 1 reply; 4+ messages in thread
From: Seung-Woo Kim @ 2020-01-07 5:25 UTC (permalink / raw)
To: u-boot
With gcc9, below warnings are shown:
drivers/serial/usbtty.c: In function 'usbtty_init_strings':
drivers/serial/usbtty.c:590:44: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
590 | str2wide (CONFIG_USBD_MANUFACTURER, string->wData);
| ~~~~~~^~~~~~~
drivers/serial/usbtty.c:596:44: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
597 | str2wide (CONFIG_USBD_PRODUCT_NAME, string->wData);
| ~~~~~~^~~~~~~
drivers/serial/usbtty.c:603:33: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
604 | str2wide (serial_number, string->wData);
| ~~~~~~^~~~~~~
drivers/serial/usbtty.c:610:49: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
611 | str2wide (CONFIG_USBD_CONFIGURATION_STR, string->wData);
| ~~~~~~^~~~~~~
drivers/serial/usbtty.c:617:50: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
618 | str2wide (CONFIG_USBD_DATA_INTERFACE_STR, string->wData);
| ~~~~~~^~~~~~~
drivers/serial/usbtty.c:623:50: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
624 | str2wide (CONFIG_USBD_CTRL_INTERFACE_STR, string->wData);
| ~~~~~~^~~~~~~
Fix the issues by using packed structure.
Ref: commit 616ebd8b9cb4 ("usb: composite: fix possible alignment issues")
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
resend with proper e-mail address
---
drivers/serial/usbtty.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index f1c1a260da..54e67dd0d1 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -48,6 +48,8 @@
#define CONFIG_USBD_DATA_INTERFACE_STR "Bulk Data Interface"
#define CONFIG_USBD_CTRL_INTERFACE_STR "Control Interface"
+typedef struct { __le16 val; } __attribute__((aligned(16))) __le16_packed;
+
/*
* Buffers to hold input and output data
*/
@@ -372,14 +374,15 @@ static int fill_buffer (circbuf_t * buf);
void usbtty_poll (void);
/* utility function for converting char* to wide string used by USB */
-static void str2wide (char *str, u16 * wide)
+static void str2wide (char *str, void *wide)
{
int i;
+ __le16_packed *tmp = wide;
for (i = 0; i < strlen (str) && str[i]; i++){
#if defined(__LITTLE_ENDIAN)
- wide[i] = (u16) str[i];
+ tmp[i].val = (u16) str[i];
#elif defined(__BIG_ENDIAN)
- wide[i] = ((u16)(str[i])<<8);
+ tmp[i].val = ((u16)(str[i])<<8);
#else
#error "__LITTLE_ENDIAN or __BIG_ENDIAN undefined"
#endif
--
2.19.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH RESEND] usbtty: fix possible alignment issues
2020-01-07 5:25 ` [PATCH RESEND] usbtty: fix possible alignment issues Seung-Woo Kim
@ 2020-01-07 13:22 ` Tom Rini
2020-01-08 0:32 ` Seung-Woo Kim
0 siblings, 1 reply; 4+ messages in thread
From: Tom Rini @ 2020-01-07 13:22 UTC (permalink / raw)
To: u-boot
On Tue, Jan 07, 2020 at 02:25:02PM +0900, Seung-Woo Kim wrote:
> With gcc9, below warnings are shown:
> drivers/serial/usbtty.c: In function 'usbtty_init_strings':
> drivers/serial/usbtty.c:590:44: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 590 | str2wide (CONFIG_USBD_MANUFACTURER, string->wData);
> | ~~~~~~^~~~~~~
> drivers/serial/usbtty.c:596:44: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 597 | str2wide (CONFIG_USBD_PRODUCT_NAME, string->wData);
> | ~~~~~~^~~~~~~
> drivers/serial/usbtty.c:603:33: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 604 | str2wide (serial_number, string->wData);
> | ~~~~~~^~~~~~~
> drivers/serial/usbtty.c:610:49: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 611 | str2wide (CONFIG_USBD_CONFIGURATION_STR, string->wData);
> | ~~~~~~^~~~~~~
> drivers/serial/usbtty.c:617:50: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 618 | str2wide (CONFIG_USBD_DATA_INTERFACE_STR, string->wData);
> | ~~~~~~^~~~~~~
> drivers/serial/usbtty.c:623:50: warning: taking address of packed member of 'struct usb_string_descriptor' may result in an unaligned pointer value [-Waddress-of-packed-member]
> 624 | str2wide (CONFIG_USBD_CTRL_INTERFACE_STR, string->wData);
> | ~~~~~~^~~~~~~
>
> Fix the issues by using packed structure.
>
> Ref: commit 616ebd8b9cb4 ("usb: composite: fix possible alignment issues")
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> ---
> resend with proper e-mail address
> ---
> drivers/serial/usbtty.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> index f1c1a260da..54e67dd0d1 100644
> --- a/drivers/serial/usbtty.c
> +++ b/drivers/serial/usbtty.c
> @@ -48,6 +48,8 @@
> #define CONFIG_USBD_DATA_INTERFACE_STR "Bulk Data Interface"
> #define CONFIG_USBD_CTRL_INTERFACE_STR "Control Interface"
>
> +typedef struct { __le16 val; } __attribute__((aligned(16))) __le16_packed;
> +
> /*
> * Buffers to hold input and output data
> */
> @@ -372,14 +374,15 @@ static int fill_buffer (circbuf_t * buf);
> void usbtty_poll (void);
>
> /* utility function for converting char* to wide string used by USB */
> -static void str2wide (char *str, u16 * wide)
> +static void str2wide (char *str, void *wide)
> {
> int i;
> + __le16_packed *tmp = wide;
> for (i = 0; i < strlen (str) && str[i]; i++){
> #if defined(__LITTLE_ENDIAN)
> - wide[i] = (u16) str[i];
> + tmp[i].val = (u16) str[i];
> #elif defined(__BIG_ENDIAN)
> - wide[i] = ((u16)(str[i])<<8);
> + tmp[i].val = ((u16)(str[i])<<8);
> #else
> #error "__LITTLE_ENDIAN or __BIG_ENDIAN undefined"
> #endif
We generally don't want packed structs and do need to disable this
warning in general. Is this _really_ a problem, or are we just
silencing the compiler here? Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200107/d43451a4/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH RESEND] usbtty: fix possible alignment issues
2020-01-07 13:22 ` Tom Rini
@ 2020-01-08 0:32 ` Seung-Woo Kim
2020-01-08 0:46 ` Tom Rini
0 siblings, 1 reply; 4+ messages in thread
From: Seung-Woo Kim @ 2020-01-08 0:32 UTC (permalink / raw)
To: u-boot
Hi,
On 2020년 01월 07일 22:22, Tom Rini wrote:
> On Tue, Jan 07, 2020 at 02:25:02PM +0900, Seung-Woo Kim wrote:
>
...
>> ---
>> drivers/serial/usbtty.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
>> index f1c1a260da..54e67dd0d1 100644
>> --- a/drivers/serial/usbtty.c
>> +++ b/drivers/serial/usbtty.c
>> @@ -48,6 +48,8 @@
>> #define CONFIG_USBD_DATA_INTERFACE_STR "Bulk Data Interface"
>> #define CONFIG_USBD_CTRL_INTERFACE_STR "Control Interface"
>>
>> +typedef struct { __le16 val; } __attribute__((aligned(16))) __le16_packed;
>> +
>> /*
>> * Buffers to hold input and output data
>> */
>> @@ -372,14 +374,15 @@ static int fill_buffer (circbuf_t * buf);
>> void usbtty_poll (void);
>>
>> /* utility function for converting char* to wide string used by USB */
>> -static void str2wide (char *str, u16 * wide)
>> +static void str2wide (char *str, void *wide)
>> {
>> int i;
>> + __le16_packed *tmp = wide;
>> for (i = 0; i < strlen (str) && str[i]; i++){
>> #if defined(__LITTLE_ENDIAN)
>> - wide[i] = (u16) str[i];
>> + tmp[i].val = (u16) str[i];
>> #elif defined(__BIG_ENDIAN)
>> - wide[i] = ((u16)(str[i])<<8);
>> + tmp[i].val = ((u16)(str[i])<<8);
>> #else
>> #error "__LITTLE_ENDIAN or __BIG_ENDIAN undefined"
>> #endif
>
> We generally don't want packed structs and do need to disable this
> warning in general. Is this _really_ a problem, or are we just
> silencing the compiler here? Thanks!
The change makes just silence for the warnings as like composite.c in
usb gadget.
Regards,
- Seung-Woo Kim
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH RESEND] usbtty: fix possible alignment issues
2020-01-08 0:32 ` Seung-Woo Kim
@ 2020-01-08 0:46 ` Tom Rini
0 siblings, 0 replies; 4+ messages in thread
From: Tom Rini @ 2020-01-08 0:46 UTC (permalink / raw)
To: u-boot
On Wed, Jan 08, 2020 at 09:32:03AM +0900, Seung-Woo Kim wrote:
> Hi,
>
> On 2020년 01월 07일 22:22, Tom Rini wrote:
> > On Tue, Jan 07, 2020 at 02:25:02PM +0900, Seung-Woo Kim wrote:
> >
> ...
> >> ---
> >> drivers/serial/usbtty.c | 9 ++++++---
> >> 1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> >> index f1c1a260da..54e67dd0d1 100644
> >> --- a/drivers/serial/usbtty.c
> >> +++ b/drivers/serial/usbtty.c
> >> @@ -48,6 +48,8 @@
> >> #define CONFIG_USBD_DATA_INTERFACE_STR "Bulk Data Interface"
> >> #define CONFIG_USBD_CTRL_INTERFACE_STR "Control Interface"
> >>
> >> +typedef struct { __le16 val; } __attribute__((aligned(16))) __le16_packed;
> >> +
> >> /*
> >> * Buffers to hold input and output data
> >> */
> >> @@ -372,14 +374,15 @@ static int fill_buffer (circbuf_t * buf);
> >> void usbtty_poll (void);
> >>
> >> /* utility function for converting char* to wide string used by USB */
> >> -static void str2wide (char *str, u16 * wide)
> >> +static void str2wide (char *str, void *wide)
> >> {
> >> int i;
> >> + __le16_packed *tmp = wide;
> >> for (i = 0; i < strlen (str) && str[i]; i++){
> >> #if defined(__LITTLE_ENDIAN)
> >> - wide[i] = (u16) str[i];
> >> + tmp[i].val = (u16) str[i];
> >> #elif defined(__BIG_ENDIAN)
> >> - wide[i] = ((u16)(str[i])<<8);
> >> + tmp[i].val = ((u16)(str[i])<<8);
> >> #else
> >> #error "__LITTLE_ENDIAN or __BIG_ENDIAN undefined"
> >> #endif
> >
> > We generally don't want packed structs and do need to disable this
> > warning in general. Is this _really_ a problem, or are we just
> > silencing the compiler here? Thanks!
>
> The change makes just silence for the warnings as like composite.c in
> usb gadget.
OK, thanks. NAK on this and I will pick up the general change to
disable this warning soon.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200107/d6b056e6/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-08 0:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20200107052020epcas1p2a5809802832bcf3b8305c6be49e87c00@epcas1p2.samsung.com>
2020-01-07 5:25 ` [PATCH RESEND] usbtty: fix possible alignment issues Seung-Woo Kim
2020-01-07 13:22 ` Tom Rini
2020-01-08 0:32 ` Seung-Woo Kim
2020-01-08 0:46 ` Tom Rini
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.