rsi: Add RS9113 driver

PATCH In review Jahnavi 2 participants 3 messages Jan 30, 2014 – Jan 31, 2014

Subsystems (2)

Networking drivers (wireless) Redpine wireless driver

lore

Share & tools

lore

Jahnavi

author

2014-01-30 15:59 UTC #0 lore raw

PATCH 1/9 rsi: Add RS9113 driver

From: Jahnavi Meher <[email protected]>

This driver supports the RS9113 chipset from Redpine Signals Inc. This chipset supports the 802.11a/b/g/n standards, but this driver currently supports only b/g/n. Both SDIO and USB interfaces are supported. Can be upgraded to 91x quite easily. More information can be found at:
http://www.redpinesignals.com/Technologies_&_Chipsets/Chipsets/RS9113.php

Signed-off-by Jahnavi Meher <[email protected]>

---

Kconfig                      |    1
Makefile                     |    1
rsi/Kconfig                  |   33 +++++
rsi/Makefile                 |    1
rsi/include/rsi_device_ops.h |  121 +++++++++++++++++++++
rsi/include/rsi_hw_intf.h    |  173 ++++++++++++++++++++++++++++++
rsi/include/rsi_mac80211.h   |  154 ++++++++++++++++++++++++++
rsi/include/rsi_main.h       |  110 +++++++++++++++++++
rsi/include/rsi_mgmt.h       |  246 +++++++++++++++++++++++++++++++++++++++++++
9 files changed, 840 insertions(+)

drivers/net/wireless/Kconfig +1 -0

diff -uprN a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
@@ -281,5 +281,6 @@ source "drivers/net/wireless/ti/Kconfig"
 source "drivers/net/wireless/zd1211rw/Kconfig"
 source "drivers/net/wireless/mwifiex/Kconfig"
 source "drivers/net/wireless/cw1200/Kconfig"
+source "drivers/net/wireless/rsi/Kconfig"
 
 endif # WLAN

drivers/net/wireless/Makefile +1 -0

diff -uprN a/drivers/net/wireless/Makefile b/drivers/net/wireless/Makefile
@@ -59,3 +59,4 @@ obj-$(CONFIG_BRCMFMAC)	+= brcm80211/
 obj-$(CONFIG_BRCMSMAC)	+= brcm80211/
 
 obj-$(CONFIG_CW1200)	+= cw1200/
+obj-$(CONFIG_RSI)	+= rsi/

drivers/net/wireless/rsi/include/rsi_device_ops.h +121 -0

121 lines not on this page · raw message

drivers/net/wireless/rsi/include/rsi_hw_intf.h +173 -0

173 lines not on this page · raw message

drivers/net/wireless/rsi/include/rsi_mac80211.h +154 -0

154 lines not on this page · raw message

drivers/net/wireless/rsi/include/rsi_main.h +110 -0

110 lines not on this page · raw message

drivers/net/wireless/rsi/include/rsi_mgmt.h +246 -0

246 lines not on this page · raw message

drivers/net/wireless/rsi/Kconfig +33 -0

diff -uprN a/drivers/net/wireless/rsi/Kconfig b/drivers/net/wireless/rsi/Kconfig
@@ -0,0 +1,33 @@
+menuconfig RSI
+	bool "Redpine Signals Inc Wireless Chipset"
+	default y
+	---help---
+	  This will enable support for RSI wireless chips
+
+config RSI_91x
+	tristate "Redpine Signals Inc 1x1 device support"
+	depends on RSI && MAC80211
+	default m
+	---help---
+	  This option enabes support for RSI 1x1 devices.
+	  Select M (recommended), if you have a RSI 1x1 wireless module.
+
+choice
+	prompt "Redpine Signals Inc hardware interface support"
+	depends on RSI_91x
+	default RSI_USB
+
+config RSI_USB
+	bool "Redpine Signals USB bus support"
+	depends on USB && RSI_91x
+	---help---
+	  This option enables the USB bus support in rsi drivers.
+	  Select Y, if you have a RSI USB 1x1 wireless module.
+
+config RSI_SDIO
+	bool "Redpine Signals SDIO bus support"
+	depends on MMC && RSI_91x
+	---help---
+	  This option enables the SDIO bus support in rsi drivers.
+	  Select Y, if you have a RSI SDIO 1x1 wireless module.
+endchoice

drivers/net/wireless/rsi/Makefile +1 -0

diff -uprN a/drivers/net/wireless/rsi/Makefile b/drivers/net/wireless/rsi/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_RSI_91x) := 91x/ common/

Johannes Berg

maintainer

2014-01-30 16:18 UTC #1 lore raw

Quoted 7 lines · Jahnavi

From: Jahnavi Meher <[email protected]>
This driver supports the RS9113 chipset from Redpine Signals Inc. This chipset supports the 802.11a/b/g/n standards, but this driver currently supports only b/g/n. Both SDIO and USB interfaces are supported. Can be upgraded to 91x quite easily. More information can be found at:
http://www.redpinesignals.com/Technologies_&_Chipsets/Chipsets/RS9113.php

You should probably propose it for staging... let me take a few minutes and make a quick review pass.

Quoted 6 lines

+/**
+ * @file rsi_device_ops.h
+ * @author
+ * @version 1.0
+ *
+ * @section LICENSE

Those lines are all useless

Quoted 1 line

+ * Copyright (c) 2013 Redpine Signals Inc.

It's 2014 now, surely you want to claim copyright this year?

Quoted 1 line

+ * @section DESCRIPTION

That's unnecessary.

Quoted 1 line

+ * This file contians the data structures and variables/ macros commonly

contains

Quoted 1 line

+#define RSI_HEADER_SIZE 18

That should probably be some struct, and the define doing sizeof()?

Quoted 4 lines

drivers/net/wireless/rsi/include/rsi_device_ops.h +4 -0

+static inline unsigned int rsi_get_queueno(unsigned char *addr,
+					   unsigned short offset)
+{
+	return (le16_to_cpu(*(unsigned short *)&addr[offset]) & 0x7000) >> 12;

drivers/net/wireless/rsi/include/rsi_device_ops.h line 86

Quoted 10 lines

drivers/net/wireless/rsi/include/rsi_device_ops.h +6 -0

 
+static inline unsigned int rsi_get_length(unsigned char *addr,
+					  unsigned short offset)
 
+static inline unsigned char rsi_get_extended_desc(unsigned char *addr,
+						  unsigned short offset)
 
+static inline unsigned char rsi_get_rssi(unsigned char *addr)
 
+static inline unsigned char rsi_get_channel(unsigned char *addr)

These seem like they should then just be replaced by

	some_struct_ptr->rssi

and similar?

drivers/net/wireless/rsi/include/rsi_device_ops.h line 96

Quoted 4 lines

drivers/net/wireless/rsi/include/rsi_device_ops.h +4 -0

+int rsi_send_ampdu_indication_frame(struct rsi_common *common,
+				    unsigned short tid,
+				    unsigned short ssn, unsigned char buf_size,
+				    unsigned char event);

what are "A-MPDU indication frame[s]"?

Quoted 1 line

+#ifdef USE_USB_INTF

Shouldn't that be just CONFIG_RSI_SOMETHING?

Quoted 1 line

+typedef void (*SD_INTERRUPT)(void *pcontext);

no typedefs please

Quoted 1 line

+#define SDIO_BLOCK_SIZE 256

Seems like there should be a define for that already?

drivers/net/wireless/rsi/include/rsi_mac80211.h line 150

Quoted 9 lines

+++ b/drivers/net/wireless/rsi/include/rsi_mac80211.h 2014-01-30 16:01:00.194777922 +0530
drivers/net/wireless/rsi/include/rsi_mac80211.h +4 -0
+static const struct ieee80211_channel rsi_2ghz_channels[] = {
 
+static const struct ieee80211_channel rsi_5ghz_channels[] = {
 
+static struct ieee80211_rate rsi_rates[] = {
 
+static const unsigned short rsi_mcsrates[] = {

None of this belongs into a header file.

Quoted 2 lines

+ * This file contians the os dependent( Linux) specific function prototypes
+ * and macros

That kind of thing is generally frowned upon.

Quoted 1 line

+void rsi_print(int zone, unsigned char *vdata, int len);

Unless it's more than just a simple printk wrapper?

drivers/net/wireless/rsi/include/rsi_mgmt.h line 136

Quoted 8 lines

drivers/net/wireless/rsi/include/rsi_mgmt.h +8 -0

+struct module_ps_config {
+	unsigned int not_in_use:1;
+	unsigned int clock_gate:1;
+	unsigned int logical_prgm:1;
+	unsigned int logical_poweroff:1;
+	unsigned int power_off:1;
+	unsigned int resvd:27;
+};

Uh, no, you can't use bitfields in hardware/firmware communication structs. They also need to be __packed, in most cases.

Quoted 1 line

+struct rsi_mac_frame {

All those unions seem a bit ... overblown? Probably better to define separate structs for different usages and cast appropriately.

johannes

Jahnavi Meher

author

2014-01-31 10:52 UTC #2 lore raw

Hi Johannes,
Thank you for the review comments, will make the changes as you have suggested. A-MPDU indication frames are sent to the firmware to start/stop tx/rx ampdu aggregation.

Regards,
Jahnavi

Quoted 96 lines · Johannes Berg

Quoted 7 lines · Jahnavi
From: Jahnavi Meher <[email protected]>
This driver supports the RS9113 chipset from Redpine Signals Inc. This chipset supports the 802.11a/b/g/n standards, but this driver currently supports only b/g/n. Both SDIO and USB interfaces are supported. Can be upgraded to 91x quite easily. More information can be found at:
http://www.redpinesignals.com/Technologies_&_Chipsets/Chipsets/RS9113.php
You should probably propose it for staging... let me take a few minutes and make a quick review pass.
Quoted 6 lines
+/**
+ * @file rsi_device_ops.h
+ * @author
+ * @version 1.0
+ *
+ * @section LICENSE
Those lines are all useless
Quoted 1 line
+ * Copyright (c) 2013 Redpine Signals Inc.
It's 2014 now, surely you want to claim copyright this year?
Quoted 1 line
+ * @section DESCRIPTION
That's unnecessary.
Quoted 1 line
+ * This file contians the data structures and variables/ macros commonly
contains
Quoted 1 line
+#define RSI_HEADER_SIZE 18
That should probably be some struct, and the define doing sizeof()?
drivers/net/wireless/rsi/include/rsi_device_ops.h line 86
Quoted 10 lines
drivers/net/wireless/rsi/include/rsi_device_ops.h +10 -0
+static inline unsigned int rsi_get_queueno(unsigned char *addr,
+					   unsigned short offset)
+{
+	return (le16_to_cpu(*(unsigned short *)&addr[offset]) & 0x7000) >> 12;
+static inline unsigned int rsi_get_length(unsigned char *addr,
+					  unsigned short offset)
+static inline unsigned char rsi_get_extended_desc(unsigned char *addr,
+						  unsigned short offset)
+static inline unsigned char rsi_get_rssi(unsigned char *addr)
+static inline unsigned char rsi_get_channel(unsigned char *addr)
These seem like they should then just be replaced by
	some_struct_ptr->rssi
and similar?
drivers/net/wireless/rsi/include/rsi_device_ops.h line 96
Quoted 4 lines
drivers/net/wireless/rsi/include/rsi_device_ops.h +4 -0
+int rsi_send_ampdu_indication_frame(struct rsi_common *common,
+				    unsigned short tid,
+				    unsigned short ssn, unsigned char buf_size,
+				    unsigned char event);
what are "A-MPDU indication frame[s]"?
Quoted 1 line
+#ifdef USE_USB_INTF
Shouldn't that be just CONFIG_RSI_SOMETHING?
Quoted 1 line
+typedef void (*SD_INTERRUPT)(void *pcontext);
no typedefs please
Quoted 1 line
+#define SDIO_BLOCK_SIZE 256
Seems like there should be a define for that already?
drivers/net/wireless/rsi/include/rsi_mac80211.h line 150
Quoted 5 lines
+++ b/drivers/net/wireless/rsi/include/rsi_mac80211.h 2014-01-30 16:01:00.194777922 +0530
drivers/net/wireless/rsi/include/rsi_mac80211.h +4 -0
+static const struct ieee80211_channel rsi_2ghz_channels[] = {
+static const struct ieee80211_channel rsi_5ghz_channels[] = {
+static struct ieee80211_rate rsi_rates[] = {
+static const unsigned short rsi_mcsrates[] = {
None of this belongs into a header file.
Quoted 2 lines
+ * This file contians the os dependent( Linux) specific function prototypes
+ * and macros
That kind of thing is generally frowned upon.
Quoted 1 line
+void rsi_print(int zone, unsigned char *vdata, int len);
Unless it's more than just a simple printk wrapper?
drivers/net/wireless/rsi/include/rsi_mgmt.h line 136
Quoted 8 lines
drivers/net/wireless/rsi/include/rsi_mgmt.h +8 -0
+struct module_ps_config {
+	unsigned int not_in_use:1;
+	unsigned int clock_gate:1;
+	unsigned int logical_prgm:1;
+	unsigned int logical_poweroff:1;
+	unsigned int power_off:1;
+	unsigned int resvd:27;
+};
Uh, no, you can't use bitfields in hardware/firmware communication structs. They also need to be __packed, in most cases.
Quoted 1 line
+struct rsi_mac_frame {
All those unions seem a bit ... overblown? Probably better to define separate structs for different usages and cast appropriately.
johannes