[PATCH 2/4] Staging: ks7010: hostif: Fix multiple use of arguments in SME queue macros.

[PATCH 2/4] Staging: ks7010: hostif: Fix multiple use of arguments in SME queue macros.

[Quytelda Kahja]

Use GCC extensions to prevent macro arguments from accidentally being evaluated
multiple times when the macro is called.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks_hostif.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 975dbbb3abd0..30c9592b3a00 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -22,12 +22,19 @@
 #include <net/iw_handler.h>	/* New driver API */
 
 /* macro */
-#define inc_smeqhead(priv) \
-	(priv->sme_i.qhead = (priv->sme_i.qhead + 1) % SME_EVENT_BUFF_SIZE)
-#define inc_smeqtail(priv) \
-	(priv->sme_i.qtail = (priv->sme_i.qtail + 1) % SME_EVENT_BUFF_SIZE)
-#define cnt_smeqbody(priv) \
-	(((priv->sme_i.qtail + SME_EVENT_BUFF_SIZE) - (priv->sme_i.qhead)) % SME_EVENT_BUFF_SIZE)
+#define inc_smeqhead(priv)						\
+	({ typeof(priv) priv_ = (priv);					\
+		unsigned int next_qhead = priv_->sme_i.qhead + 1;	\
+		priv_->sme_i.qhead = next_qhead % SME_EVENT_BUFF_SIZE; })
+#define inc_smeqtail(priv)						\
+	({ typeof(priv) priv_ = (priv);					\
+		unsigned int next_qtail = priv_->sme_i.qtail + 1;	\
+		priv_->sme_i.qtail = next_qtail % SME_EVENT_BUFF_SIZE; })
+#define cnt_smeqbody(priv)						\
+	({ typeof(priv) priv_ = (priv);					\
+		unsigned int left_cnt =					\
+			priv_->sme_i.qtail + SME_EVENT_BUFF_SIZE;	\
+		 (left_cnt - (priv_->sme_i.qhead)) % SME_EVENT_BUFF_SIZE; })
 
 #define KS_WLAN_MEM_FLAG (GFP_ATOMIC)
 
-- 
2.16.1

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

[PATCH 3/4] Staging: ks7010: hostif: Fix multiple use of arguments in rate and event masking macros.

[Quytelda Kahja]

Use GCC extensions to prevent macro arguments from accidentally being evaluated
multiple times when the macro is called.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks_hostif.h | 74 +++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h
index 5bae8d468e23..750ac86cee77 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -599,19 +599,39 @@ struct hostif_mic_failure_confirm_t {
 #define TX_RATE_48M	(uint8_t)(480 / 5)
 #define TX_RATE_54M	(uint8_t)(540 / 5)
 
-#define IS_11B_RATE(A) (((A & RATE_MASK) == TX_RATE_1M) || ((A & RATE_MASK) == TX_RATE_2M) || \
-			((A & RATE_MASK) == TX_RATE_5M) || ((A & RATE_MASK) == TX_RATE_11M))
-
-#define IS_OFDM_RATE(A) (((A & RATE_MASK) == TX_RATE_6M) || ((A & RATE_MASK) == TX_RATE_12M) || \
-			 ((A & RATE_MASK) == TX_RATE_24M) || ((A & RATE_MASK) == TX_RATE_9M) || \
-			 ((A & RATE_MASK) == TX_RATE_18M) || ((A & RATE_MASK) == TX_RATE_36M) || \
-			 ((A & RATE_MASK) == TX_RATE_48M) || ((A & RATE_MASK) == TX_RATE_54M))
-
-#define IS_11BG_RATE(A) (IS_11B_RATE(A) || IS_OFDM_RATE(A))
-
-#define IS_OFDM_EXT_RATE(A) (((A & RATE_MASK) == TX_RATE_9M) || ((A & RATE_MASK) == TX_RATE_18M) || \
-			     ((A & RATE_MASK) == TX_RATE_36M) || ((A & RATE_MASK) == TX_RATE_48M) || \
-			     ((A & RATE_MASK) == TX_RATE_54M))
+#define IS_11B_RATE(A)							\
+	({								\
+		typeof(A) A_ = (A);					\
+		((A_ & RATE_MASK) == TX_RATE_1M) ||			\
+			((A_ & RATE_MASK) == TX_RATE_2M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_5M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_11M); })
+
+#define IS_OFDM_RATE(A)							\
+	({								\
+		typeof(A) A_ = (A);					\
+		((A_ & RATE_MASK) == TX_RATE_6M) ||			\
+			((A_ & RATE_MASK) == TX_RATE_12M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_24M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_9M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_18M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_36M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_48M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_54M); })
+
+#define IS_11BG_RATE(A)					\
+	({						\
+		typeof(A) A_ = (A);			\
+		IS_11B_RATE(A_) || IS_OFDM_RATE(A_); })
+
+#define IS_OFDM_EXT_RATE(A)						\
+	({								\
+		typeof(A) A_ = (A);					\
+		((A_ & RATE_MASK) == TX_RATE_9M) ||			\
+			((A_ & RATE_MASK) == TX_RATE_18M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_36M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_48M) ||		\
+			((A_ & RATE_MASK) == TX_RATE_54M); })
 
 enum connect_status_type {
 	CONNECT_STATUS,
@@ -633,17 +653,23 @@ enum multicast_filter_type {
 
 /* macro function */
 #define HIF_EVENT_MASK 0xE800
-#define IS_HIF_IND(_EVENT)  ((_EVENT & HIF_EVENT_MASK) == 0xE800  && \
-			     ((_EVENT & ~HIF_EVENT_MASK) == 0x0001 || \
-			     (_EVENT & ~HIF_EVENT_MASK) == 0x0006 || \
-			     (_EVENT & ~HIF_EVENT_MASK) == 0x000C || \
-			     (_EVENT & ~HIF_EVENT_MASK) == 0x0011 || \
-			     (_EVENT & ~HIF_EVENT_MASK) == 0x0012))
-
-#define IS_HIF_CONF(_EVENT) ((_EVENT & HIF_EVENT_MASK) == 0xE800  && \
-			     (_EVENT & ~HIF_EVENT_MASK) > 0x0000  && \
-			     (_EVENT & ~HIF_EVENT_MASK) < 0x0012  && \
-			     !IS_HIF_IND(_EVENT))
+#define IS_HIF_IND(_EVENT)					      \
+	({							      \
+		typeof(_EVENT) EVENT_ = (_EVENT);		      \
+		(EVENT_ & HIF_EVENT_MASK) == 0xE800  &&		      \
+			((EVENT_ & ~HIF_EVENT_MASK) == 0x0001 ||      \
+			 (EVENT_ & ~HIF_EVENT_MASK) == 0x0006 ||      \
+			 (EVENT_ & ~HIF_EVENT_MASK) == 0x000C ||      \
+			 (EVENT_ & ~HIF_EVENT_MASK) == 0x0011 ||      \
+			 (EVENT_ & ~HIF_EVENT_MASK) == 0x0012); })
+
+#define IS_HIF_CONF(_EVENT)					     \
+	({							     \
+		typeof(_EVENT) EVENT_ = (_EVENT);		     \
+		(EVENT_ & HIF_EVENT_MASK) == 0xE800  &&		     \
+			(EVENT_ & ~HIF_EVENT_MASK) > 0x0000  &&	     \
+			(EVENT_ & ~HIF_EVENT_MASK) < 0x0012  &&	     \
+			!IS_HIF_IND(EVENT_); })
 
 #ifdef __KERNEL__
 
-- 
2.16.1

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

Re: [PATCH 3/4] Staging: ks7010: hostif: Fix multiple use of arguments in rate and event masking macros.

[Greg KH]

On Fri, Feb 16, 2018 at 06:25:02PM -0800, Quytelda Kahja wrote:
> Use GCC extensions to prevent macro arguments from accidentally being evaluated
> multiple times when the macro is called.
> 
> Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
> ---
>  drivers/staging/ks7010/ks_hostif.h | 74 +++++++++++++++++++++++++-------------
>  1 file changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h
> index 5bae8d468e23..750ac86cee77 100644
> --- a/drivers/staging/ks7010/ks_hostif.h
> +++ b/drivers/staging/ks7010/ks_hostif.h
> @@ -599,19 +599,39 @@ struct hostif_mic_failure_confirm_t {
>  #define TX_RATE_48M	(uint8_t)(480 / 5)
>  #define TX_RATE_54M	(uint8_t)(540 / 5)
>  
> -#define IS_11B_RATE(A) (((A & RATE_MASK) == TX_RATE_1M) || ((A & RATE_MASK) == TX_RATE_2M) || \
> -			((A & RATE_MASK) == TX_RATE_5M) || ((A & RATE_MASK) == TX_RATE_11M))
> -
> -#define IS_OFDM_RATE(A) (((A & RATE_MASK) == TX_RATE_6M) || ((A & RATE_MASK) == TX_RATE_12M) || \
> -			 ((A & RATE_MASK) == TX_RATE_24M) || ((A & RATE_MASK) == TX_RATE_9M) || \
> -			 ((A & RATE_MASK) == TX_RATE_18M) || ((A & RATE_MASK) == TX_RATE_36M) || \
> -			 ((A & RATE_MASK) == TX_RATE_48M) || ((A & RATE_MASK) == TX_RATE_54M))
> -
> -#define IS_11BG_RATE(A) (IS_11B_RATE(A) || IS_OFDM_RATE(A))
> -
> -#define IS_OFDM_EXT_RATE(A) (((A & RATE_MASK) == TX_RATE_9M) || ((A & RATE_MASK) == TX_RATE_18M) || \
> -			     ((A & RATE_MASK) == TX_RATE_36M) || ((A & RATE_MASK) == TX_RATE_48M) || \
> -			     ((A & RATE_MASK) == TX_RATE_54M))
> +#define IS_11B_RATE(A)							\
> +	({								\
> +		typeof(A) A_ = (A);					\
> +		((A_ & RATE_MASK) == TX_RATE_1M) ||			\
> +			((A_ & RATE_MASK) == TX_RATE_2M) ||		\
> +			((A_ & RATE_MASK) == TX_RATE_5M) ||		\
> +			((A_ & RATE_MASK) == TX_RATE_11M); })
> +

Again, please just make these real functions, if they are actually used.

thanks,

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

[PATCH 4/4] Staging: ks7010: hostif: Fix multiple use of arguments in ps_confirm_wait_inc() macro.

[Quytelda Kahja]

Use GCC extensions to prevent macro arguments from accidentally being evaluated
multiple times when the macro is called.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks_hostif.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 30c9592b3a00..92035e8ac843 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1306,11 +1306,10 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb)
 	return ret;
 }
 
-#define ps_confirm_wait_inc(priv)					 \
-	do {								 \
-		if (atomic_read(&priv->psstatus.status) > PS_ACTIVE_SET) \
-			atomic_inc(&priv->psstatus.confirm_wait);	 \
-	} while (0)
+#define ps_confirm_wait_inc(priv)					\
+	({ typeof(priv) priv_ = (priv);					\
+		if (atomic_read(&priv_->psstatus.status) > PS_ACTIVE_SET) \
+			atomic_inc(&priv_->psstatus.confirm_wait); })
 
 static
 void hostif_mib_get_request(struct ks_wlan_private *priv,
-- 
2.16.1

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

Re: [PATCH 2/4] Staging: ks7010: hostif: Fix multiple use of arguments in SME queue macros.

[Greg KH]

On Fri, Feb 16, 2018 at 06:25:01PM -0800, Quytelda Kahja wrote:
> Use GCC extensions to prevent macro arguments from accidentally being evaluated
> multiple times when the macro is called.
> 
> Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
> ---
>  drivers/staging/ks7010/ks_hostif.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> index 975dbbb3abd0..30c9592b3a00 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -22,12 +22,19 @@
>  #include <net/iw_handler.h>	/* New driver API */
>  
>  /* macro */
> -#define inc_smeqhead(priv) \
> -	(priv->sme_i.qhead = (priv->sme_i.qhead + 1) % SME_EVENT_BUFF_SIZE)
> -#define inc_smeqtail(priv) \
> -	(priv->sme_i.qtail = (priv->sme_i.qtail + 1) % SME_EVENT_BUFF_SIZE)
> -#define cnt_smeqbody(priv) \
> -	(((priv->sme_i.qtail + SME_EVENT_BUFF_SIZE) - (priv->sme_i.qhead)) % SME_EVENT_BUFF_SIZE)
> +#define inc_smeqhead(priv)						\
> +	({ typeof(priv) priv_ = (priv);					\
> +		unsigned int next_qhead = priv_->sme_i.qhead + 1;	\
> +		priv_->sme_i.qhead = next_qhead % SME_EVENT_BUFF_SIZE; })

No need, just make these real functions please.

thanks,

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

[PATCH v2 1/3] Staging: ks7010: sdio: Convert RX/TX queue macros into real functions.

[Quytelda Kahja]

Convert the unsafe macros into inline functions.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks7010_sdio.c | 46 ++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 8cfdff198334..7de78d1758b8 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -32,19 +32,39 @@ static const struct sdio_device_id ks7010_sdio_ids[] = {
 };
 MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
 
-#define inc_txqhead(priv) \
-	(priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
-#define inc_txqtail(priv) \
-	(priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
-#define cnt_txqbody(priv) \
-	(((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE)
-
-#define inc_rxqhead(priv) \
-	(priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
-#define inc_rxqtail(priv) \
-	(priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
-#define cnt_rxqbody(priv) \
-	(((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE)
+static inline void inc_txqhead(struct ks_wlan_private *priv)
+{
+	priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE;
+}
+
+static inline void inc_txqtail(struct ks_wlan_private *priv)
+{
+	priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE;
+}
+
+static inline unsigned int cnt_txqbody(struct ks_wlan_private *priv)
+{
+	unsigned int tx_cnt = priv->tx_dev.qtail - priv->tx_dev.qhead;
+
+	return (tx_cnt + TX_DEVICE_BUFF_SIZE) % TX_DEVICE_BUFF_SIZE;
+}
+
+static inline void inc_rxqhead(struct ks_wlan_private *priv)
+{
+	priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE;
+}
+
+static inline void inc_rxqtail(struct ks_wlan_private *priv)
+{
+	priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE;
+}
+
+static inline unsigned int cnt_rxqbody(struct ks_wlan_private *priv)
+{
+	unsigned int rx_cnt = priv->rx_dev.qtail - priv->rx_dev.qhead;
+
+	return (rx_cnt + RX_DEVICE_BUFF_SIZE) % RX_DEVICE_BUFF_SIZE;
+}
 
 /* Read single byte from device address into byte (CMD52) */
 static int ks7010_sdio_readb(struct ks_wlan_private *priv, unsigned int address,
-- 
2.16.2

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

[PATCH v2 2/3] Staging: ks7010: hostif: Convert SME queue macros to real functions.

[Quytelda Kahja]

Convert the unsafe macros into inline functions.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks_hostif.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 975dbbb3abd0..f554477fe6bc 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -21,13 +21,22 @@
 /* Include Wireless Extension definition and check version */
 #include <net/iw_handler.h>	/* New driver API */
 
-/* macro */
-#define inc_smeqhead(priv) \
-	(priv->sme_i.qhead = (priv->sme_i.qhead + 1) % SME_EVENT_BUFF_SIZE)
-#define inc_smeqtail(priv) \
-	(priv->sme_i.qtail = (priv->sme_i.qtail + 1) % SME_EVENT_BUFF_SIZE)
-#define cnt_smeqbody(priv) \
-	(((priv->sme_i.qtail + SME_EVENT_BUFF_SIZE) - (priv->sme_i.qhead)) % SME_EVENT_BUFF_SIZE)
+static inline void inc_smeqhead(struct ks_wlan_private *priv)
+{
+	priv->sme_i.qhead = (priv->sme_i.qhead + 1) % SME_EVENT_BUFF_SIZE;
+}
+
+static inline void inc_smeqtail(struct ks_wlan_private *priv)
+{
+	priv->sme_i.qtail = (priv->sme_i.qtail + 1) % SME_EVENT_BUFF_SIZE;
+}
+
+static inline unsigned int cnt_smeqbody(struct ks_wlan_private *priv)
+{
+	unsigned int sme_cnt = priv->sme_i.qtail - priv->sme_i.qhead;
+
+	return (sme_cnt + SME_EVENT_BUFF_SIZE) % SME_EVENT_BUFF_SIZE;
+}
 
 #define KS_WLAN_MEM_FLAG (GFP_ATOMIC)
 
-- 
2.16.2

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

[PATCH v2 3/3] Staging: ks7010: hostif: Convert the ps_confirm_wait_inc() macro to a real function.

[Quytelda Kahja]

Convert the unsafe macro into an inline function.

Signed-off-by: Quytelda Kahja <quytelda@tamalin.org>
---
 drivers/staging/ks7010/ks_hostif.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index f554477fe6bc..74a08417bd0b 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1308,11 +1308,11 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb)
 	return ret;
 }
 
-#define ps_confirm_wait_inc(priv)					 \
-	do {								 \
-		if (atomic_read(&priv->psstatus.status) > PS_ACTIVE_SET) \
-			atomic_inc(&priv->psstatus.confirm_wait);	 \
-	} while (0)
+static inline void ps_confirm_wait_inc(struct ks_wlan_private *priv)
+{
+	if (atomic_read(&priv->psstatus.status) > PS_ACTIVE_SET)
+		atomic_inc(&priv->psstatus.confirm_wait);
+}
 
 static
 void hostif_mib_get_request(struct ks_wlan_private *priv,
-- 
2.16.2

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