[PATCH 1/5] rtl8712: code clean up

[PATCH 1/5] rtl8712: code clean up

[Przemo Firszt]

Clean some trivial formating problems in rtl8712 from staging tree. This patch
also changes the way preprocessor macros are defined to keep checkpatch.pl
quiet.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/staging/rtl8712/ieee80211.h        |   2 +-
 drivers/staging/rtl8712/rtl871x_cmd.h      |   2 +-
 drivers/staging/rtl8712/rtl871x_security.h |   6 +-
 drivers/staging/rtl8712/sta_info.h         |   2 +-
 drivers/staging/rtl8712/wifi.h             | 167 +++++++++++++----------------
 5 files changed, 79 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/rtl8712/ieee80211.h b/drivers/staging/rtl8712/ieee80211.h
index 21515c3..da4000e 100644
--- a/drivers/staging/rtl8712/ieee80211.h
+++ b/drivers/staging/rtl8712/ieee80211.h
@@ -777,7 +777,7 @@ extern inline int ieee80211_get_hdrlen(u16 fc)
 struct registry_priv;
 
 u8 *r8712_set_ie(u8 *pbuf, sint index, uint len, u8 *source, uint *frlen);
-u8 *r8712_get_ie(u8*pbuf, sint index, sint *len, sint limit);
+u8 *r8712_get_ie(u8 *pbuf, sint index, sint *len, sint limit);
 unsigned char *r8712_get_wpa_ie(unsigned char *pie, int *rsn_ie_len, int limit);
 unsigned char *r8712_get_wpa2_ie(unsigned char *pie, int *rsn_ie_len,
 				 int limit);
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.h b/drivers/staging/rtl8712/rtl871x_cmd.h
index 9d93189..0ce79b1 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.h
+++ b/drivers/staging/rtl8712/rtl871x_cmd.h
@@ -749,7 +749,7 @@ u8 r8712_setopmode_cmd(struct _adapter *padapter,
 u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset);
 u8 r8712_set_chplan_cmd(struct _adapter  *padapter, int chplan);
 u8 r8712_setbasicrate_cmd(struct _adapter *padapter, u8 *rateset);
-u8 r8712_getrfreg_cmd(struct _adapter *padapter, u8 offset, u8 * pval);
+u8 r8712_getrfreg_cmd(struct _adapter *padapter, u8 offset, u8 *pval);
 u8 r8712_setrfintfs_cmd(struct _adapter *padapter, u8 mode);
 u8 r8712_setrfreg_cmd(struct _adapter  *padapter, u8 offset, u32 val);
 u8 r8712_setrttbl_cmd(struct _adapter  *padapter,
diff --git a/drivers/staging/rtl8712/rtl871x_security.h b/drivers/staging/rtl8712/rtl871x_security.h
index a13395f..c732aea 100644
--- a/drivers/staging/rtl8712/rtl871x_security.h
+++ b/drivers/staging/rtl8712/rtl871x_security.h
@@ -207,9 +207,9 @@ void seccalctkipmic(
 	u8  *Miccode,
 	u8   priority);
 
-void r8712_secmicsetkey(struct mic_data *pmicdata, u8 * key);
-void r8712_secmicappend(struct mic_data *pmicdata, u8 * src, u32 nBytes);
-void r8712_secgetmic(struct mic_data *pmicdata, u8 * dst);
+void r8712_secmicsetkey(struct mic_data *pmicdata, u8 *key);
+void r8712_secmicappend(struct mic_data *pmicdata, u8 *src, u32 nBytes);
+void r8712_secgetmic(struct mic_data *pmicdata, u8 *dst);
 u32 r8712_aes_encrypt(struct _adapter *padapter, u8 *pxmitframe);
 u32 r8712_tkip_encrypt(struct _adapter *padapter, u8 *pxmitframe);
 void r8712_wep_encrypt(struct _adapter *padapter, u8  *pxmitframe);
diff --git a/drivers/staging/rtl8712/sta_info.h b/drivers/staging/rtl8712/sta_info.h
index f8016e9..c4e0ef2 100644
--- a/drivers/staging/rtl8712/sta_info.h
+++ b/drivers/staging/rtl8712/sta_info.h
@@ -140,7 +140,7 @@ void r8712_free_all_stainfo(struct _adapter *padapter);
 struct sta_info *r8712_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr);
 void r8712_init_bcmc_stainfo(struct _adapter *padapter);
 struct sta_info *r8712_get_bcmc_stainfo(struct _adapter *padapter);
-u8 r8712_access_ctrl(struct wlan_acl_pool *pacl_list, u8 * mac_addr);
+u8 r8712_access_ctrl(struct wlan_acl_pool *pacl_list, u8 *mac_addr);
 
 #endif /* _STA_INFO_H_ */
 
diff --git a/drivers/staging/rtl8712/wifi.h b/drivers/staging/rtl8712/wifi.h
index 793443e..c08ed2a 100644
--- a/drivers/staging/rtl8712/wifi.h
+++ b/drivers/staging/rtl8712/wifi.h
@@ -159,99 +159,85 @@ enum WIFI_REG_DOMAIN {
 #define _PRIVACY_	BIT(14)
 #define _ORDER_		BIT(15)
 
-#define SetToDs(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_TO_DS_); \
-	} while (0)
+#define SetToDs(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_TO_DS_); \
+})
 
 #define GetToDs(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_TO_DS_)) != 0)
 
-#define ClearToDs(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_TO_DS_)); \
-	} while (0)
+#define ClearToDs(pbuf)	({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_TO_DS_)); \
+})
 
-#define SetFrDs(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_FROM_DS_); \
-	} while (0)
+#define SetFrDs(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_FROM_DS_); \
+})
 
 #define GetFrDs(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_FROM_DS_)) != 0)
 
-#define ClearFrDs(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_FROM_DS_)); \
-	} while (0)
+#define ClearFrDs(pbuf)	({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_FROM_DS_)); \
+})
 
 #define get_tofr_ds(pframe)	((GetToDs(pframe) << 1) | GetFrDs(pframe))
 
 
-#define SetMFrag(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_MORE_FRAG_); \
-	} while (0)
+#define SetMFrag(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_MORE_FRAG_); \
+})
 
 #define GetMFrag(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_MORE_FRAG_)) != 0)
 
-#define ClearMFrag(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_MORE_FRAG_)); \
-	} while (0)
+#define ClearMFrag(pbuf) ({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_MORE_FRAG_)); \
+})
 
-#define SetRetry(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_RETRY_); \
-	} while (0)
+#define SetRetry(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_RETRY_); \
+})
 
 #define GetRetry(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_RETRY_)) != 0)
 
-#define ClearRetry(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_RETRY_)); \
-	} while (0)
+#define ClearRetry(pbuf) ({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_RETRY_)); \
+})
 
-#define SetPwrMgt(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_PWRMGT_); \
-	} while (0)
+#define SetPwrMgt(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_PWRMGT_); \
+})
 
 #define GetPwrMgt(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_PWRMGT_)) != 0)
 
-#define ClearPwrMgt(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_PWRMGT_)); \
-	} while (0)
+#define ClearPwrMgt(pbuf) ({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_PWRMGT_)); \
+})
 
-#define SetMData(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_MORE_DATA_); \
-	} while (0)
+#define SetMData(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_MORE_DATA_); \
+})
 
 #define GetMData(pbuf)	(((*(unsigned short *)(pbuf)) & \
 			le16_to_cpu(_MORE_DATA_)) != 0)
 
-#define ClearMData(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_MORE_DATA_)); \
-	} while (0)
+#define ClearMData(pbuf) ({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_MORE_DATA_)); \
+})
 
-#define SetPrivacy(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_PRIVACY_); \
-	} while (0)
+#define SetPrivacy(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_PRIVACY_); \
+})
 
 #define GetPrivacy(pbuf)	(((*(unsigned short *)(pbuf)) & \
 				le16_to_cpu(_PRIVACY_)) != 0)
 
-#define ClearPrivacy(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) &= (~cpu_to_le16(_PRIVACY_)); \
-	} while (0)
+#define ClearPrivacy(pbuf) ({ \
+	*(unsigned short *)(pbuf) &= (~cpu_to_le16(_PRIVACY_)); \
+})
 
 
 #define GetOrder(pbuf)	(((*(unsigned short *)(pbuf)) & \
@@ -287,48 +273,42 @@ enum WIFI_REG_DOMAIN {
 #define GetTupleCache(pbuf)	(cpu_to_le16(*(unsigned short *)\
 				((addr_t)(pbuf) + 22)))
 
-#define SetFragNum(pbuf, num) \
-	do {    \
-		*(unsigned short *)((addr_t)(pbuf) + 22) = \
-			((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
-			le16_to_cpu(~(0x000f))) | \
-			cpu_to_le16(0x0f & (num));     \
-	} while (0)
-
-#define SetSeqNum(pbuf, num) \
-	do {    \
-		*(unsigned short *)((addr_t)(pbuf) + 22) = \
-			((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
-			le16_to_cpu((unsigned short)0x000f)) | \
-			le16_to_cpu((unsigned short)(0xfff0 & (num << 4))); \
-	} while (0)
-
-#define SetDuration(pbuf, dur) \
-	do {    \
-		*(unsigned short *)((addr_t)(pbuf) + 2) |= \
-			cpu_to_le16(0xffff & (dur)); \
-	} while (0)
-
-#define SetPriority(pbuf, tid)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(tid & 0xf); \
-	} while (0)
+#define SetFragNum(pbuf, num) ({ \
+	*(unsigned short *)((addr_t)(pbuf) + 22) = \
+	((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
+	le16_to_cpu(~(0x000f))) | \
+	cpu_to_le16(0x0f & (num));     \
+})
+
+#define SetSeqNum(pbuf, num) ({ \
+	*(unsigned short *)((addr_t)(pbuf) + 22) = \
+	((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
+	le16_to_cpu((unsigned short)0x000f)) | \
+	le16_to_cpu((unsigned short)(0xfff0 & (num << 4))); \
+})
+
+#define SetDuration(pbuf, dur) ({ \
+	*(unsigned short *)((addr_t)(pbuf) + 2) |= \
+	cpu_to_le16(0xffff & (dur)); \
+})
+
+#define SetPriority(pbuf, tid) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(tid & 0xf); \
+})
 
 #define GetPriority(pbuf)	((le16_to_cpu(*(unsigned short *)(pbuf))) & 0xf)
 
-#define SetAckpolicy(pbuf, ack)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16((ack & 3) << 5); \
-	} while (0)
+#define SetAckpolicy(pbuf, ack) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16((ack & 3) << 5); \
+})
 
 #define GetAckpolicy(pbuf) (((le16_to_cpu(*(unsigned short *)pbuf)) >> 5) & 0x3)
 
 #define GetAMsdu(pbuf) (((le16_to_cpu(*(unsigned short *)pbuf)) >> 7) & 0x1)
 
-#define SetAMsdu(pbuf, amsdu)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16((amsdu & 1) << 7); \
-	} while (0)
+#define SetAMsdu(pbuf, amsdu) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16((amsdu & 1) << 7); \
+})
 
 #define GetAid(pbuf)	(cpu_to_le16(*(unsigned short *)((addr_t)(pbuf) + 2)) \
 			& 0x3fff)
@@ -526,10 +506,9 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
 #define IEEE80211_DELBA_PARAM_TID_MASK 0xF000
 #define IEEE80211_DELBA_PARAM_INITIATOR_MASK 0x0800
 
-#define SetOrderBit(pbuf)	\
-	do	{	\
-		*(unsigned short *)(pbuf) |= cpu_to_le16(_ORDER_); \
-	} while (0)
+#define SetOrderBit(pbuf) ({ \
+	*(unsigned short *)(pbuf) |= cpu_to_le16(_ORDER_); \
+})
 
 #define GetOrderBit(pbuf)	(((*(unsigned short *)(pbuf)) & \
 				le16_to_cpu(_ORDER_)) != 0)
-- 
1.8.0.1

[PATCH 2/5] rtl8712: remove unused macros from rtl8712/wifi.h

[Przemo Firszt]

Those definitions are not used anywhere in the kernel. If you know any
reason why they should stay in the code please speak up!

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/staging/rtl8712/wifi.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/rtl8712/wifi.h b/drivers/staging/rtl8712/wifi.h
index c08ed2a..73d7cd2 100644
--- a/drivers/staging/rtl8712/wifi.h
+++ b/drivers/staging/rtl8712/wifi.h
@@ -437,11 +437,7 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
 #define _SSID_IE_		0
 #define _SUPPORTEDRATES_IE_	1
 #define _DSSET_IE_		3
-#define _TIM_IE_		5
 #define _IBSS_PARA_IE_		6
-#define _CHLGETXT_IE_		16
-#define _RSN_IE_2_		48`
-#define _SSN_IE_1_		221
 #define _ERPINFO_IE_		42
 #define _EXT_SUPPORTEDRATES_IE_	50
 
-- 
1.8.0.1

[PATCH 3/5] rtl8712: replace printk with better solutions

[Przemo Firszt]

Replace printk with netdev_printk helpers, dev_printk helpers or
pr_err/warn/info if there is no device info available.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/staging/rtl8712/hal_init.c            | 17 ++++++-----
 drivers/staging/rtl8712/os_intfs.c            |  3 +-
 drivers/staging/rtl8712/rtl8712_recv.c        | 14 ++++-----
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 44 +++++++++++----------------
 drivers/staging/rtl8712/rtl871x_mlme.c        |  2 +-
 drivers/staging/rtl8712/usb_intf.c            | 23 ++++++--------
 drivers/staging/rtl8712/usb_ops_linux.c       | 12 +++-----
 drivers/staging/rtl8712/xmit_linux.c          |  3 +-
 8 files changed, 50 insertions(+), 68 deletions(-)

diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
index cb9d4cf..9747d45 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -49,7 +49,7 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
 	if (!firmware) {
 		struct usb_device *udev = padapter->dvobjpriv.pusbdev;
 		struct usb_interface *pusb_intf = padapter->pusb_intf;
-		printk(KERN_ERR "r8712u: Firmware request failed\n");
+		dev_err(&udev->dev, "r8712u: Firmware request failed\n");
 		padapter->fw_found = false;
 		usb_put_dev(udev);
 		usb_set_intfdata(pusb_intf, NULL);
@@ -69,12 +69,11 @@ int rtl871x_load_fw(struct _adapter *padapter)
 	int rc;
 
 	init_completion(&padapter->rtl8712_fw_ready);
-	printk(KERN_INFO "r8712u: Loading firmware from \"%s\"\n",
-	       firmware_file);
+	dev_info(dev, "r8712u: Loading firmware from \"%s\"\n", firmware_file);
 	rc = request_firmware_nowait(THIS_MODULE, 1, firmware_file, dev,
 				     GFP_KERNEL, padapter, rtl871x_load_fw_cb);
 	if (rc)
-		printk(KERN_ERR "r8712u: Firmware request error %d\n", rc);
+		dev_err(dev, "r8712u: Firmware request error %d\n", rc);
 	return rc;
 }
 MODULE_FIRMWARE("rtlwifi/rtl8712u.bin");
@@ -84,8 +83,8 @@ static u32 rtl871x_open_fw(struct _adapter *padapter, const u8 **ppmappedfw)
 	const struct firmware **praw = &padapter->fw;
 
 	if (padapter->fw->size > 200000) {
-		printk(KERN_ERR "r8172u: Badfw->size of %d\n",
-		       (int)padapter->fw->size);
+		dev_err(&padapter->pnetdev->dev, "r8172u: Badfw->size of %d\n",
+			       (int)padapter->fw->size);
 		return 0;
 	}
 	*ppmappedfw = (u8 *)((*praw)->data);
@@ -334,11 +333,13 @@ uint rtl8712_hal_init(struct _adapter *padapter)
 	if (rtl8712_dl_fw(padapter) != _SUCCESS)
 		return _FAIL;
 
-	printk(KERN_INFO "r8712u: 1 RCR=0x%x\n",  r8712_read32(padapter, RCR));
+	netdev_info(padapter->pnetdev, "1 RCR=0x%x\n",
+			r8712_read32(padapter, RCR));
 	val32 = r8712_read32(padapter, RCR);
 	r8712_write32(padapter, RCR, (val32 | BIT(26))); /* Enable RX TCP
 							    Checksum offload */
-	printk(KERN_INFO "r8712u: 2 RCR=0x%x\n", r8712_read32(padapter, RCR));
+	netdev_info(padapter->pnetdev, "2 RCR=0x%x\n",
+			r8712_read32(padapter, RCR));
 	val32 = r8712_read32(padapter, RCR);
 	r8712_write32(padapter, RCR, (val32|BIT(25))); /* Append PHY status */
 	val32 = 0;
diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index e00f791..b65bf5e 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -224,8 +224,7 @@ struct net_device *r8712_init_netdev(void)
 	}
 	padapter = (struct _adapter *) netdev_priv(pnetdev);
 	padapter->pnetdev = pnetdev;
-	printk(KERN_INFO "r8712u: register rtl8712_netdev_ops to"
-	       " netdev_ops\n");
+	pr_info("r8712u: register rtl8712_netdev_ops to netdev_ops\n");
 	pnetdev->netdev_ops = &rtl8712_netdev_ops;
 	pnetdev->watchdog_timeo = HZ; /* 1 second timeout */
 	pnetdev->wireless_handlers = (struct iw_handler_def *)
diff --git a/drivers/staging/rtl8712/rtl8712_recv.c b/drivers/staging/rtl8712/rtl8712_recv.c
index c76732c..725a820 100644
--- a/drivers/staging/rtl8712/rtl8712_recv.c
+++ b/drivers/staging/rtl8712/rtl8712_recv.c
@@ -115,11 +115,11 @@ void r8712_free_recv_priv(struct recv_priv *precvpriv)
 	kfree(precvpriv->pallocated_recv_buf);
 	skb_queue_purge(&precvpriv->rx_skb_queue);
 	if (skb_queue_len(&precvpriv->rx_skb_queue))
-		printk(KERN_WARNING "r8712u: rx_skb_queue not empty\n");
+		netdev_warn(padapter->pnetdev, "r8712u: rx_skb_queue not empty\n");
 	skb_queue_purge(&precvpriv->free_recv_skb_queue);
 	if (skb_queue_len(&precvpriv->free_recv_skb_queue))
-		printk(KERN_WARNING "r8712u: free_recv_skb_queue not empty "
-		       "%d\n", skb_queue_len(&precvpriv->free_recv_skb_queue));
+		netdev_warn(padapter->pnetdev, "r8712u: free_recv_skb_queue not empty %d\n",
+			 skb_queue_len(&precvpriv->free_recv_skb_queue));
 }
 
 int r8712_init_recvbuf(struct _adapter *padapter, struct recv_buf *precvbuf)
@@ -364,9 +364,8 @@ static int amsdu_to_msdu(struct _adapter *padapter, union recv_frame *prframe)
 		nSubframe_Length = (nSubframe_Length >> 8) +
 				   (nSubframe_Length << 8);
 		if (a_len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
-			printk(KERN_WARNING "r8712u: nRemain_Length is %d and"
-			    " nSubframe_Length is: %d\n",
-			    a_len, nSubframe_Length);
+			netdev_warn(padapter->pnetdev, "r8712u: nRemain_Length is %d and nSubframe_Length is: %d\n",
+				a_len, nSubframe_Length);
 			goto exit;
 		}
 		/* move the data point to data content */
@@ -381,8 +380,7 @@ static int amsdu_to_msdu(struct _adapter *padapter, union recv_frame *prframe)
 		memcpy(data_ptr, pdata, nSubframe_Length);
 		subframes[nr_subframes++] = sub_skb;
 		if (nr_subframes >= MAX_SUBFRAME_COUNT) {
-			printk(KERN_WARNING "r8712u: ParseSubframe(): Too"
-			    " many Subframes! Packets dropped!\n");
+			netdev_warn(padapter->pnetdev, "r8712u: ParseSubframe(): Too many Subframes! Packets dropped!\n");
 			break;
 		}
 		pdata += nSubframe_Length;
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index c9a6a7f..cdb51d7 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -415,8 +415,7 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 	} else
 		return -EINVAL;
 	if (strcmp(param->u.crypt.alg, "WEP") == 0) {
-		printk(KERN_INFO "r8712u: wpa_set_encryption, crypt.alg ="
-		       " WEP\n");
+		netdev_info(dev, "r8712u: wpa_set_encryption, crypt.alg = WEP\n");
 		padapter->securitypriv.ndisencryptstatus =
 			     Ndis802_11Encryption1Enabled;
 		padapter->securitypriv.PrivacyAlgrthm = _WEP40_;
@@ -608,8 +607,7 @@ static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
 
 				if ((eid == _VENDOR_SPECIFIC_IE_) &&
 				    (!memcmp(&buf[cnt+2], wps_oui, 4))) {
-					printk(KERN_INFO "r8712u: "
-					       "SET WPS_IE\n");
+					netdev_info(padapter->pnetdev, "r8712u: SET WPS_IE\n");
 					padapter->securitypriv.wps_ie_len =
 					    ((buf[cnt+1] + 2) <
 					    (MAX_WPA_IE_LEN << 2)) ?
@@ -620,8 +618,7 @@ static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
 					    padapter->securitypriv.wps_ie_len);
 					padapter->securitypriv.wps_phase =
 								 true;
-					printk(KERN_INFO "r8712u: SET WPS_IE,"
-					    " wps_phase==true\n");
+					netdev_info(padapter->pnetdev, "r8712u: SET WPS_IE, wps_phase==true\n");
 					cnt += buf[cnt+1]+2;
 					break;
 				} else
@@ -829,8 +826,7 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
 			    strIssueBssid, ETH_ALEN)) {
 				/* BSSID is matched, the same AP => rewrite
 				 * with new PMKID. */
-				printk(KERN_INFO "r8712u: r871x_wx_set_pmkid:"
-				    " BSSID exists in the PMKList.\n");
+				netdev_info(dev, "r8712u: r871x_wx_set_pmkid: BSSID exists in the PMKList.\n");
 				memcpy(psecuritypriv->PMKIDList[j].PMKID,
 					pPMK->pmkid, IW_PMKID_LEN);
 				psecuritypriv->PMKIDList[j].bUsed = true;
@@ -841,8 +837,7 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
 		}
 		if (!blInserted) {
 			/* Find a new entry */
-			printk(KERN_INFO "r8712u: r871x_wx_set_pmkid: Use the"
-			    " new entry index = %d for this PMKID.\n",
+			netdev_info(dev, "r8712u: r871x_wx_set_pmkid: Use the new entry index = %d for this PMKID.\n",
 			    psecuritypriv->PMKIDIndex);
 			memcpy(psecuritypriv->PMKIDList[psecuritypriv->
 				PMKIDIndex].Bssid, strIssueBssid, ETH_ALEN);
@@ -876,8 +871,7 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
 		intReturn = true;
 		break;
 	default:
-		printk(KERN_INFO "r8712u: r871x_wx_set_pmkid: "
-		       "unknown Command\n");
+		netdev_info(dev, "r8712u: r871x_wx_set_pmkid: unknown Command\n");
 		intReturn = false;
 		break;
 	}
@@ -1045,8 +1039,8 @@ static int r871x_wx_set_priv(struct net_device *dev,
 		);
 		sprintf(ext, "OK");
 	} else {
-		printk(KERN_INFO "r8712u: r871x_wx_set_priv: unknown Command"
-		       " %s.\n", ext);
+		netdev_info(dev, "r8712u: r871x_wx_set_priv: unknown Command %s.\n",
+				ext);
 		goto FREE_EXT;
 	}
 	if (copy_to_user(dwrq->pointer, ext,
@@ -1183,8 +1177,8 @@ static int r8711_wx_set_scan(struct net_device *dev,
 	u8 status = true;
 
 	if (padapter->bDriverStopped == true) {
-		printk(KERN_WARNING "r8712u: in r8711_wx_set_scan: "
-		    "bDriverStopped=%d\n", padapter->bDriverStopped);
+		netdev_info(dev, "In r8711_wx_set_scan: bDriverStopped=%d\n",
+					padapter->bDriverStopped);
 		return -1;
 	}
 	if (padapter->bup == false)
@@ -1556,8 +1550,7 @@ static int r8711_wx_set_enc(struct net_device *dev,
 	key = erq->flags & IW_ENCODE_INDEX;
 	memset(&wep, 0, sizeof(struct NDIS_802_11_WEP));
 	if (erq->flags & IW_ENCODE_DISABLED) {
-		printk(KERN_INFO "r8712u: r8711_wx_set_enc: "
-		       "EncryptionDisabled\n");
+		netdev_info(dev, "r8712u: r8711_wx_set_enc: EncryptionDisabled\n");
 		padapter->securitypriv.ndisencryptstatus =
 				 Ndis802_11EncryptionDisabled;
 		padapter->securitypriv.PrivacyAlgrthm = _NO_PRIVACY_;
@@ -1578,8 +1571,7 @@ static int r8711_wx_set_enc(struct net_device *dev,
 	}
 	/* set authentication mode */
 	if (erq->flags & IW_ENCODE_OPEN) {
-		printk(KERN_INFO "r8712u: r8711_wx_set_enc: "
-		       "IW_ENCODE_OPEN\n");
+		netdev_info(dev, "r8712u: r8711_wx_set_enc: IW_ENCODE_OPEN\n");
 		padapter->securitypriv.ndisencryptstatus =
 				 Ndis802_11Encryption1Enabled;
 		padapter->securitypriv.AuthAlgrthm = 0; /* open system */
@@ -1588,8 +1580,7 @@ static int r8711_wx_set_enc(struct net_device *dev,
 		authmode = Ndis802_11AuthModeOpen;
 		padapter->securitypriv.ndisauthtype = authmode;
 	} else if (erq->flags & IW_ENCODE_RESTRICTED) {
-		printk(KERN_INFO "r8712u: r8711_wx_set_enc: "
-		       "IW_ENCODE_RESTRICTED\n");
+		netdev_info(dev, "r8712u: r8711_wx_set_enc: IW_ENCODE_RESTRICTED\n");
 		padapter->securitypriv.ndisencryptstatus =
 				 Ndis802_11Encryption1Enabled;
 		padapter->securitypriv.AuthAlgrthm = 1; /* shared system */
@@ -1977,8 +1968,7 @@ static int r871x_mp_ioctl_hdl(struct net_device *dev,
 		status = phandler->handler(&oid_par);
 		/* todo:check status, BytesNeeded, etc. */
 	} else {
-		printk(KERN_INFO "r8712u: r871x_mp_ioctl_hdl(): err!,"
-		    " subcode=%d, oid=%d, handler=%p\n",
+		netdev_info(dev, "r8712u: r871x_mp_ioctl_hdl(): err!, subcode=%d, oid=%d, handler=%p\n",
 		    poidparam->subcode, phandler->oid, phandler->handler);
 		ret = -EFAULT;
 		goto _r871x_mp_ioctl_hdl_exit;
@@ -2034,13 +2024,13 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 		pnetwork = LIST_CONTAINOR(plist, struct wlan_network, list);
 		if (hwaddr_aton_i(data, bssid)) {
-			printk(KERN_INFO "r8712u: Invalid BSSID '%s'.\n",
-			       (u8 *)data);
+			netdev_info(dev, "r8712u: Invalid BSSID '%s'.\n",
+				       (u8 *)data);
 			spin_unlock_irqrestore(&(pmlmepriv->scanned_queue.lock),
 									irqL);
 			return -EINVAL;
 		}
-		printk(KERN_INFO "r8712u: BSSID:%pM\n", bssid);
+		netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
 		if (!memcmp(bssid, pnetwork->network.MacAddress, ETH_ALEN)) {
 			/* BSSID match, then check if supporting wpa/wpa2 */
 			pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index c51ad9e..0e20c62 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1048,7 +1048,7 @@ void r8712_got_addbareq_event_callback(struct _adapter *adapter, u8 *pbuf)
 	struct	sta_priv *pstapriv = &adapter->stapriv;
 	struct	recv_reorder_ctrl *precvreorder_ctrl = NULL;
 
-	printk(KERN_INFO "r8712u: [%s] mac = %pM, seq = %d, tid = %d\n",
+	netdev_info(adapter->pnetdev, "[%s] mac = %pM, seq = %d, tid = %d\n",
 	     __func__, pAddbareq_pram->MacAddress,
 	    pAddbareq_pram->StartSeqNum, pAddbareq_pram->tid);
 	psta = r8712_get_stainfo(pstapriv, pAddbareq_pram->MacAddress);
diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
index 6b73843..41ab9e8 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -203,9 +203,9 @@ static int r871x_suspend(struct usb_interface *pusb_intf, pm_message_t state)
 {
 	struct net_device *pnetdev = usb_get_intfdata(pusb_intf);
 
-	printk(KERN_INFO "r8712: suspending...\n");
+	netdev_info(pnetdev, "Suspending...\n");
 	if (!pnetdev || !netif_running(pnetdev)) {
-		printk(KERN_INFO "r8712: unable to suspend\n");
+		netdev_info(pnetdev, "Unable to suspend\n");
 		return 0;
 	}
 	if (pnetdev->netdev_ops->ndo_stop)
@@ -219,9 +219,9 @@ static int r871x_resume(struct usb_interface *pusb_intf)
 {
 	struct net_device *pnetdev = usb_get_intfdata(pusb_intf);
 
-	printk(KERN_INFO "r8712: resuming...\n");
+	netdev_info(pnetdev,  "Resuming...\n");
 	if (!pnetdev || !netif_running(pnetdev)) {
-		printk(KERN_INFO "r8712: unable to resume\n");
+		netdev_info(pnetdev, "Unable to resume\n");
 		return 0;
 	}
 	netif_device_attach(pnetdev);
@@ -271,11 +271,11 @@ static uint r8712_usb_dvobj_init(struct _adapter *padapter)
 	pdvobjpriv->nr_endpoint = piface_desc->bNumEndpoints;
 	if (pusbd->speed == USB_SPEED_HIGH) {
 		pdvobjpriv->ishighspeed = true;
-		printk(KERN_INFO "r8712u: USB_SPEED_HIGH with %d endpoints\n",
+		dev_info(&pusbd->dev, "r8712u: USB_SPEED_HIGH with %d endpoints\n",
 		       pdvobjpriv->nr_endpoint);
 	} else {
 		pdvobjpriv->ishighspeed = false;
-		printk(KERN_INFO "r8712u: USB_SPEED_LOW with %d endpoints\n",
+		dev_info(&pusbd->dev, "r8712u: USB_SPEED_LOW with %d endpoints\n",
 		       pdvobjpriv->nr_endpoint);
 	}
 	if ((r8712_alloc_io_queue(padapter)) == _FAIL)
@@ -421,7 +421,7 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
 		tmpU1b = r8712_read8(padapter, EE_9346CR);/*CR9346*/
 
 		/* To check system boot selection.*/
-		printk(KERN_INFO "r8712u: Boot from %s: Autoload %s\n",
+		dev_info(&udev->dev, "r8712u: Boot from %s: Autoload %s\n",
 		       (tmpU1b & _9356SEL) ? "EEPROM" : "EFUSE",
 		       (tmpU1b & _EEPROM_EN) ? "OK" : "Failed");
 
@@ -531,7 +531,7 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
 						 RT_CID_DEFAULT;
 				break;
 			}
-			printk(KERN_INFO "r8712u: CustomerID = 0x%.4x\n",
+			dev_info(&udev->dev, "CustomerID = 0x%.4x\n",
 			     padapter->eeprompriv.CustomerID);
 			/* Led mode */
 			switch (padapter->eeprompriv.CustomerID) {
@@ -588,11 +588,9 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
 			 * address by setting bit 1 of first octet.
 			 */
 			mac[0] &= 0xFE;
-			printk(KERN_INFO "r8712u: MAC Address from user = "
-			       "%pM\n", mac);
+			dev_info(&udev->dev, "r8712u: MAC Address from user = %pM\n", mac);
 		} else
-			printk(KERN_INFO "r8712u: MAC Address from efuse = "
-			       "%pM\n", mac);
+			dev_info(&udev->dev, "r8712u: MAC Address from efuse = %pM\n", mac);
 		memcpy(pnetdev->dev_addr, mac, ETH_ALEN);
 	}
 	/* step 6. Load the firmware asynchronously */
@@ -659,7 +657,6 @@ static void __exit r8712u_drv_halt(void)
 {
 	drvpriv.drv_registered = false;
 	usb_deregister(&drvpriv.r871xu_drv);
-	printk(KERN_INFO "r8712u: Driver unloaded\n");
 }
 
 module_init(r8712u_drv_entry);
diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
index 24e1ec5..d3234d5 100644
--- a/drivers/staging/rtl8712/usb_ops_linux.c
+++ b/drivers/staging/rtl8712/usb_ops_linux.c
@@ -243,8 +243,7 @@ static void r8712_usb_read_port_complete(struct urb *purb)
 				  (unsigned char *)precvbuf);
 			break;
 		case -EINPROGRESS:
-			printk(KERN_ERR "r8712u: ERROR: URB IS IN"
-			       " PROGRESS!/n");
+			netdev_err(padapter->pnetdev, "ERROR: URB IS IN PROGRESS!/n");
 			break;
 		default:
 			break;
@@ -336,8 +335,7 @@ void r8712_xmit_bh(void *priv)
 
 	if ((padapter->bDriverStopped == true) ||
 	    (padapter->bSurpriseRemoved == true)) {
-		printk(KERN_ERR "r8712u: xmit_bh => bDriverStopped"
-		       " or bSurpriseRemoved\n");
+		netdev_err(padapter->pnetdev, "xmit_bh => bDriverStopped or bSurpriseRemoved\n");
 		return;
 	}
 	ret = r8712_xmitframe_complete(padapter, pxmitpriv, NULL);
@@ -387,7 +385,7 @@ static void usb_write_port_complete(struct urb *purb)
 	case 0:
 		break;
 	default:
-		printk(KERN_WARNING "r8712u: pipe error: (%d)\n", purb->status);
+		netdev_warn(padapter->pnetdev, "r8712u: pipe error: (%d)\n", purb->status);
 		break;
 	}
 	/* not to consider tx fragment */
@@ -502,8 +500,8 @@ int r8712_usbctrl_vendorreq(struct intf_priv *pintfpriv, u8 request, u16 value,
 
 	palloc_buf = _malloc((u32) len + 16);
 	if (palloc_buf == NULL) {
-		printk(KERN_ERR "r8712u: [%s] Can't alloc memory for vendor"
-		       " request\n", __func__);
+		dev_err(&udev->dev, "[%s] Can't alloc memory for vendor request\n",
+			__func__);
 		return -1;
 	}
 	pIo_buf = palloc_buf + 16 - ((addr_t)(palloc_buf) & 0x0f);
diff --git a/drivers/staging/rtl8712/xmit_linux.c b/drivers/staging/rtl8712/xmit_linux.c
index 65542cb..85a5123 100644
--- a/drivers/staging/rtl8712/xmit_linux.c
+++ b/drivers/staging/rtl8712/xmit_linux.c
@@ -134,8 +134,7 @@ int r8712_xmit_resource_alloc(struct _adapter *padapter,
 	for (i = 0; i < 8; i++) {
 		pxmitbuf->pxmit_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
 		if (pxmitbuf->pxmit_urb[i] == NULL) {
-			printk(KERN_ERR "r8712u: pxmitbuf->pxmit_urb[i]"
-			    " == NULL");
+			netdev_err(padapter->pnetdev, "pxmitbuf->pxmit_urb[i] == NULL");
 			return _FAIL;
 		}
 	}
-- 
1.8.0.1

Re: [PATCH 3/5] rtl8712: replace printk with better solutions

[Joe Perches]

On Sun, 2012-12-09 at 10:15 +0000, Przemo Firszt wrote:
> Replace printk with netdev_printk helpers, dev_printk helpers or
> pr_err/warn/info if there is no device info available.

Here's a few trivial comments

> @@ -84,8 +83,8 @@ static u32 rtl871x_open_fw(struct _adapter *padapter, const u8 **ppmappedfw)
>  	const struct firmware **praw = &padapter->fw;
>  
>  	if (padapter->fw->size > 200000) {
> -		printk(KERN_ERR "r8172u: Badfw->size of %d\n",
> -		       (int)padapter->fw->size);
> +		dev_err(&padapter->pnetdev->dev, "r8172u: Badfw->size of %d\n",
> +			       (int)padapter->fw->size);

Please align arguments to the open parenthesis
like this:

		dev_err(dev, format,
			args);

If you are using checkpatch, add --strict to get the
script to tell you when the alignment isn't correct.

> diff --git a/drivers/staging/rtl8712/rtl8712_recv.c b/drivers/staging/rtl8712/rtl8712_recv.c
[]
> @@ -829,8 +826,7 @@ static int r871x_wx_set_pmkid(struct net_device *dev,
>  			    strIssueBssid, ETH_ALEN)) {
>  				/* BSSID is matched, the same AP => rewrite
>  				 * with new PMKID. */
> -				printk(KERN_INFO "r8712u: r871x_wx_set_pmkid:"
> -				    " BSSID exists in the PMKList.\n");
> +				netdev_info(dev, "r8712u: r871x_wx_set_pmkid: BSSID exists in the PMKList.\n");

when the format contains the function name, convert
it to use %s, ... __func__
Remove unnecessary trailing periods from the formats.

				netdev_info(dev, "%s: BSSID exists in the PMKList\n",
					    __func__);

> diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
[]
> @@ -243,8 +243,7 @@ static void r8712_usb_read_port_complete(struct urb *purb)
>  				  (unsigned char *)precvbuf);
>  			break;
>  		case -EINPROGRESS:
> -			printk(KERN_ERR "r8712u: ERROR: URB IS IN"
> -			       " PROGRESS!/n");
> +			netdev_err(padapter->pnetdev, "ERROR: URB IS IN PROGRESS!/n");

newlines are \n not /n

> diff --git a/drivers/staging/rtl8712/xmit_linux.c b/drivers/staging/rtl8712/xmit_linux.c
[]
> @@ -134,8 +134,7 @@ int r8712_xmit_resource_alloc(struct _adapter *padapter,
>  	for (i = 0; i < 8; i++) {
>  		pxmitbuf->pxmit_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
>  		if (pxmitbuf->pxmit_urb[i] == NULL) {
> -			printk(KERN_ERR "r8712u: pxmitbuf->pxmit_urb[i]"
> -			    " == NULL");
> +			netdev_err(padapter->pnetdev, "pxmitbuf->pxmit_urb[i] == NULL");

Make sure message formats are newline terminated.

[PATCH 4/5] rtl8712: replace min with min_t

[Przemo Firszt]

A clean up change suggested by checkpatch.pl

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index cdb51d7..b131b61 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -188,8 +188,7 @@ static inline char *translate_scan(struct _adapter *padapter,
 	/* Add the ESSID */
 	iwe.cmd = SIOCGIWESSID;
 	iwe.u.data.flags = 1;
-	iwe.u.data.length = (u16)min((u16)pnetwork->network.Ssid.SsidLength,
-			    (u16)32);
+	iwe.u.data.length = min_t(u16, pnetwork->network.Ssid.SsidLength, 32);
 	start = iwe_stream_add_point(info, start, stop, &iwe,
 				     pnetwork->network.Ssid.Ssid);
 	/* parsing HT_CAP_IE */
@@ -1193,8 +1192,7 @@ static int r8711_wx_set_scan(struct net_device *dev,
 		if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
 			struct ndis_802_11_ssid ssid;
 			unsigned long irqL;
-			u32 len = (u32) min((u8)req->essid_len,
-				  (u8)IW_ESSID_MAX_SIZE);
+			u32 len = min_t(u8, req->essid_len, IW_ESSID_MAX_SIZE);
 			memset((unsigned char *)&ssid, 0,
 				 sizeof(struct ndis_802_11_ssid));
 			memcpy(ssid.Ssid, req->essid, len);
-- 
1.8.0.1

Re: [PATCH 4/5] rtl8712: replace min with min_t

[Dan Carpenter]

On Sun, Dec 09, 2012 at 10:15:09AM +0000, Przemo Firszt wrote:
> A clean up change suggested by checkpatch.pl
> 
> Signed-off-by: Przemo Firszt <przemo@firszt.eu>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index cdb51d7..b131b61 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -188,8 +188,7 @@ static inline char *translate_scan(struct _adapter *padapter,
>  	/* Add the ESSID */
>  	iwe.cmd = SIOCGIWESSID;
>  	iwe.u.data.flags = 1;
> -	iwe.u.data.length = (u16)min((u16)pnetwork->network.Ssid.SsidLength,
> -			    (u16)32);
> +	iwe.u.data.length = min_t(u16, pnetwork->network.Ssid.SsidLength, 32);

pnetwork->network.Ssid.SsidLength is a u32 so it would be better to
not truncate the upper bits away.  It's not going to cause a
problem, but its slightly messy.

This is a common problem where people take the type of
iwe.u.data.length and cast to that instead of considering the types
for the data they are comparing.

regards,
dan carpenter

Re: [PATCH 4/5] rtl8712: replace min with min_t

[Przemo Firszt]

Dnia 2012-12-10, pon o godzinie 12:41 +0300, Dan Carpenter pisze:
> On Sun, Dec 09, 2012 at 10:15:09AM +0000, Przemo Firszt wrote:
> > A clean up change suggested by checkpatch.pl
> > 
> > Signed-off-by: Przemo Firszt <przemo@firszt.eu>
> > ---
> >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > index cdb51d7..b131b61 100644
> > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > @@ -188,8 +188,7 @@ static inline char *translate_scan(struct _adapter *padapter,
> >  	/* Add the ESSID */
> >  	iwe.cmd = SIOCGIWESSID;
> >  	iwe.u.data.flags = 1;
> > -	iwe.u.data.length = (u16)min((u16)pnetwork->network.Ssid.SsidLength,
> > -			    (u16)32);
> > +	iwe.u.data.length = min_t(u16, pnetwork->network.Ssid.SsidLength, 32);
> 
> pnetwork->network.Ssid.SsidLength is a u32 so it would be better to
> not truncate the upper bits away.  It's not going to cause a
> problem, but its slightly messy.
> 
> This is a common problem where people take the type of
> iwe.u.data.length and cast to that instead of considering the types
> for the data they are comparing.
> 
Dan,
Thanks for the comment!

iew.u.data.length is __u16

min_t is defined as:
#define min_t(type, x, y) ({                    \
        type __min1 = (x);                      \
        type __min2 = (y);                      \
        __min1 < __min2 ? __min1: __min2; })

If I understand you correctly I should use this:

iwe.u.data.length = min_t(u32, pnetwork->network.Ssid.SsidLength, 32);

the result of min_t is u32, but iwe.u.data.lenght is __u16

Is that correct?
-- 
Kind regards,
Przemo Firszt

Re: [PATCH 4/5] rtl8712: replace min with min_t

[Dan Carpenter]

On Mon, Dec 10, 2012 at 10:21:52PM +0000, Przemo Firszt wrote:
> Dnia 2012-12-10, pon o godzinie 12:41 +0300, Dan Carpenter pisze:
> > On Sun, Dec 09, 2012 at 10:15:09AM +0000, Przemo Firszt wrote:
> > > A clean up change suggested by checkpatch.pl
> > > 
> > > Signed-off-by: Przemo Firszt <przemo@firszt.eu>
> > > ---
> > >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > index cdb51d7..b131b61 100644
> > > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > @@ -188,8 +188,7 @@ static inline char *translate_scan(struct _adapter *padapter,
> > >  	/* Add the ESSID */
> > >  	iwe.cmd = SIOCGIWESSID;
> > >  	iwe.u.data.flags = 1;
> > > -	iwe.u.data.length = (u16)min((u16)pnetwork->network.Ssid.SsidLength,
> > > -			    (u16)32);
> > > +	iwe.u.data.length = min_t(u16, pnetwork->network.Ssid.SsidLength, 32);
> > 
> > pnetwork->network.Ssid.SsidLength is a u32 so it would be better to
> > not truncate the upper bits away.  It's not going to cause a
> > problem, but its slightly messy.
> > 
> > This is a common problem where people take the type of
> > iwe.u.data.length and cast to that instead of considering the types
> > for the data they are comparing.
> > 
> Dan,
> Thanks for the comment!
> 
> iew.u.data.length is __u16
> 
> min_t is defined as:
> #define min_t(type, x, y) ({                    \
>         type __min1 = (x);                      \
>         type __min2 = (y);                      \
>         __min1 < __min2 ? __min1: __min2; })
> 
> If I understand you correctly I should use this:
> 
> iwe.u.data.length = min_t(u32, pnetwork->network.Ssid.SsidLength, 32);
> 
> the result of min_t is u32, but iwe.u.data.lenght is __u16
> 

At the end iwe.u.data.length is going to be a number between 0 and
32.  That can easily fit in 16 bits no problem.

The difference is that, imagine ->network.Ssid.SsidLength is larger
than u16 like 0x10001.  The cast to u16 changes it to 0x0001 which
is less than 32 so we would say the minimum is 1 when actually we
want to say 32 is the min.

regards,
dan carpenter

Re: [PATCH 4/5] rtl8712: replace min with min_t

[Przemo Firszt]

Dnia 2012-12-11, wto o godzinie 01:49 +0300, Dan Carpenter pisze:
[..]
> At the end iwe.u.data.length is going to be a number between 0 and
> 32.  That can easily fit in 16 bits no problem.
> 
> The difference is that, imagine ->network.Ssid.SsidLength is larger
> than u16 like 0x10001.  The cast to u16 changes it to 0x0001 which
> is less than 32 so we would say the minimum is 1 when actually we
> want to say 32 is the min.
> 
Dan,
Thank you! That was the thing that I missed...
New set of patches on the way.
-- 
Kind regards,
Przemo Firszt

[PATCH 5/5] rtl8712: replace leading spaces with tab

[Przemo Firszt]

Replace leading spaces with tab

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index b131b61..b4629c3 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -510,7 +510,7 @@ exit:
 }
 
 static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
-			    unsigned short ielen)
+				unsigned short ielen)
 {
 	u8 *buf = NULL, *pos = NULL;
 	int group_cipher = 0, pairwise_cipher = 0;
-- 
1.8.0.1

Re: [PATCH 5/5] rtl8712: replace leading spaces with tab

[Dan Carpenter]

On Sun, Dec 09, 2012 at 10:15:10AM +0000, Przemo Firszt wrote:
> Replace leading spaces with tab
> 
> Signed-off-by: Przemo Firszt <przemo@firszt.eu>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index b131b61..b4629c3 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -510,7 +510,7 @@ exit:
>  }
>  
>  static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
> -			    unsigned short ielen)
> +				unsigned short ielen)

The original version is better because it lines up correctly.

Checkpatch doesn't complain for me so I'm not sure what the story is
here.

regards,
dan carpenter

Re: [PATCH 5/5] rtl8712: replace leading spaces with tab

[Joe Perches]

On Mon, 2012-12-10 at 12:46 +0300, Dan Carpenter wrote:
> On Sun, Dec 09, 2012 at 10:15:10AM +0000, Przemo Firszt wrote:
> > Replace leading spaces with tab
[]
> > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
[]
> >  static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
> > -			    unsigned short ielen)
> > +				unsigned short ielen)
> 
> The original version is better because it lines up correctly.
> 
> Checkpatch doesn't complain for me so I'm not sure what the story is
> here.

Parenthesis alignment isn't described in CodingStyle.
It's maintainer's preference.  Whether or not it should
be is a different question.

checkpatch parenthesis alignment of arguments checks are
emitted only when adding --strict to the command line.

Re: [PATCH 5/5] rtl8712: replace leading spaces with tab

[Dan Carpenter]

On Mon, Dec 10, 2012 at 03:49:46AM -0800, Joe Perches wrote:
> On Mon, 2012-12-10 at 12:46 +0300, Dan Carpenter wrote:
> > On Sun, Dec 09, 2012 at 10:15:10AM +0000, Przemo Firszt wrote:
> > > Replace leading spaces with tab
> []
> > > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> []
> > >  static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
> > > -			    unsigned short ielen)
> > > +				unsigned short ielen)
> > 
> > The original version is better because it lines up correctly.
> > 
> > Checkpatch doesn't complain for me so I'm not sure what the story is
> > here.
> 
> Parenthesis alignment isn't described in CodingStyle.
> It's maintainer's preference.  Whether or not it should
> be is a different question.
> 
> checkpatch parenthesis alignment of arguments checks are
> emitted only when adding --strict to the command line.
> 

Even with --strict checkpatch.pl is fine with the original code.

I don't know any subsystems where the new version would be prefered
over the original version.

regards,
dan carpenter

Re: [PATCH 5/5] rtl8712: replace leading spaces with tab

[Joe Perches]

On Mon, 2012-12-10 at 15:21 +0300, Dan Carpenter wrote:
> On Mon, Dec 10, 2012 at 03:49:46AM -0800, Joe Perches wrote:
> > checkpatch parenthesis alignment of arguments checks are
> > emitted only when adding --strict to the command line.
> Even with --strict checkpatch.pl is fine with the original code.

Right. I just checked.
Parenthesis alignment is only done for

	if (

It might be tricky to do it correctly
for other cases.