2008-01-25 13:14:59

by Holger Schurig

[permalink] [raw]
Subject: [PATCH] libertas: fix memory alignment problems on the blackfin

From: Ihar Hrachyshka <[email protected]>

Fixing unaligned memory access on the blackfin architecture (maybe on the
ARM also).

Signed-off-by: Ihar Hrachyshka <[email protected]>
Signed-off-by: Holger Schurig <[email protected]>

Index: wireless-2.6/drivers/net/wireless/libertas/dev.h
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/libertas/dev.h 2008-01-25 10:03:29.000000000 +0100
+++ wireless-2.6/drivers/net/wireless/libertas/dev.h 2008-01-25 14:03:00.000000000 +0100
@@ -349,7 +349,7 @@ struct assoc_request {
u8 channel;
u8 band;
u8 mode;
- u8 bssid[ETH_ALEN];
+ u8 bssid[ETH_ALEN] __attribute__ ((aligned (2)));

/** WEP keys */
struct enc_key wep_keys[4];
Index: wireless-2.6/drivers/net/wireless/libertas/assoc.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/libertas/assoc.c 2008-01-25 14:03:14.000000000 +0100
+++ wireless-2.6/drivers/net/wireless/libertas/assoc.c 2008-01-25 14:03:33.000000000 +0100
@@ -12,8 +12,10 @@
#include "cmd.h"


-static const u8 bssid_any[ETH_ALEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
-static const u8 bssid_off[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+static const u8 bssid_any[ETH_ALEN] __attribute__ ((aligned (2))) =
+ { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+static const u8 bssid_off[ETH_ALEN] __attribute__ ((aligned (2))) =
+ { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };


static int assoc_helper_essid(struct lbs_private *priv,


2008-01-25 15:51:40

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Fri, 2008-01-25 at 14:15 +0100, Holger Schurig wrote:
> From: Ihar Hrachyshka <[email protected]>
>
> Fixing unaligned memory access on the blackfin architecture (maybe on the
> ARM also).
>
> Signed-off-by: Ihar Hrachyshka <[email protected]>
> Signed-off-by: Holger Schurig <[email protected]>

Acked-by: Dan Williams <[email protected]>

>
> Index: wireless-2.6/drivers/net/wireless/libertas/dev.h
> ===================================================================
> --- wireless-2.6.orig/drivers/net/wireless/libertas/dev.h 2008-01-25 10:03:29.000000000 +0100
> +++ wireless-2.6/drivers/net/wireless/libertas/dev.h 2008-01-25 14:03:00.000000000 +0100
> @@ -349,7 +349,7 @@ struct assoc_request {
> u8 channel;
> u8 band;
> u8 mode;
> - u8 bssid[ETH_ALEN];
> + u8 bssid[ETH_ALEN] __attribute__ ((aligned (2)));
>
> /** WEP keys */
> struct enc_key wep_keys[4];
> Index: wireless-2.6/drivers/net/wireless/libertas/assoc.c
> ===================================================================
> --- wireless-2.6.orig/drivers/net/wireless/libertas/assoc.c 2008-01-25 14:03:14.000000000 +0100
> +++ wireless-2.6/drivers/net/wireless/libertas/assoc.c 2008-01-25 14:03:33.000000000 +0100
> @@ -12,8 +12,10 @@
> #include "cmd.h"
>
>
> -static const u8 bssid_any[ETH_ALEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
> -static const u8 bssid_off[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
> +static const u8 bssid_any[ETH_ALEN] __attribute__ ((aligned (2))) =
> + { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
> +static const u8 bssid_off[ETH_ALEN] __attribute__ ((aligned (2))) =
> + { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
>
>
> static int assoc_helper_essid(struct lbs_private *priv,


2008-07-09 13:21:23

by Holger Schurig

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

> Fixing unaligned memory access on the blackfin architecture
> (maybe on the ARM also).

You can delete the "maybe", on ARM (more specificially, on a
PXA255, which is ARMv5TE) I didn't had a problem.

2008-07-09 21:03:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Wed, 2008-07-09 at 23:55 +0300, Ihar Hrachyshka wrote:
> On Wed, Jul 9, 2008 at 11:07 PM, Johannes Berg
> <[email protected]> wrote:
> > On Wed, 2008-07-09 at 23:03 +0300, Ihar Hrachyshka wrote:
> >> On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
> >> <[email protected]> wrote:
> >> >> Fixing unaligned memory access on the blackfin architecture
> >> >> (maybe on the ARM also).
> >> >
> >> > You can delete the "maybe", on ARM (more specificially, on a
> >> > PXA255, which is ARMv5TE) I didn't had a problem.
> >>
> >> Ok, just blackfin. Is it a joke or should I resend the patch with
> >> another description?..
> >
> > I'm sure arm has that problem too and sparc and others, but it may not
> > always show up because the frame might actually be aligned sometimes.
> >
> > OTOH, why are you getting unaligned frames in the first place? This is
> > the beacon interval, so it's at a fixed modulo-two position from the
> > start of the frame.
>
> Do you mean that we should add __attribute__ ((aligned (2))) for our
> 'bss_descriptor' structure definition rather align particular fields
> at runtime?

Oh, no, sorry, I got confused. The firmware apparently lays out the BSS
structure in that way, I was thinking you were getting the beacon/probe
response frame itself. The patch is correct.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-09 20:03:11

by Ihar Hrachyshka

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
<[email protected]> wrote:
>> Fixing unaligned memory access on the blackfin architecture
>> (maybe on the ARM also).
>
> You can delete the "maybe", on ARM (more specificially, on a
> PXA255, which is ARMv5TE) I didn't had a problem.

Ok, just blackfin. Is it a joke or should I resend the patch with
another description?..

>
> _______________________________________________
> libertas-dev mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>

2008-07-09 00:25:12

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Wed, 2008-07-09 at 03:18 +0300, Ihar Hrachyshka wrote:
> Fixing unaligned memory access on the blackfin architecture (maybe on the ARM also).
>

There's helpers for this now: get_unaligned_le16

Cheers,

Harvey


2008-07-10 06:41:26

by Holger Schurig

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

> > You can delete the "maybe", on ARM (more specificially, on a
> > PXA255, which is ARMv5TE) I didn't had a problem.
>
> Ok, just blackfin. Is it a joke or should I resend the patch
> with another description?..

It's neither a joke (I never had any problem with my embedded
device) nor do I insist that you change the commit message. From
my part you'd get an ACK (but I'm not the maintainer of this
code).

2008-07-09 21:07:58

by Ihar Hrachyshka

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
<[email protected]> wrote:
>> Fixing unaligned memory access on the blackfin architecture
>> (maybe on the ARM also).
>
> You can delete the "maybe", on ARM (more specificially, on a
> PXA255, which is ARMv5TE) I didn't had a problem.

As I know, ARMs are too different to say about some "generic" ARM
behavior, in memory alignment also I think.
Too be more correct, the patch fixes alignment problems on all
machines where structures aren't aligned for even addresses by
default, blackfin, arm, or anything else I don't know about.

>
> _______________________________________________
> libertas-dev mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>

2008-07-09 20:08:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

On Wed, 2008-07-09 at 23:03 +0300, Ihar Hrachyshka wrote:
> On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
> <[email protected]> wrote:
> >> Fixing unaligned memory access on the blackfin architecture
> >> (maybe on the ARM also).
> >
> > You can delete the "maybe", on ARM (more specificially, on a
> > PXA255, which is ARMv5TE) I didn't had a problem.
>
> Ok, just blackfin. Is it a joke or should I resend the patch with
> another description?..

I'm sure arm has that problem too and sparc and others, but it may not
always show up because the frame might actually be aligned sometimes.

OTOH, why are you getting unaligned frames in the first place? This is
the beacon interval, so it's at a fixed modulo-two position from the
start of the frame.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-09 06:30:29

by Ihar Hrachyshka

[permalink] [raw]
Subject: Re: [PATCH] libertas: fix memory alignment problems on the blackfin

> There's helpers for this now: get_unaligned_le16
Ok, third pass - the first was in January... :)

Fixing unaligned memory access on the blackfin architecture (maybe on
the ARM also).

Signed-off-by: Ihar Hrachyshka <[email protected]>

---

diff --git a/drivers/net/wireless/libertas/scan.c b/drivers/net/wireless/libertas/scan.c
index 343ed38..4b27456 100644
--- a/drivers/net/wireless/libertas/scan.c
+++ b/drivers/net/wireless/libertas/scan.c
@@ -567,11 +567,11 @@ static int lbs_process_bss(struct bss_descriptor *bss,
pos += 8;

/* beacon interval is 2 bytes long */
- bss->beaconperiod = le16_to_cpup((void *) pos);
+ bss->beaconperiod = get_unaligned_le16(pos);
pos += 2;

/* capability information is 2 bytes long */
- bss->capability = le16_to_cpup((void *) pos);
+ bss->capability = get_unaligned_le16(pos);
lbs_deb_scan("process_bss: capabilities 0x%04x\n", bss->capability);
pos += 2;