2010-08-22 11:23:34

by Mike Rapoport

[permalink] [raw]
Subject: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card

The commit 886275ce41a9751117367fb387ed171049eb6148 (param: lock
if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes)
introduced new fields into the if_sdio_card structure. It caused
missalignment of the if_sdio_card.buffer field and failure at driver
load time:

~# modprobe libertas_sdio
[ 62.315124] libertas_sdio: Libertas SDIO driver
[ 62.319976] libertas_sdio: Copyright Pierre Ossman
[ 63.020629] DMA misaligned error with device 48
[ 63.025207] mmci-omap-hs mmci-omap-hs.1: unexpected dma status 800
[ 66.005035] libertas: command 0x0003 timed out
[ 66.009826] libertas: Timeout submitting command 0x0003
[ 66.016296] libertas: PREP_CMD: command 0x0003 failed: -110

Adding explicit alignment attribute for the if_sdio_card.buffer field
fixes this problem.

Signed-off-by: Mike Rapoport <[email protected]>
---
drivers/net/wireless/libertas/if_sdio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index ba854c7..87b6349 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -128,7 +128,7 @@ struct if_sdio_card {
bool helper_allocated;
bool firmware_allocated;

- u8 buffer[65536];
+ u8 buffer[65536] __attribute__((aligned(4)));

spinlock_t lock;
struct if_sdio_packet *packets;
--
1.6.6.2



2010-08-22 13:58:23

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card

Dne Ne 22. srpna 2010 15:22:08 Mike Rapoport napsal(a):
> Marek Vasut wrote:
> > Dne Ne 22. srpna 2010 13:22:40 Mike Rapoport napsal(a):
> >> The commit 886275ce41a9751117367fb387ed171049eb6148 (param: lock
> >> if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes)
> >> introduced new fields into the if_sdio_card structure. It caused
> >> missalignment of the if_sdio_card.buffer field and failure at driver
> >>
> >> load time:
> >> ~# modprobe libertas_sdio
> >> [ 62.315124] libertas_sdio: Libertas SDIO driver
> >> [ 62.319976] libertas_sdio: Copyright Pierre Ossman
> >> [ 63.020629] DMA misaligned error with device 48
> >> [ 63.025207] mmci-omap-hs mmci-omap-hs.1: unexpected dma status 800
> >> [ 66.005035] libertas: command 0x0003 timed out
> >> [ 66.009826] libertas: Timeout submitting command 0x0003
> >> [ 66.016296] libertas: PREP_CMD: command 0x0003 failed: -110
> >>
> >> Adding explicit alignment attribute for the if_sdio_card.buffer field
> >> fixes this problem.
> >>
> >> Signed-off-by: Mike Rapoport <[email protected]>
> >> ---
> >>
> >> drivers/net/wireless/libertas/if_sdio.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/libertas/if_sdio.c
> >> b/drivers/net/wireless/libertas/if_sdio.c index ba854c7..87b6349 100644
> >> --- a/drivers/net/wireless/libertas/if_sdio.c
> >> +++ b/drivers/net/wireless/libertas/if_sdio.c
> >> @@ -128,7 +128,7 @@ struct if_sdio_card {
> >>
> >> bool helper_allocated;
> >> bool firmware_allocated;
> >>
> >> - u8 buffer[65536];
> >> + u8 buffer[65536] __attribute__((aligned(4)));
> >>
> >> spinlock_t lock;
> >> struct if_sdio_packet *packets;
> >
> > Will this work even on platforms that need some weird DMA destination
> > address alignment ? I might be totally off with this question though ;-)
>
> I've tested it on PXA and OMAP and it works on both.
> I think that on platforms with weird DMA destination address alignment
> libertas_sdio will fail anyway because of if_sdio_packet alignment.

You're right ... thanks for clearing this.

Acked-by: Marek Vasut <[email protected]>
>
> > Cheers
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless"
> > in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-08-22 13:00:05

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card

Dne Ne 22. srpna 2010 13:22:40 Mike Rapoport napsal(a):
> The commit 886275ce41a9751117367fb387ed171049eb6148 (param: lock
> if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes)
> introduced new fields into the if_sdio_card structure. It caused
> missalignment of the if_sdio_card.buffer field and failure at driver
> load time:
>
> ~# modprobe libertas_sdio
> [ 62.315124] libertas_sdio: Libertas SDIO driver
> [ 62.319976] libertas_sdio: Copyright Pierre Ossman
> [ 63.020629] DMA misaligned error with device 48
> [ 63.025207] mmci-omap-hs mmci-omap-hs.1: unexpected dma status 800
> [ 66.005035] libertas: command 0x0003 timed out
> [ 66.009826] libertas: Timeout submitting command 0x0003
> [ 66.016296] libertas: PREP_CMD: command 0x0003 failed: -110
>
> Adding explicit alignment attribute for the if_sdio_card.buffer field
> fixes this problem.
>
> Signed-off-by: Mike Rapoport <[email protected]>
> ---
> drivers/net/wireless/libertas/if_sdio.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/if_sdio.c
> b/drivers/net/wireless/libertas/if_sdio.c index ba854c7..87b6349 100644
> --- a/drivers/net/wireless/libertas/if_sdio.c
> +++ b/drivers/net/wireless/libertas/if_sdio.c
> @@ -128,7 +128,7 @@ struct if_sdio_card {
> bool helper_allocated;
> bool firmware_allocated;
>
> - u8 buffer[65536];
> + u8 buffer[65536] __attribute__((aligned(4)));
>
> spinlock_t lock;
> struct if_sdio_packet *packets;

Will this work even on platforms that need some weird DMA destination address
alignment ? I might be totally off with this question though ;-)

Cheers

2010-08-25 21:57:06

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card

On Sun, 2010-08-22 at 15:57 +0200, Marek Vasut wrote:
> Dne Ne 22. srpna 2010 15:22:08 Mike Rapoport napsal(a):
> > Marek Vasut wrote:
> > > Dne Ne 22. srpna 2010 13:22:40 Mike Rapoport napsal(a):
> > >> The commit 886275ce41a9751117367fb387ed171049eb6148 (param: lock
> > >> if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes)
> > >> introduced new fields into the if_sdio_card structure. It caused
> > >> missalignment of the if_sdio_card.buffer field and failure at driver
> > >>
> > >> load time:
> > >> ~# modprobe libertas_sdio
> > >> [ 62.315124] libertas_sdio: Libertas SDIO driver
> > >> [ 62.319976] libertas_sdio: Copyright Pierre Ossman
> > >> [ 63.020629] DMA misaligned error with device 48
> > >> [ 63.025207] mmci-omap-hs mmci-omap-hs.1: unexpected dma status 800
> > >> [ 66.005035] libertas: command 0x0003 timed out
> > >> [ 66.009826] libertas: Timeout submitting command 0x0003
> > >> [ 66.016296] libertas: PREP_CMD: command 0x0003 failed: -110
> > >>
> > >> Adding explicit alignment attribute for the if_sdio_card.buffer field
> > >> fixes this problem.
> > >>
> > >> Signed-off-by: Mike Rapoport <[email protected]>
> > >> ---
> > >>
> > >> drivers/net/wireless/libertas/if_sdio.c | 2 +-
> > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/drivers/net/wireless/libertas/if_sdio.c
> > >> b/drivers/net/wireless/libertas/if_sdio.c index ba854c7..87b6349 100644
> > >> --- a/drivers/net/wireless/libertas/if_sdio.c
> > >> +++ b/drivers/net/wireless/libertas/if_sdio.c
> > >> @@ -128,7 +128,7 @@ struct if_sdio_card {
> > >>
> > >> bool helper_allocated;
> > >> bool firmware_allocated;
> > >>
> > >> - u8 buffer[65536];
> > >> + u8 buffer[65536] __attribute__((aligned(4)));
> > >>
> > >> spinlock_t lock;
> > >> struct if_sdio_packet *packets;
> > >
> > > Will this work even on platforms that need some weird DMA destination
> > > address alignment ? I might be totally off with this question though ;-)
> >
> > I've tested it on PXA and OMAP and it works on both.
> > I think that on platforms with weird DMA destination address alignment
> > libertas_sdio will fail anyway because of if_sdio_packet alignment.
>
> You're right ... thanks for clearing this.
>
> Acked-by: Marek Vasut <[email protected]>

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



2010-08-22 13:22:55

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card

Marek Vasut wrote:
> Dne Ne 22. srpna 2010 13:22:40 Mike Rapoport napsal(a):
>> The commit 886275ce41a9751117367fb387ed171049eb6148 (param: lock
>> if_sdio's lbs_helper_name and lbs_fw_name against sysfs changes)
>> introduced new fields into the if_sdio_card structure. It caused
>> missalignment of the if_sdio_card.buffer field and failure at driver
>> load time:
>>
>> ~# modprobe libertas_sdio
>> [ 62.315124] libertas_sdio: Libertas SDIO driver
>> [ 62.319976] libertas_sdio: Copyright Pierre Ossman
>> [ 63.020629] DMA misaligned error with device 48
>> [ 63.025207] mmci-omap-hs mmci-omap-hs.1: unexpected dma status 800
>> [ 66.005035] libertas: command 0x0003 timed out
>> [ 66.009826] libertas: Timeout submitting command 0x0003
>> [ 66.016296] libertas: PREP_CMD: command 0x0003 failed: -110
>>
>> Adding explicit alignment attribute for the if_sdio_card.buffer field
>> fixes this problem.
>>
>> Signed-off-by: Mike Rapoport <[email protected]>
>> ---
>> drivers/net/wireless/libertas/if_sdio.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/libertas/if_sdio.c
>> b/drivers/net/wireless/libertas/if_sdio.c index ba854c7..87b6349 100644
>> --- a/drivers/net/wireless/libertas/if_sdio.c
>> +++ b/drivers/net/wireless/libertas/if_sdio.c
>> @@ -128,7 +128,7 @@ struct if_sdio_card {
>> bool helper_allocated;
>> bool firmware_allocated;
>>
>> - u8 buffer[65536];
>> + u8 buffer[65536] __attribute__((aligned(4)));
>>
>> spinlock_t lock;
>> struct if_sdio_packet *packets;
>
> Will this work even on platforms that need some weird DMA destination address
> alignment ? I might be totally off with this question though ;-)

I've tested it on PXA and OMAP and it works on both.
I think that on platforms with weird DMA destination address alignment
libertas_sdio will fail anyway because of if_sdio_packet alignment.

> Cheers
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


--
Sincerely yours,
Mike.