diff mbox

[2/2] rtl8180: don't use weird trick to access "far" registers

Message ID 1396026868-5622-1-git-send-email-andrea.merello@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Andrea Merello March 28, 2014, 5:14 p.m. UTC
In rtl8180/rtl8185/rtl8187se the register space is represented
using packed structure type. Register are thus accessed using a
pointer of this type.
All registers are packed toghether, and only small gaps are present.

However Rtl8187se has also some "sparse" registers, very far from
the "main register block".

It could be possible to access them by simply declare huge reserved
blocks inside the register struct (and this causes NO memory waste).
However, for various reasons, access to those "far" registers is
done with special dedicated macros, without declaring them in the
register struct.

This is done in an intricate manner, that makes code less readable
and caused static analisys tool to produce warnings.

This patch keeps the "macro" mechanism, but it changes its
implementation in a simplier and more straightforward way.

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/net/wireless/rtl818x/rtl818x.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Dave April 2, 2014, 7:24 p.m. UTC | #1
On 28/03/2014 17:14, Andrea Merello wrote:
> @@ -340,9 +334,9 @@ struct rtl818x_csr {
>    * I don't like to introduce a ton of "reserved"..
>    * They are for RTL8187SE
>    */
> -#define REG_ADDR1(addr)	((u8 __iomem *)(&priv->map->offset1[(addr)]))
> -#define REG_ADDR2(addr)	((__le16 __iomem *)(&priv->map->offset2[((addr) >> 1)]))
> -#define REG_ADDR4(addr)	((__le32 __iomem *)(&priv->map->offset4[((addr) >> 2)]))
> +#define REG_ADDR1(addr)	((u8 __iomem *)priv->map + addr)
> +#define REG_ADDR2(addr)	((__le16 __iomem *)priv->map + (addr >> 1))
> +#define REG_ADDR4(addr)	((__le32 __iomem *)priv->map + (addr >> 2))
>   
>   #define FEMR_SE		REG_ADDR2(0x1D4)
>   #define ARFR		REG_ADDR2(0x1E0)
I suggest parenthesizing the use of addr in your macros (as the original 
code does), to avoid any issues with operator precedence wrt >>.

If the removal was intentional and you've verified there's no issues, 
you should probably mention it in the commit message.


Dave.
--
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
diff mbox

Patch

diff --git a/drivers/net/wireless/rtl818x/rtl818x.h b/drivers/net/wireless/rtl818x/rtl818x.h
index 99dbc12..45ea4e1 100644
--- a/drivers/net/wireless/rtl818x/rtl818x.h
+++ b/drivers/net/wireless/rtl818x/rtl818x.h
@@ -17,13 +17,7 @@ 
 
 struct rtl818x_csr {
 
-	union {
-		u8	MAC[6];
-		u8	offset1[6];     /* upper page indexing helpers */
-		__le16	offset2[1];
-		__le32	offset4[1];
-	} __packed;
-
+	u8	MAC[6];
 	u8	reserved_0[2];
 
 	union {
@@ -340,9 +334,9 @@  struct rtl818x_csr {
  * I don't like to introduce a ton of "reserved"..
  * They are for RTL8187SE
  */
-#define REG_ADDR1(addr)	((u8 __iomem *)(&priv->map->offset1[(addr)]))
-#define REG_ADDR2(addr)	((__le16 __iomem *)(&priv->map->offset2[((addr) >> 1)]))
-#define REG_ADDR4(addr)	((__le32 __iomem *)(&priv->map->offset4[((addr) >> 2)]))
+#define REG_ADDR1(addr)	((u8 __iomem *)priv->map + addr)
+#define REG_ADDR2(addr)	((__le16 __iomem *)priv->map + (addr >> 1))
+#define REG_ADDR4(addr)	((__le32 __iomem *)priv->map + (addr >> 2))
 
 #define FEMR_SE		REG_ADDR2(0x1D4)
 #define ARFR		REG_ADDR2(0x1E0)