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

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

[Holger Schurig]

From: Ihar Hrachyshka <ihar.hrachyshka@promwad.com>

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

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@promwad.com>
Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

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,

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

[Dan Williams]

On Fri, 2008-01-25 at 14:15 +0100, Holger Schurig wrote:
> From: Ihar Hrachyshka <ihar.hrachyshka@promwad.com>
> 
> Fixing unaligned memory access on the blackfin architecture (maybe on the 
> ARM also).
> 
> Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@promwad.com>
> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Acked-by: Dan Williams <dcbw@redhat.com>

> 
> 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,

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

[Ihar Hrachyshka]

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

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@promwad.com>

---

diff --git a/drivers/net/wireless/libertas/scan.c b/drivers/net/wireless/libertas/scan.c
index 343ed38..9dbfad3 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 = le16_to_cpu(get_unaligned((__le16 *)pos));
	pos += 2;

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

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

[Harvey Harrison]

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

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

[Ihar Hrachyshka]

> 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 <ihar.hrachyshka@promwad.com>

---

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;

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

[Holger Schurig]

> 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.

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

[Ihar Hrachyshka]

On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
<hs4233@mail.mn-solutions.de> 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
> libertas-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Wed, 2008-07-09 at 23:03 +0300, Ihar Hrachyshka wrote:
> On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
> <hs4233@mail.mn-solutions.de> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Ihar Hrachyshka]

On Wed, Jul 9, 2008 at 11:07 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2008-07-09 at 23:03 +0300, Ihar Hrachyshka wrote:
>> On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
>> <hs4233@mail.mn-solutions.de> 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?

>
> johannes
>

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

On Wed, 2008-07-09 at 23:55 +0300, Ihar Hrachyshka wrote:
> On Wed, Jul 9, 2008 at 11:07 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2008-07-09 at 23:03 +0300, Ihar Hrachyshka wrote:
> >> On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
> >> <hs4233@mail.mn-solutions.de> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Ihar Hrachyshka]

On Wed, Jul 9, 2008 at 4:21 PM, Holger Schurig
<hs4233@mail.mn-solutions.de> 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
> libertas-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>

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

[Holger Schurig]

> > 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).

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

[Dan Williams]

On Wed, 2008-07-09 at 09:29 +0300, Ihar Hrachyshka wrote:
> > 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 <ihar.hrachyshka@promwad.com>

Acked-by: Dan Williams <dcbw@redhat.com>

> ---
> 
> 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;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html