Skip to contentSkip to conversation

rsi: Adding RS9113 driver files

PATCH In review Fariya Fatima 4 participants 5 messages Mar 3, 2014 – Mar 5, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/10 rsi: Adding RS9113 driver files

From: Fariya Fatima

This driver supports the RS9113 chipset from Redpine Signals Inc. This driver
supports the IEEE 802.11 b/g/n standard. Both SDIO and USB interfaces are supported. This patch contains the include files.

---

 Kconfig               |    1 
 Makefile              |    1 
 rsi/Kconfig           |   31 ++++++
 rsi/Makefile          |   12 ++
 rsi/rsi_boot_params.h |  126 +++++++++++++++++++++++++++

rsi/rsi_debugfs.h | 52 +++++++++++diff -rupN linux-3.14-rc5/drivers/net/wireless/Kconfig linux-3.14-rc5_new/drivers/net/wireless/Kconfig

drivers/net/wireless/Kconfig +1 -0
@@ -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 -rupN linux-3.14-rc5/drivers/net/wireless/Makefile linux-3.14-rc5_new/drivers/net/wireless/Makefile
@@ -59,3 +59,4 @@ obj-$(CONFIG_BRCMFMAC)	+= brcm80211/
 obj-$(CONFIG_BRCMSMAC)	+= brcm80211/
 
 obj-$(CONFIG_CW1200)	+= cw1200/
+obj-$(CONFIG_RSI_91x)	+= rsi/
drivers/net/wireless/rsi/Kconfig +31 -0
diff -rupN linux-3.14-rc5/drivers/net/wireless/rsi/Kconfig linux-3.14-rc5_new/drivers/net/wireless/rsi/Kconfig
@@ -0,0 +1,31 @@
+config RSI_91x
+	tristate "Redpine Signals Inc 91x WLAN driver support"
+	depends on MAC80211
+	default m
+	---help---
+	  This option enabes support for RSI 1x1 devices.
+	  Select M (recommended), if you have a RSI 1x1 wireless module.
+
+config RSI_DEBUGFS
+	bool "Redpine Signals Inc debug support"
+	depends on RSI_91x
+	default y
+	---help---
+	 Say Y, if you would like to enable debug support. This option
+	 creates debugfs entries 
+
+config RSI_SDIO
+	tristate "Redpine Signals SDIO bus support"
+	depends on MMC && RSI_91x
+	default m
+	---help---
+	  This option enables the SDIO bus support in rsi drivers.
+	  Select M (recommended), if you have a RSI 1x1 wireless module.
+
+config RSI_USB
+	tristate "Redpine Signals USB bus support"
+	depends on USB && RSI_91x
+	default m
+	---help---
+	  This option enables the USB bus support in rsi drivers.
+	  Select M (recommended), if you have a RSI 1x1 wireless module.
drivers/net/wireless/rsi/Makefile +12 -0
diff -rupN linux-3.14-rc5/drivers/net/wireless/rsi/Makefile linux-3.14-rc5_new/drivers/net/wireless/rsi/Makefile
@@ -0,0 +1,12 @@
+rsi_91x-y			+= rsi_91x_main.o 
+rsi_91x-y			+= rsi_91x_core.o
+rsi_91x-y			+= rsi_91x_mac80211.o
+rsi_91x-y			+= rsi_91x_mgmt.o
+rsi_91x-y			+= rsi_91x_pkt.o
+rsi_91x-$(CONFIG_RSI_DEBUGFS)	+= rsi_91x_debugfs.o
+
+rsi_usb-y			+= rsi_91x_usb.o rsi_91x_usb_ops.o
+rsi_sdio-y			+= rsi_91x_sdio.o rsi_91x_sdio_ops.o
+obj-$(CONFIG_RSI_91x) 		+= rsi_91x.o
+obj-$(CONFIG_RSI_SDIO)		+= rsi_sdio.o
+obj-$(CONFIG_RSI_USB)		+= rsi_usb.o
drivers/net/wireless/rsi/rsi_boot_params.h +126 -0

126 lines not on this page · raw message

drivers/net/wireless/rsi/rsi_debugfs.h +52 -0

52 lines not on this page · raw message

drivers/net/wireless/rsi/rsi_main.h +232 -0

232 lines not on this page · raw message

drivers/net/wireless/rsi/rsi_sdio.h +129 -0

129 lines not on this page · raw message

drivers/net/wireless/rsi/rsi_usb.h +70 -0

70 lines not on this page · raw message

 rsi/rsi_main.h        |  232 ++++++++++++++++++++++++++++++++++++++++++++++++++
 rsi/rsi_sdio.h        |  129 +++++++++++++++++++++++++++
 rsi/rsi_usb.h         |   70 +++++++++++++++
 9 files changed, 654 insertions(+)
#1 lore raw

Does anyone have any last objections or criticisms to this patch series?

Quoted 8 lines · Fariya Fatima

From: Fariya Fatima

This driver supports the RS9113 chipset from Redpine Signals Inc. This driver
supports the IEEE 802.11 b/g/n standard. Both SDIO and USB interfaces are supported. This patch contains the include files.

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.
#2 lore raw
Quoted 5 lines · John W. Linville

Does anyone have any last objections or criticisms to this patch series?

Quoted 2 lines · Fariya Fatima
Kconfig               |    1
Makefile              |    1

A minor critique would be that these Kconfig/Makefile changes should be added in the last patch, else any in-between will result in a state where the driver is selectable, but fails to build because of missing source files.

Also the subject suggests the patch set was based on linus' tree, not wireless-next (or whatever the correct tree is), but this does not necessarily mean it does not apply; I did not test this.

Regards
Jonas

#3 lore raw
Quoted 11 lines · Jonas Gorski
Quoted 5 lines · John W. Linville

Does anyone have any last objections or criticisms to this patch series?

Quoted 2 lines · Fariya Fatima
Kconfig               |    1
Makefile              |    1

A minor critique would be that these Kconfig/Makefile changes should be added in the last patch, else any in-between will result in a state where the driver is selectable, but fails to build because of missing source files.

FWIW, I tend to collapse a series like this (i.e. one that adds a new driver using a series of small, non-buildable patches) into a single patch when merging.

Quoted 3 lines

Also the subject suggests the patch set was based on linus' tree, not wireless-next (or whatever the correct tree is), but this does not necessarily mean it does not apply; I did not test this.

This is usually not a (big) merge problem for a new driver.

John

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.
maintainer
#4 lore raw
In reply to Fariya Fatima
Quoted 1 line · Fariya Fatima

+config RSI_91x

Convention would be to use RSI_91X.

drivers/net/wireless/rsi/Kconfig line 4

Quoted 3 lines
drivers/net/wireless/rsi/Kconfig +3 -0
+	tristate "Redpine Signals Inc 91x WLAN driver support"
+	depends on MAC80211
+	default m

Please don't add a default, the driver is probably useless for almost everyone running 'make oldconfig' and similar.

drivers/net/wireless/rsi/Kconfig line 12

Quoted 4 lines
drivers/net/wireless/rsi/Kconfig +4 -0
+config RSI_DEBUGFS
+	bool "Redpine Signals Inc debug support"
+	depends on RSI_91x
+	default y

This on the other hand is fine since it depends on RSI_91x

drivers/net/wireless/rsi/rsi_debugfs.h line 28

Quoted 12 lines
drivers/net/wireless/rsi/rsi_debugfs.h +12 -0
+#ifndef __RSI_DEBUGFS_H__
+#define __RSI_DEBUGFS_H__
+
+#include "rsi_main.h"
+#include <linux/debugfs.h>
+
+#define FOPS(fopen) { \
+	.owner = THIS_MODULE, \
+	.open = (fopen), \
+	.read = seq_read, \
+	.llseek = seq_lseek, \
+}

I'm not sure these should be in a header file? Seems only the debugfs implemnetation would need them.

drivers/net/wireless/rsi/rsi_main.h line 97

Quoted 11 lines
drivers/net/wireless/rsi/rsi_main.h +11 -0
+#define TID_TO_WME_AC(_tid) (      \
+	((_tid) == 0 || (_tid) == 3) ? BE_Q : \
+	((_tid) < 3) ? BK_Q : \
+	((_tid) < 6) ? VI_Q : \
+	VO_Q)
+
+#define WME_AC(_q) (    \
+	((_q) == BK_Q) ? IEEE80211_AC_BK : \
+	((_q) == BE_Q) ? IEEE80211_AC_BE : \
+	((_q) == VI_Q) ? IEEE80211_AC_VI : \
+	IEEE80211_AC_VO)

What are those "BE_Q" constants? Those seem a tad short to me.

Quoted 1 line

+enum EDCA_QUEUE {

Ah, here they are. enums should typically not be uppercase though.

drivers/net/wireless/rsi/rsi_main.h line 156

Quoted 12 lines
drivers/net/wireless/rsi/rsi_main.h +12 -0
+struct rsi_event {
+	atomic_t event_condition;
+	wait_queue_head_t event_queue;
+};
+
+struct rsi_thread {
+	void (*thread_function)(void *);
+	struct completion completion;
+	struct task_struct *task;
+	struct rsi_event event;
+	atomic_t thread_done;
+};

These seem odd ... maybe they should at least come with comments about how generic kernel functionality can't be used and why it needs another abstraction layer?

drivers/net/wireless/rsi/rsi_main.h line 199

Quoted 3 lines
drivers/net/wireless/rsi/rsi_main.h +3 -0
+	/* Generic */
+	u8 channel;
+	u8 *rx_data_pkt;

That seems rather odd, how can you have one rx_data_pkt in the global struct?

drivers/net/wireless/rsi/rsi_main.h line 224

Quoted 4 lines
drivers/net/wireless/rsi/rsi_main.h +4 -0
+#ifdef CONFIG_RSI_DEBUGFS
+	struct rsi_debugfs *dfsentry;
+	u8 num_debugfs_entries;
+#endif

What do you need num_entries for?

johannes