[PATCH v3 0/6] staging: ks7010: some clean ups in source and header files

[PATCH v3 0/6] staging: ks7010: some clean ups in source and header files

[Sergio Paracuellos]

The following patch series makes several changes to improve readability of
some functions around several header and source files. 

One checkpatch script warning and some camel cases are removed also.

v2:
 * Remove compilation warning because a declaration after initialization 
  in hostif_aplist_init factored out function

v3:
 * PATCH 6/6: add blank line between declarations and normal code 
 statements to better readability.

Sergio Paracuellos (6):
  staging: ks7010: use GENMASK instead of custom defines in
    SME_WEP_VAL_MASK
  staging: ks7010: remove some dead code from ks_wlan_set_essid function
  staging: ks7010: avoid camel case in function name get_BYTE
  staging: ks7010: avoid camel case in function name get_WORD
  staging: ks7010: avoid camel case in function name get_DWORD
  staging: ks7010: factor out some functions from hostif_init

 drivers/staging/ks7010/ks_hostif.c   | 140 +++++++++++++++++++++--------------
 drivers/staging/ks7010/ks_wlan.h     |   2 +-
 drivers/staging/ks7010/ks_wlan_net.c |  15 ----
 3 files changed, 84 insertions(+), 73 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/6] staging: ks7010: use GENMASK instead of custom defines in SME_WEP_VAL_MASK

[Sergio Paracuellos]

This commits replaces custom defines that were been used
to define a mask using GENMASK macro from linux bitops
header file.

With this change the "WARNING: line over 80 characters"
warning message reported by checkpatch script is also
removed.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index aeabbe3..ff6b65a 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -87,7 +87,7 @@ enum {
 #define SME_WEP_VAL2        BIT(6)
 #define SME_WEP_VAL3        BIT(7)
 #define SME_WEP_VAL4        BIT(8)
-#define SME_WEP_VAL_MASK    (SME_WEP_VAL1 | SME_WEP_VAL2 | SME_WEP_VAL3 | SME_WEP_VAL4)
+#define SME_WEP_VAL_MASK    GENMASK(8, 5)
 #define SME_RSN             BIT(9)
 #define SME_RSN_MULTICAST   BIT(10)
 #define SME_RSN_UNICAST	    BIT(11)
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/6] staging: ks7010: remove some dead code from ks_wlan_set_essid function

[Sergio Paracuellos]

This commit removes death code which is not being used at all because
it is inside a preprocessor if 0 directive. This change improves a bit
readability of ks_wlan_set_essid function.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 6106e79..4019f3a 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -276,7 +276,6 @@ static int ks_wlan_set_essid(struct net_device *dev,
 		memset(priv->reg.ssid.body, 0, sizeof(priv->reg.ssid.body));
 		priv->reg.ssid.size = 0;
 	} else {
-#if 1
 		len = dwrq->length;
 		/* iwconfig uses nul termination in SSID.. */
 		if (len > 0 && extra[len - 1] == '\0')
@@ -286,28 +285,14 @@ static int ks_wlan_set_essid(struct net_device *dev,
 		if (len > IW_ESSID_MAX_SIZE)
 			return -EINVAL;
 
-#else
-		/* Check the size of the string */
-		if (dwrq->length > IW_ESSID_MAX_SIZE + 1)
-			return -E2BIG;
-
-#endif
-
 		/* Set the SSID */
 		memset(priv->reg.ssid.body, 0, sizeof(priv->reg.ssid.body));
-
-#if 1
 		memcpy(priv->reg.ssid.body, extra, len);
 		priv->reg.ssid.size = len;
-#else
-		memcpy(priv->reg.ssid.body, extra, dwrq->length);
-		priv->reg.ssid.size = dwrq->length;
-#endif
 	}
 	/* Write it to the card */
 	priv->need_commit |= SME_MODE_SET;
 
-//      return  -EINPROGRESS;   /* Call commit handler */
 	ks_wlan_setup_parameter(priv, priv->need_commit);
 	priv->need_commit = 0;
 	return 0;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 3/6] staging: ks7010: avoid camel case in function name get_BYTE

[Sergio Paracuellos]

This commit renames get_BYTE function in favour of get_byte
to avoid camel case.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_hostif.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index cab62b4..7777d38 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -40,8 +40,7 @@ static inline unsigned int cnt_smeqbody(struct ks_wlan_private *priv)
 
 #define KS_WLAN_MEM_FLAG (GFP_ATOMIC)
 
-static
-inline u8 get_BYTE(struct ks_wlan_private *priv)
+static inline u8 get_byte(struct ks_wlan_private *priv)
 {
 	u8 data;
 
@@ -56,8 +55,8 @@ inline u16 get_WORD(struct ks_wlan_private *priv)
 {
 	u16 data;
 
-	data = (get_BYTE(priv) & 0xff);
-	data |= ((get_BYTE(priv) << 8) & 0xff00);
+	data = (get_byte(priv) & 0xff);
+	data |= ((get_byte(priv) << 8) & 0xff00);
 	return data;
 }
 
@@ -66,10 +65,10 @@ inline u32 get_DWORD(struct ks_wlan_private *priv)
 {
 	u32 data;
 
-	data = (get_BYTE(priv) & 0xff);
-	data |= ((get_BYTE(priv) << 8) & 0x0000ff00);
-	data |= ((get_BYTE(priv) << 16) & 0x00ff0000);
-	data |= ((get_BYTE(priv) << 24) & 0xff000000);
+	data = (get_byte(priv) & 0xff);
+	data |= ((get_byte(priv) << 8) & 0x0000ff00);
+	data |= ((get_byte(priv) << 16) & 0x00ff0000);
+	data |= ((get_byte(priv) << 24) & 0xff000000);
 	return data;
 }
 
@@ -934,10 +933,10 @@ void hostif_phy_information_confirm(struct ks_wlan_private *priv)
 	unsigned int transmitted_frame_count, received_fragment_count;
 	unsigned int failed_count, fcs_error_count;
 
-	rssi = get_BYTE(priv);
-	signal = get_BYTE(priv);
-	noise = get_BYTE(priv);
-	link_speed = get_BYTE(priv);
+	rssi = get_byte(priv);
+	signal = get_byte(priv);
+	noise = get_byte(priv);
+	link_speed = get_byte(priv);
 	transmitted_frame_count = get_DWORD(priv);
 	received_fragment_count = get_DWORD(priv);
 	failed_count = get_DWORD(priv);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 4/6] staging: ks7010: avoid camel case in function name get_WORD

[Sergio Paracuellos]

This commit renames get_WORD function into get_word to avoid
camel case.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_hostif.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 7777d38..5bc29dd 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -50,8 +50,7 @@ static inline u8 get_byte(struct ks_wlan_private *priv)
 	return data;
 }
 
-static
-inline u16 get_WORD(struct ks_wlan_private *priv)
+static inline u16 get_word(struct ks_wlan_private *priv)
 {
 	u16 data;
 
@@ -406,8 +405,8 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 		return;
 	}
 
-	auth_type = get_WORD(priv);	/* AuthType */
-	get_WORD(priv);	/* Reserve Area */
+	auth_type = get_word(priv);	/* AuthType */
+	get_word(priv);	/* Reserve Area */
 
 	eth_hdr = (struct ether_hdr *)(priv->rxp);
 	eth_proto = ntohs(eth_hdr->h_proto);
@@ -510,8 +509,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 
 	mib_status = get_DWORD(priv);	/* MIB status */
 	mib_attribute = get_DWORD(priv);	/* MIB atttibute */
-	mib_val_size = get_WORD(priv);	/* MIB value size */
-	mib_val_type = get_WORD(priv);	/* MIB value type */
+	mib_val_size = get_word(priv);	/* MIB value size */
+	mib_val_type = get_word(priv);	/* MIB value type */
 
 	if (mib_status) {
 		/* in case of error */
@@ -724,7 +723,7 @@ void hostif_connect_indication(struct ks_wlan_private *priv)
 	struct net_device *netdev = priv->net_dev;
 	union iwreq_data wrqu0;
 
-	connect_code = get_WORD(priv);
+	connect_code = get_word(priv);
 
 	switch (connect_code) {
 	case RESULT_CONNECT:	/* connect */
@@ -850,7 +849,7 @@ void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
 {
 	u16 result_code;
 
-	result_code = get_WORD(priv);
+	result_code = get_word(priv);
 	priv->infra_status = 1;	/* infrastructure mode set */
 	hostif_sme_enqueue(priv, SME_MODE_SET_CONFIRM);
 }
@@ -975,7 +974,7 @@ void hostif_event_check(struct ks_wlan_private *priv)
 {
 	unsigned short event;
 
-	event = get_WORD(priv);	/* get event */
+	event = get_word(priv);	/* get event */
 	switch (event) {
 	case HIF_DATA_IND:
 		hostif_data_indication(priv);
@@ -1606,7 +1605,7 @@ void hostif_receive(struct ks_wlan_private *priv, unsigned char *p,
 	priv->rxp = p;
 	priv->rx_size = size;
 
-	if (get_WORD(priv) == priv->rx_size) {	/* length check !! */
+	if (get_word(priv) == priv->rx_size) {	/* length check !! */
 		hostif_event_check(priv);	/* event check */
 	}
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 5/6] staging: ks7010: avoid camel case in function name get_DWORD

[Sergio Paracuellos]

This commit renames function get_DWORD into get_dword to avoid
camel case.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_hostif.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 5bc29dd..037eb6e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -59,8 +59,7 @@ static inline u16 get_word(struct ks_wlan_private *priv)
 	return data;
 }
 
-static
-inline u32 get_DWORD(struct ks_wlan_private *priv)
+static inline u32 get_dword(struct ks_wlan_private *priv)
 {
 	u32 data;
 
@@ -507,8 +506,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 	u16 mib_val_size;
 	u16 mib_val_type;
 
-	mib_status = get_DWORD(priv);	/* MIB status */
-	mib_attribute = get_DWORD(priv);	/* MIB atttibute */
+	mib_status = get_dword(priv);	/* MIB status */
+	mib_attribute = get_dword(priv);	/* MIB atttibute */
 	mib_val_size = get_word(priv);	/* MIB value size */
 	mib_val_type = get_word(priv);	/* MIB value type */
 
@@ -580,8 +579,8 @@ void hostif_mib_set_confirm(struct ks_wlan_private *priv)
 	u32 mib_status;	/* +04 MIB Status */
 	u32 mib_attribute;	/* +08 MIB attribute */
 
-	mib_status = get_DWORD(priv);	/* MIB Status */
-	mib_attribute = get_DWORD(priv);	/* MIB attribute */
+	mib_status = get_dword(priv);	/* MIB Status */
+	mib_attribute = get_dword(priv);	/* MIB attribute */
 
 	if (mib_status) {
 		/* in case of error */
@@ -910,7 +909,7 @@ void hostif_bss_scan_confirm(struct ks_wlan_private *priv)
 	struct net_device *dev = priv->net_dev;
 	union iwreq_data wrqu;
 
-	result_code = get_DWORD(priv);
+	result_code = get_dword(priv);
 	netdev_dbg(priv->net_dev, "result=%d :: scan_ind_count=%d\n", result_code,
 		   priv->scan_ind_count);
 
@@ -936,10 +935,10 @@ void hostif_phy_information_confirm(struct ks_wlan_private *priv)
 	signal = get_byte(priv);
 	noise = get_byte(priv);
 	link_speed = get_byte(priv);
-	transmitted_frame_count = get_DWORD(priv);
-	received_fragment_count = get_DWORD(priv);
-	failed_count = get_DWORD(priv);
-	fcs_error_count = get_DWORD(priv);
+	transmitted_frame_count = get_dword(priv);
+	received_fragment_count = get_dword(priv);
+	failed_count = get_dword(priv);
+	fcs_error_count = get_dword(priv);
 
 	netdev_dbg(priv->net_dev, "phyinfo confirm rssi=%d signal=%d\n",
 		   rssi, signal);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 6/6] staging: ks7010: factor out some functions from hostif_init

[Sergio Paracuellos]

This commit factors out some init functions from hostif_init
to improve readability.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_hostif.c | 77 ++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 037eb6e..923f196 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2357,50 +2357,79 @@ void hostif_sme_enqueue(struct ks_wlan_private *priv, unsigned short event)
 	tasklet_schedule(&priv->sme_task);
 }
 
-int hostif_init(struct ks_wlan_private *priv)
+static inline void hostif_aplist_init(struct ks_wlan_private *priv)
 {
-	int i;
-
+	size_t size = LOCAL_APLIST_MAX * sizeof(struct local_ap_t);
 	priv->aplist.size = 0;
-	for (i = 0; i < LOCAL_APLIST_MAX; i++)
-		memset(&priv->aplist.ap[i], 0, sizeof(struct local_ap_t));
+	memset(&priv->aplist.ap[0], 0, size);
+}
+
+static inline void hostif_status_init(struct ks_wlan_private *priv)
+{
 	priv->infra_status = 0;
 	priv->current_rate = 4;
 	priv->connect_status = DISCONNECT_STATUS;
+}
 
-	spin_lock_init(&priv->multicast_spin);
+static inline void hostif_sme_init(struct ks_wlan_private *priv)
+{
+	priv->sme_i.sme_status = SME_IDLE;
+	priv->sme_i.qhead = 0;
+	priv->sme_i.qtail = 0;
+	spin_lock_init(&priv->sme_i.sme_spin);
+	priv->sme_i.sme_flag = 0;
+	tasklet_init(&priv->sme_task, hostif_sme_task, (unsigned long)priv);
+}
 
-	spin_lock_init(&priv->dev_read_lock);
-	init_waitqueue_head(&priv->devread_wait);
-	priv->dev_count = 0;
-	atomic_set(&priv->event_count, 0);
-	atomic_set(&priv->rec_count, 0);
+static inline void hostif_wpa_init(struct ks_wlan_private *priv)
+{
+	memset(&priv->wpa, 0, sizeof(priv->wpa));
+	priv->wpa.rsn_enabled = 0;
+	priv->wpa.mic_failure.failure = 0;
+	priv->wpa.mic_failure.last_failure_time = 0;
+	priv->wpa.mic_failure.stop = 0;
+}
 
-	/* for power save */
+static inline void hostif_power_save_init(struct ks_wlan_private *priv)
+{
 	atomic_set(&priv->psstatus.status, PS_NONE);
 	atomic_set(&priv->psstatus.confirm_wait, 0);
 	atomic_set(&priv->psstatus.snooze_guard, 0);
 	init_completion(&priv->psstatus.wakeup_wait);
 	INIT_WORK(&priv->wakeup_work, ks_wlan_hw_wakeup_task);
+}
+
+static inline void hostif_pmklist_init(struct ks_wlan_private *priv)
+{
+	int i;
 
-	/* WPA */
-	memset(&priv->wpa, 0, sizeof(priv->wpa));
-	priv->wpa.rsn_enabled = 0;
-	priv->wpa.mic_failure.failure = 0;
-	priv->wpa.mic_failure.last_failure_time = 0;
-	priv->wpa.mic_failure.stop = 0;
 	memset(&priv->pmklist, 0, sizeof(priv->pmklist));
 	INIT_LIST_HEAD(&priv->pmklist.head);
 	for (i = 0; i < PMK_LIST_MAX; i++)
 		INIT_LIST_HEAD(&priv->pmklist.pmk[i].list);
+}
 
-	priv->sme_i.sme_status = SME_IDLE;
-	priv->sme_i.qhead = 0;
-	priv->sme_i.qtail = 0;
-	spin_lock_init(&priv->sme_i.sme_spin);
-	priv->sme_i.sme_flag = 0;
+static inline void hostif_counters_init(struct ks_wlan_private *priv)
+{
+	priv->dev_count = 0;
+	atomic_set(&priv->event_count, 0);
+	atomic_set(&priv->rec_count, 0);
+}
 
-	tasklet_init(&priv->sme_task, hostif_sme_task, (unsigned long)priv);
+int hostif_init(struct ks_wlan_private *priv)
+{
+	hostif_aplist_init(priv);
+	hostif_status_init(priv);
+
+	spin_lock_init(&priv->multicast_spin);
+	spin_lock_init(&priv->dev_read_lock);
+	init_waitqueue_head(&priv->devread_wait);
+
+	hostif_counters_init(priv);
+	hostif_power_save_init(priv);
+	hostif_wpa_init(priv);
+	hostif_pmklist_init(priv);
+	hostif_sme_init(priv);
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/6] staging: ks7010: remove some dead code from ks_wlan_set_essid function

[Greg KH]

On Mon, Mar 26, 2018 at 06:52:48PM +0200, Sergio Paracuellos wrote:
> This commit removes death code which is not being used at all because
> it is inside a preprocessor if 0 directive. This change improves a bit
> readability of ks_wlan_set_essid function.

That's not what your patch does:

> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/ks7010/ks_wlan_net.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
> index 6106e79..4019f3a 100644
> --- a/drivers/staging/ks7010/ks_wlan_net.c
> +++ b/drivers/staging/ks7010/ks_wlan_net.c
> @@ -276,7 +276,6 @@ static int ks_wlan_set_essid(struct net_device *dev,
>  		memset(priv->reg.ssid.body, 0, sizeof(priv->reg.ssid.body));
>  		priv->reg.ssid.size = 0;
>  	} else {
> -#if 1

It has a #if 1, not "0" :(

Being specific is good...

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel