Message-ID: <52346DE2.9090904@lwfinger.net> (sfid-20130914_160854_856363_843118B5)
Date: Sat, 14 Sep 2013 09:08:34 -0500
From: Larry Finger <Larry.Finger@lwfinger.net>
MIME-Version: 1.0
To: Jason Andrews <jasona@cadence.com>
CC: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: guidance on struct alignment for rtl8192cu driver
References: <985A2B0C3F73B74792F35D4098E1CA7CB143670E93@MAILSJ3.global.cadence.com>
In-Reply-To: <985A2B0C3F73B74792F35D4098E1CA7CB143670E93@MAILSJ3.global.cadence.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Sender: linux-wireless-owner@vger.kernel.org

On 09/14/2013 12:36 AM, Jason Andrews wrote:
> I'm using an ASUS USB N13 on an ARM platform with the rtl8192cu driver.
> Linux kernel is 3.10 so I probably don't have the latest and greatest driver.
>
> When I booted I got an ARM alignment trap caused by the driver.
>
> I determined the cause was the 1st argument to spin_lock_irqsave() has an unaligned address.
>
> By trial-and-error I found that if I edit wifi.h and insert 2 dummy bytes into the rtl_priv struct just above priv (last variable) the locks work and the driver works fine.
>
> What is the recommended way to make sure the last variable in the rtl_priv struct (u8 priv[0]) is aligned on a 4 byte boundary so the driver works on ARM machines?

There are a lot of improvements for this driver in 3.11. The backports release 
has that code. In addition, I am currently working at improving the power 
management for 3.13.

The presence of unaligned variables that cause alignment traps on ARM does not 
surprise me as I test only on x86 and ppc architectures. I now own a Raspberry 
Pi and I will soon be testing with it as well.

What does surprise me is that the first argument in all the calls to 
spin_lock_irqsave() are contained within the rtl_locks struct and everything 
there should be aligned. Perhaps some ARM expert will know why aligning the last 
item in the rtl_priv struct fixes the problem.

As far as I know, the proper way to do a 4-byte alignment is as in the following 
patch:

Index: wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/wifi.h
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
@@ -2057,7 +2057,7 @@ struct rtl_priv {
  	  that it points to the data allocated
  	  beyond  this structure like:
  	  rtl_pci_priv or rtl_usb_priv */
-	u8 priv[0];
+	u8 __aligned(4) priv[0];
  };

  #define rtl_priv(hw)           (((struct rtl_priv *)(hw)->priv))

Larry