linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card
@ 2010-08-22 11:22 Mike Rapoport
  2010-08-22 12:59 ` Marek Vasut
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Rapoport @ 2010-08-22 11:22 UTC (permalink / raw)
  To: linux-wireless; +Cc: libertas-dev, rusty, Mike Rapoport

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 <mike@compulab.co.il>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card
  2010-08-22 11:22 [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card Mike Rapoport
@ 2010-08-22 12:59 ` Marek Vasut
  2010-08-22 13:22   ` Mike Rapoport
  0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2010-08-22 12:59 UTC (permalink / raw)
  To: libertas-dev; +Cc: Mike Rapoport, linux-wireless, rusty

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 <mike@compulab.co.il>
> ---
>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card
  2010-08-22 12:59 ` Marek Vasut
@ 2010-08-22 13:22   ` Mike Rapoport
  2010-08-22 13:57     ` Marek Vasut
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Rapoport @ 2010-08-22 13:22 UTC (permalink / raw)
  To: Marek Vasut; +Cc: libertas-dev, linux-wireless, rusty, Mike Rapoport

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 <mike@compulab.co.il>
>> ---
>>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card
  2010-08-22 13:22   ` Mike Rapoport
@ 2010-08-22 13:57     ` Marek Vasut
  2010-08-25 21:59       ` Dan Williams
  0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2010-08-22 13:57 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: libertas-dev, linux-wireless, rusty

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 <mike@compulab.co.il>
> >> ---
> >> 
> >>  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 <marek.vasut@gmail.com>
> 
> > Cheers
> > --
> > 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card
  2010-08-22 13:57     ` Marek Vasut
@ 2010-08-25 21:59       ` Dan Williams
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2010-08-25 21:59 UTC (permalink / raw)
  To: Marek Vasut; +Cc: Mike Rapoport, rusty, linux-wireless, libertas-dev

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 <mike@compulab.co.il>
> > >> ---
> > >> 
> > >>  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 <marek.vasut@gmail.com>

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-08-25 21:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-22 11:22 [PATCH] libertas: if_sdio: fix buffer alignment in struct if_sdio_card Mike Rapoport
2010-08-22 12:59 ` Marek Vasut
2010-08-22 13:22   ` Mike Rapoport
2010-08-22 13:57     ` Marek Vasut
2010-08-25 21:59       ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).