Please note: This project is no longer active. The website is kept online for historic purposes only.
If you´re looking for a Linux driver for your Atheros WLAN device, you should continue here .

Ticket #1280 (closed defect: fixed)

Opened 12 years ago

Last modified 12 years ago

Bitfield endianess not handled correctly

Reported by: rudger@hopling.com Assigned to: proski
Priority: major Milestone: version 0.9.5
Component: madwifi: driver Version: trunk
Keywords: xscale be byte swap Cc:
Patch is attached: 1 Pending:

Description

We are using madwifi (0.9.3) on ar5414/13 devices on longer links (1 to 4 km). We've noticed reproducible behavior of having good throughput (20 to 25Mbps) and after a few seconds the rate drops (3 till 10Mbps) and never comes back again.

After a lot of debugging efforts in the rate algorithms I found the problem lies in the tx_complete descriptors being little endian read while the host (xscale-be) is interpreting them as big endian. (Part of this is already described in ticket #617 ).

It looks like the Xscale-arm kernel doesn't byte swap pci register while other big endian targets do. So the attached patch will add the _bswap code again (formaly removed) but now only enable it for Xscale BE platforms using a define in the hal header file for Xscale-be.

Using this fix the rate algoritms seems to work a lot better.

Attachments

xscale-be-descriptor-swap.patch (3.3 kB) - added by rudger@hopling.com on 04/24/07 21:39:42.
Patch fixing byte swap for xscale be platforms.
big-endian-ratectrl.patch (13.6 kB) - added by rudger on 04/28/07 00:01:55.
Patch fixing bitmask usage of struct ar5212_desc in sample.c

Change History

04/24/07 21:39:42 changed by rudger@hopling.com

  • attachment xscale-be-descriptor-swap.patch added.

Patch fixing byte swap for xscale be platforms.

04/24/07 21:42:34 changed by rudger@hopling.com

Signed-off-by: R. van Brenk <rudger@hopling.com>

(follow-up: ↓ 3 ) 04/25/07 03:09:11 changed by mentor

What platform are you running? Is the hardware attached in Big Endian or Little Endian mode?

(in reply to: ↑ 2 ) 04/25/07 09:01:26 changed by anonymous

Replying to mentor:

What platform are you running? Is the hardware attached in Big Endian or Little Endian mode?

As the subject says Xscale BE so Big Endian! It's an ADI phronghorn metro board with an ixp425 in bigendian mode and Senao NMP8601 radio card (AR5414 mac).

04/25/07 20:44:17 changed by mentor

Sorry, I'll be more clear, is the IO bus connected in BE or LE mode?

04/25/07 21:41:39 changed by mentor

OK. So the problem here is that you have a processor running in Big Endian mode, but the hardware/bus attached in Little Endian mode. Fixing this simply requires adding the relevant byte swapping to the register read macros. However, I do not know how to detect which mode hardware is attached in.

04/26/07 00:30:06 changed by rudger

Hmm I found this in some old old madwifi mailing list from 2005.

From: Mark Rakes <mrakes@ma...>
Re: [Madwifi-devel] Re: Hi, Everyone. I find a interesting issue about "HAL status 3" error, " ath0: hardware error; reseting "error and "-DAH_BYTE_ORDER=AH_BIG_ENDIAN" option.  
2005-01-29 00:17

be aware that the patches Steffen and I were discussing were specific
to ixp42x-xscale target (which needs readl/writel to access pci space).
the patch also swaps which address range is "bswapped" because both the
xscale hardware and the atheros hardware was doing the le-to-be swap in
hardware, thus making the data little-endian again.

hope this helps.
-mark

I'm not exactly sure what is meant here, and if this is true is there any way to stop the atheros mac/hal swapping bytes?

04/28/07 00:00:10 changed by rudger@hopling.com

Hmm. Today I found that the previous patch doesn't solve anything and does make no difference. So we can skip that, and I still don't know why I concluded it fixed my problems.

Ok looking more at the problem i found out my arm-linux-gcc version 3.4.4 is having problems decoding the bitmasks in struct ar5212_desc (the file sample.h). This is were all the strange values come from...

I've attached a patch which will create define macros in hal/ah_desc.h and the patch will remove usage of the struct ar5212_desc in sample.c

I'm sorry the patch is looking a bit ugly but there is no time left since I'm not having vary much time next week to clean up things.

Note: The patch isn't tested fully on LE systems, also the new rate algorithm minstrel is using these bitmasks and thus will also fail.

04/28/07 00:01:55 changed by rudger

  • attachment big-endian-ratectrl.patch added.

Patch fixing bitmask usage of struct ar5212_desc in sample.c

04/28/07 00:02:40 changed by rudger@hopling.com

Signed-off-by: R. van Brenk <rudger@hopling.com>

(follow-up: ↓ 12 ) 04/28/07 23:09:15 changed by mentor

  • summary changed from [PATCH] Rate control out of control on Xscale BE to ARM GCC-3.4 bitfield support is broken.

What is the status of GCC on ARM? Does a later version of GCC work properly?

05/03/07 14:10:28 changed by mentor

Hmmm, the bitfields ar listed as uint32_t, so they should work as unsigned values... However, a common problem seems to be confusion over whether bit fields are signed or unsigned. Maybe you could try with -funsigned-bitfields?

05/29/07 14:59:00 changed by proski

  • status changed from new to assigned.
  • owner set to proski.

xscale-be-descriptor-swap.patch is unacceptable because it changes non-free HAL files.

(in reply to: ↑ 9 ; follow-up: ↓ 13 ) 06/04/07 20:01:09 changed by Mark Glines <mark@glines.org>

Replying to mentor:

What is the status of GCC on ARM? Does a later version of GCC work properly?

I have tried a hacked up gcc-3.4.1 I've been using for years, as well as 2 revs of gcc used by Gentoo - 3.4.4 and 4.1.2. They all show the same behavior, which is consistent with what I would expect from a big endian platform. It's possible that the creator of this ticket has a broken gcc, or he might just be running on a big endian platform.

The problem is that the field order within the bitfield is declared correctly for little endian platforms, but not big endian ones. The structure needs to be redefined with this in mind, using <asm/byteorder.h> and LITTLE_ENDIAN_BITFIELD / BIG_ENDIAN_BITFIELD ifdef clauses. (Those macro names have two leading underscores each, but it looks like the wiki markup eats them.)

(in reply to: ↑ 12 ) 06/04/07 20:07:11 changed by Mark Glines <mark@glines.org>

Replying to Mark Glines <mark@glines.org>:

The problem is that the field order within the bitfield is declared correctly for little endian platforms, but not big endian ones. The structure needs to be redefined with this in mind, using <asm/byteorder.h> and LITTLE_ENDIAN_BITFIELD / BIG_ENDIAN_BITFIELD ifdef clauses. (Those macro names have two leading underscores each, but it looks like the wiki markup eats them.)

I also strongly suspect this issue has already been fixed, in svn r2360. rudger@hopling.com, would you mind retesting?

Mark

06/04/07 21:14:30 changed by mentor

  • summary changed from ARM GCC-3.4 bitfield support is broken to Bitfield endianess not handled correctly.

Spiffy. This means I can fix support without having to use icky macros.

06/21/07 00:53:06 changed by rudger@hopling.com

svn r2360 indeed fixes the problem.

06/27/07 09:02:00 changed by mtaylor

  • status changed from assigned to closed.
  • resolution set to fixed.

Closing, per feedback.

06/27/07 12:39:22 changed by mrenzmann

  • milestone set to version 0.9.4.

02/11/08 06:18:06 changed by mrenzmann

  • milestone changed from version 0.9.4 to version 0.9.5.