[PATCH v1 0/5] staging: wfx: fix checkpatch warnings

[PATCH v1 0/5] staging: wfx: fix checkpatch warnings

[Jules Irenge]

Fix checkpatch warnings.

Jules Irenge (5):
  staging: wfx: fix warnings of no space is necessary
  staging: wfx: fix warning of line over 80 characters
  staging: wfx: fix warnings of logical continuation
  staging: wfx: correct misspelled words
  staging: wfx: fix warnings of alignment should match open parenthesis

 drivers/staging/wfx/bh.c       |  25 ++++---
 drivers/staging/wfx/bus.h      |   6 +-
 drivers/staging/wfx/bus_sdio.c |   9 +--
 drivers/staging/wfx/bus_spi.c  |  11 +--
 drivers/staging/wfx/data_rx.c  |  35 +++++----
 drivers/staging/wfx/data_tx.c  | 127 +++++++++++++++++++++------------
 drivers/staging/wfx/data_tx.h  |   4 +-
 drivers/staging/wfx/debug.c    |  14 ++--
 8 files changed, 143 insertions(+), 88 deletions(-)

-- 
2.21.0

[PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Jules Irenge]

Fix warnings of no space is necessary after a cast.
Issue detected by checkpatch tool.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/wfx/bh.c       |  8 ++++----
 drivers/staging/wfx/bus_sdio.c |  6 +++---
 drivers/staging/wfx/bus_spi.c  |  2 +-
 drivers/staging/wfx/data_rx.c  |  8 ++++----
 drivers/staging/wfx/data_tx.c  | 20 ++++++++++----------
 drivers/staging/wfx/data_tx.h  |  4 ++--
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 3355183fc86c..573216b08042 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -69,13 +69,13 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	if (wfx_data_read(wdev, skb->data, alloc_len))
 		goto err;
 
-	piggyback = le16_to_cpup((u16 *) (skb->data + alloc_len - 2));
+	piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2));
 	_trace_piggyback(piggyback, false);
 
-	hif = (struct hif_msg *) skb->data;
+	hif = (struct hif_msg *)skb->data;
 	WARN(hif->encrypted & 0x1, "unsupported encryption type");
 	if (hif->encrypted == 0x2) {
-		if (wfx_sl_decode(wdev, (void *) hif)) {
+		if (wfx_sl_decode(wdev, (void *)hif)) {
 			dev_kfree_skb(skb);
 			// If frame was a confirmation, expect trouble in next
 			// exchange. However, it is harmless to fail to decode
@@ -102,7 +102,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	if (!(hif->id & HIF_ID_IS_INDICATION)) {
 		(*is_cnf)++;
 		if (hif->id == HIF_CNF_ID_MULTI_TRANSMIT)
-			release_count = le32_to_cpu(((struct hif_cnf_multi_transmit *) hif->body)->num_tx_confs);
+			release_count = le32_to_cpu(((struct hif_cnf_multi_transmit *)hif->body)->num_tx_confs);
 		else
 			release_count = 1;
 		WARN(wdev->hif.tx_buffers_used < release_count, "corrupted buffer counter");
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index f97360513150..184e20dfdd62 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -38,7 +38,7 @@ static int wfx_sdio_copy_from_io(void *priv, unsigned int reg_id,
 	int ret;
 
 	WARN(reg_id > 7, "chip only has 7 registers");
-	WARN(((uintptr_t) dst) & 3, "unaligned buffer size");
+	WARN(((uintptr_t)dst) & 3, "unaligned buffer size");
 	WARN(count & 3, "unaligned buffer address");
 
 	/* Use queue mode buffers */
@@ -59,14 +59,14 @@ static int wfx_sdio_copy_to_io(void *priv, unsigned int reg_id,
 	int ret;
 
 	WARN(reg_id > 7, "chip only has 7 registers");
-	WARN(((uintptr_t) src) & 3, "unaligned buffer size");
+	WARN(((uintptr_t)src) & 3, "unaligned buffer size");
 	WARN(count & 3, "unaligned buffer address");
 
 	/* Use queue mode buffers */
 	if (reg_id == WFX_REG_IN_OUT_QUEUE)
 		sdio_addr |= bus->buf_id_tx << 7;
 	// FIXME: discards 'const' qualifier for src
-	ret = sdio_memcpy_toio(bus->func, sdio_addr, (void *) src, count);
+	ret = sdio_memcpy_toio(bus->func, sdio_addr, (void *)src, count);
 	if (!ret && reg_id == WFX_REG_IN_OUT_QUEUE)
 		bus->buf_id_tx = (bus->buf_id_tx + 1) % 32;
 
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index f65f7d75e731..effd07957753 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -90,7 +90,7 @@ static int wfx_spi_copy_to_io(void *priv, unsigned int addr,
 	struct wfx_spi_priv *bus = priv;
 	u16 regaddr = (addr << 12) | (count / 2);
 	// FIXME: use a bounce buffer
-	u16 *src16 = (void *) src;
+	u16 *src16 = (void *)src;
 	int ret, i;
 	struct spi_message      m;
 	struct spi_transfer     t_addr = {
diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 3a79089c8501..3a79ab93e97e 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -29,7 +29,7 @@ static int wfx_handle_pspoll(struct wfx_vif *wvif, struct sk_buff *skb)
 	rcu_read_lock();
 	sta = ieee80211_find_sta(wvif->vif, pspoll->ta);
 	if (sta)
-		link_id = ((struct wfx_sta_priv *) &sta->drv_priv)->link_id;
+		link_id = ((struct wfx_sta_priv *)&sta->drv_priv)->link_id;
 	rcu_read_unlock();
 	if (link_id)
 		pspoll_mask = BIT(link_id);
@@ -102,8 +102,8 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 {
 	int link_id = arg->rx_flags.peer_sta_id;
 	struct ieee80211_rx_status *hdr = IEEE80211_SKB_RXCB(skb);
-	struct ieee80211_hdr *frame = (struct ieee80211_hdr *) skb->data;
-	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *) skb->data;
+	struct ieee80211_hdr *frame = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
 	struct wfx_link_entry *entry = NULL;
 	bool early_data = false;
 
@@ -173,7 +173,7 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 
 		tim_ie = cfg80211_find_ie(WLAN_EID_TIM, ies, ies_len);
 		if (tim_ie) {
-			struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *) &tim_ie[2];
+			struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *)&tim_ie[2];
 
 			if (wvif->dtim_period != tim->dtim_period) {
 				wvif->dtim_period = tim->dtim_period;
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 8ed38cac19f6..cf73b83ccc9e 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -427,7 +427,7 @@ void wfx_link_id_work(struct work_struct *work)
 
 static bool ieee80211_is_action_back(struct ieee80211_hdr *hdr)
 {
-	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *) hdr;
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)hdr;
 
 	if (!ieee80211_is_action(mgmt->frame_control))
 		return false;
@@ -591,7 +591,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	struct wfx_tx_priv *tx_priv;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int queue_id = tx_info->hw_queue;
 	size_t offset = (size_t) skb->data & 3;
 	int wmsg_len = sizeof(struct hif_msg) + sizeof(struct hif_req_tx) + offset;
@@ -602,7 +602,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	// From now tx_info->control is unusable
 	memset(tx_info->rate_driver_data, 0, sizeof(struct wfx_tx_priv));
 	// Fill tx_priv
-	tx_priv = (struct wfx_tx_priv *) tx_info->rate_driver_data;
+	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
 	tx_priv->tid = wfx_tx_get_tid(hdr);
 	tx_priv->raw_link_id = wfx_tx_get_raw_link_id(wvif, sta, hdr);
 	tx_priv->link_id = tx_priv->raw_link_id;
@@ -619,7 +619,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	skb_put(skb, wfx_tx_get_icv_len(tx_priv->hw_key));
 	skb_push(skb, wmsg_len);
 	memset(skb->data, 0, wmsg_len);
-	hif_msg = (struct hif_msg *) skb->data;
+	hif_msg = (struct hif_msg *)skb->data;
 	hif_msg->len = cpu_to_le16(skb->len);
 	hif_msg->id = HIF_REQ_ID_TX;
 	hif_msg->interface = wvif->id;
@@ -631,7 +631,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	}
 
 	// Fill tx request
-	req = (struct hif_req_tx *) hif_msg->body;
+	req = (struct hif_req_tx *)hif_msg->body;
 	req->packet_id = queue_id << 16 | IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 	req->data_flags.fc_offset = offset;
 	req->queue_id.peer_sta_id = tx_priv->raw_link_id;
@@ -654,7 +654,7 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	struct wfx_vif *wvif;
 	struct ieee80211_sta *sta = control ? control->sta : NULL;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	size_t driver_data_room = FIELD_SIZEOF(struct ieee80211_tx_info, rate_driver_data);
 
 	compiletime_assert(sizeof(struct wfx_tx_priv) <= driver_data_room,
@@ -662,7 +662,7 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	WARN(skb->next || skb->prev, "skb is already member of a list");
 	// control.vif can be NULL for injected frames
 	if (tx_info->control.vif)
-		wvif = (struct wfx_vif *) tx_info->control.vif->drv_priv;
+		wvif = (struct wfx_vif *)tx_info->control.vif->drv_priv;
 	else
 		wvif = wvif_iterate(wdev, NULL);
 	if (WARN_ON(!wvif))
@@ -762,7 +762,7 @@ static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb,
 				   struct hif_req_tx *req)
 {
 	struct ieee80211_sta *sta;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int tid = wfx_tx_get_tid(hdr);
 	int raw_link_id = req->queue_id.peer_sta_id;
 	u8 *buffered;
@@ -787,8 +787,8 @@ static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb,
 
 void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
 {
-	struct hif_msg *hif = (struct hif_msg *) skb->data;
-	struct hif_req_tx *req = (struct hif_req_tx *) hif->body;
+	struct hif_msg *hif = (struct hif_msg *)skb->data;
+	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
 	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 	unsigned int offset = sizeof(struct hif_req_tx) + sizeof(struct hif_msg) + req->data_flags.fc_offset;
 
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index 0a19ef10a4ab..f74d1988925d 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -79,12 +79,12 @@ static inline struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb)
 	if (!skb)
 		return NULL;
 	tx_info = IEEE80211_SKB_CB(skb);
-	return (struct wfx_tx_priv *) tx_info->rate_driver_data;
+	return (struct wfx_tx_priv *)tx_info->rate_driver_data;
 }
 
 static inline struct hif_req_tx *wfx_skb_txreq(struct sk_buff *skb)
 {
-	struct hif_msg *hif = (struct hif_msg *) skb->data;
+	struct hif_msg *hif = (struct hif_msg *)skb->data;
 	struct hif_req_tx *req = (struct hif_req_tx *) hif->body;
 
 	return req;
-- 
2.21.0

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Dan Carpenter]

On Sat, Oct 19, 2019 at 03:07:15PM +0100, Jules Irenge wrote:
> diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
> index 3355183fc86c..573216b08042 100644
> --- a/drivers/staging/wfx/bh.c
> +++ b/drivers/staging/wfx/bh.c
> @@ -69,13 +69,13 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
>  	if (wfx_data_read(wdev, skb->data, alloc_len))
>  		goto err;
>  
> -	piggyback = le16_to_cpup((u16 *) (skb->data + alloc_len - 2));
> +	piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2));
>  	_trace_piggyback(piggyback, false);
>  
> -	hif = (struct hif_msg *) skb->data;
> +	hif = (struct hif_msg *)skb->data;
>  	WARN(hif->encrypted & 0x1, "unsupported encryption type");
>  	if (hif->encrypted == 0x2) {
> -		if (wfx_sl_decode(wdev, (void *) hif)) {
> +		if (wfx_sl_decode(wdev, (void *)hif)) {

In the future you may want to go through and remove the (void *) casts.
It's not required here.

> diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> index f65f7d75e731..effd07957753 100644
> --- a/drivers/staging/wfx/bus_spi.c
> +++ b/drivers/staging/wfx/bus_spi.c
> @@ -90,7 +90,7 @@ static int wfx_spi_copy_to_io(void *priv, unsigned int addr,
>  	struct wfx_spi_priv *bus = priv;
>  	u16 regaddr = (addr << 12) | (count / 2);
>  	// FIXME: use a bounce buffer
> -	u16 *src16 = (void *) src;
> +	u16 *src16 = (void *)src;

Here we are just getting rid of the constness.  Apparently we are doing
that so we can modify it without GCC pointing out the bug!!  I don't
know the code but this seems very wrong.

>  	int ret, i;
>  	struct spi_message      m;
>  	struct spi_transfer     t_addr = {

regards,
dan carpenter

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Jules Irenge]

On Sat, 19 Oct 2019, Dan Carpenter wrote:

> On Sat, Oct 19, 2019 at 03:07:15PM +0100, Jules Irenge wrote:
> > diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
> > index 3355183fc86c..573216b08042 100644
> > --- a/drivers/staging/wfx/bh.c
> > +++ b/drivers/staging/wfx/bh.c
> > @@ -69,13 +69,13 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
> >  	if (wfx_data_read(wdev, skb->data, alloc_len))
> >  		goto err;
> >  
> > -	piggyback = le16_to_cpup((u16 *) (skb->data + alloc_len - 2));
> > +	piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2));
> >  	_trace_piggyback(piggyback, false);
> >  
> > -	hif = (struct hif_msg *) skb->data;
> > +	hif = (struct hif_msg *)skb->data;
> >  	WARN(hif->encrypted & 0x1, "unsupported encryption type");
> >  	if (hif->encrypted == 0x2) {
> > -		if (wfx_sl_decode(wdev, (void *) hif)) {
> > +		if (wfx_sl_decode(wdev, (void *)hif)) {
> 
> In the future you may want to go through and remove the (void *) casts.
> It's not required here.
> 
> > diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> > index f65f7d75e731..effd07957753 100644
> > --- a/drivers/staging/wfx/bus_spi.c
> > +++ b/drivers/staging/wfx/bus_spi.c
> > @@ -90,7 +90,7 @@ static int wfx_spi_copy_to_io(void *priv, unsigned int addr,
> >  	struct wfx_spi_priv *bus = priv;
> >  	u16 regaddr = (addr << 12) | (count / 2);
> >  	// FIXME: use a bounce buffer
> > -	u16 *src16 = (void *) src;
> > +	u16 *src16 = (void *)src;
> 
> Here we are just getting rid of the constness.  Apparently we are doing
> that so we can modify it without GCC pointing out the bug!!  I don't
> know the code but this seems very wrong.
> 
Checkpatch was complaining about  space between type cast and the 
variable. I just get rid of the space. Well I don't know whether this was 
false positive one.

Thanks for the feedback. I will update the patch.

Regards,
Jules

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

On Sat, 19 Oct 2019, Jules Irenge wrote:

>
>
> On Sat, 19 Oct 2019, Dan Carpenter wrote:
>
> > On Sat, Oct 19, 2019 at 03:07:15PM +0100, Jules Irenge wrote:
> > > diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
> > > index 3355183fc86c..573216b08042 100644
> > > --- a/drivers/staging/wfx/bh.c
> > > +++ b/drivers/staging/wfx/bh.c
> > > @@ -69,13 +69,13 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
> > >  	if (wfx_data_read(wdev, skb->data, alloc_len))
> > >  		goto err;
> > >
> > > -	piggyback = le16_to_cpup((u16 *) (skb->data + alloc_len - 2));
> > > +	piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2));
> > >  	_trace_piggyback(piggyback, false);
> > >
> > > -	hif = (struct hif_msg *) skb->data;
> > > +	hif = (struct hif_msg *)skb->data;
> > >  	WARN(hif->encrypted & 0x1, "unsupported encryption type");
> > >  	if (hif->encrypted == 0x2) {
> > > -		if (wfx_sl_decode(wdev, (void *) hif)) {
> > > +		if (wfx_sl_decode(wdev, (void *)hif)) {
> >
> > In the future you may want to go through and remove the (void *) casts.
> > It's not required here.
> >
> > > diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> > > index f65f7d75e731..effd07957753 100644
> > > --- a/drivers/staging/wfx/bus_spi.c
> > > +++ b/drivers/staging/wfx/bus_spi.c
> > > @@ -90,7 +90,7 @@ static int wfx_spi_copy_to_io(void *priv, unsigned int addr,
> > >  	struct wfx_spi_priv *bus = priv;
> > >  	u16 regaddr = (addr << 12) | (count / 2);
> > >  	// FIXME: use a bounce buffer
> > > -	u16 *src16 = (void *) src;
> > > +	u16 *src16 = (void *)src;
> >
> > Here we are just getting rid of the constness.  Apparently we are doing
> > that so we can modify it without GCC pointing out the bug!!  I don't
> > know the code but this seems very wrong.
> >
> Checkpatch was complaining about  space between type cast and the
> variable. I just get rid of the space. Well I don't know whether this was
> false positive one.

I think you missed the point.  It would be good to trace through the core
and try to figure out where this src value comes from.  Is it really
const?  Or is the const declaration there just to satisfy the type
checker, and is the actual data provided not const.  This function is
stored in a hwbus_ops structure.  It would be good to see what other
drivers that store a function in the same field of such a structure do,
and to see where the function is actually called (via a function pointer)
and where the argument comes from.

julia

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Dan Carpenter]

On Sat, Oct 19, 2019 at 04:09:03PM +0100, Jules Irenge wrote:
> Checkpatch was complaining about  space between type cast and the 
> variable. I just get rid of the space. Well I don't know whether this was 
> false positive one.
> 
> Thanks for the feedback. I will update the patch.

No no.  The patch is fine.  I was saying that in the *future* you might
want to look at the void casts.  Some of them are not required and
others are buggy code.

regards,
dan carpenter

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Joe Perches]

On Sat, 2019-10-19 at 21:05 +0300, Dan Carpenter wrote:
> On Sat, Oct 19, 2019 at 04:09:03PM +0100, Jules Irenge wrote:
> > Checkpatch was complaining about  space between type cast and the 
> > variable. I just get rid of the space. Well I don't know whether this was 
> > false positive one.
> > 
> > Thanks for the feedback. I will update the patch.
> 
> No no.  The patch is fine.  I was saying that in the *future* you might
> want to look at the void casts.  Some of them are not required and
> others are buggy code.

Hello Jules.

Frequently, checkpatch is not the best tool to find and
possibly correct various unnecessary code styles.

checkpatch is OK to look at some odd uses

coccinelle is frequently a better overall tool for these uses.

A trivial cocci script to remove some unnecessary void * casts
from mem<foo> and str<foo> calls might be like the below, but
watch out for __iomem and __force uses where coccinelle doesn't
understand the syntax.

$ cat unnecessary_void_ptr.cocci
@@
type T1;
type T2;
T1 *p1;
T2 *p2;
@@

	\(memcpy\|memmove\|strcpy\|strncmp\)(
-	(void *)
	p1,
-	(void *)
	p2, ...)

@@
type T1;
type T2;
T1 *p1;
T2 *p2;
@@

	\(memcpy\|memmove\|strcpy\|strncmp\)(
	p1,
-	(void *)
	p2, ...)

@@
type T;
T *p;
@@

	\(memcpy\|memset\|memmove\|memzero\|strcpy\|strncmp\|strnlen\)(
-	(void *)
	p, ...)

$

For instance, when run over today's drivers/staging, this produces:

$ spatch --very-quiet -sp-file unnecessary_void_ptr.cocci drivers/staging
---
diff -u -p a/exfat/exfat_core.c b/exfat/exfat_core.c
--- a/exfat/exfat_core.c
+++ b/exfat/exfat_core.c
@@ -3427,7 +3427,7 @@ s32 rename_file(struct inode *inode, str
 			return FFS_MEDIAERR;
 		}
 
-		memcpy((void *)epnew, (void *)epold, DENTRY_SIZE);
+		memcpy(epnew, epold, DENTRY_SIZE);
 		if (fs_func->get_entry_type(epnew) == TYPE_FILE) {
 			fs_func->set_entry_attr(epnew,
 						fs_func->get_entry_attr(epnew) |
@@ -3449,7 +3449,7 @@ s32 rename_file(struct inode *inode, str
 				return FFS_MEDIAERR;
 			}
 
-			memcpy((void *)epnew, (void *)epold, DENTRY_SIZE);
+			memcpy(epnew, epold, DENTRY_SIZE);
 			buf_modify(sb, sector_new);
 			buf_unlock(sb, sector_old);
 		}
@@ -3538,7 +3538,7 @@ s32 move_file(struct inode *inode, struc
 		return FFS_MEDIAERR;
 	}
 
-	memcpy((void *)epnew, (void *)epmov, DENTRY_SIZE);
+	memcpy(epnew, epmov, DENTRY_SIZE);
 	if (fs_func->get_entry_type(epnew) == TYPE_FILE) {
 		fs_func->set_entry_attr(epnew, fs_func->get_entry_attr(epnew) |
 					ATTR_ARCHIVE);
@@ -3558,7 +3558,7 @@ s32 move_file(struct inode *inode, struc
 			return FFS_MEDIAERR;
 		}
 
-		memcpy((void *)epnew, (void *)epmov, DENTRY_SIZE);
+		memcpy(epnew, epmov, DENTRY_SIZE);
 		buf_modify(sb, sector_new);
 		buf_unlock(sb, sector_mov);
 	} else if (fs_func->get_entry_type(epnew) == TYPE_DIR) {
diff -u -p a/qlge/qlge_main.c b/qlge/qlge_main.c
--- a/qlge/qlge_main.c
+++ b/qlge/qlge_main.c
@@ -2583,7 +2583,7 @@ static netdev_tx_t qlge_send(struct sk_b
 	}
 	tx_ring_desc = &tx_ring->q[tx_ring->prod_idx];
 	mac_iocb_ptr = tx_ring_desc->queue_entry;
-	memset((void *)mac_iocb_ptr, 0, sizeof(*mac_iocb_ptr));
+	memset(mac_iocb_ptr, 0, sizeof(*mac_iocb_ptr));
 
 	mac_iocb_ptr->opcode = OPCODE_OB_MAC_IOCB;
 	mac_iocb_ptr->tid = tx_ring_desc->index;
@@ -3029,7 +3029,7 @@ static int ql_start_rx_ring(struct ql_ad
 	/* PCI doorbell mem area + 0x1c */
 	rx_ring->sbq.prod_idx_db_reg = (u32 __iomem *)(doorbell_area + 0x1c);
 
-	memset((void *)cqicb, 0, sizeof(struct cqicb));
+	memset(cqicb, 0, sizeof(struct cqicb));
 	cqicb->msix_vect = rx_ring->irq;
 
 	cqicb->len = cpu_to_le16(QLGE_FIT16(rx_ring->cq_len) | LEN_V |
@@ -3473,7 +3473,7 @@ static int ql_start_rss(struct ql_adapte
 	int i;
 	u8 *hash_id = (u8 *) ricb->hash_cq_id;
 
-	memset((void *)ricb, 0, sizeof(*ricb));
+	memset(ricb, 0, sizeof(*ricb));
 
 	ricb->base_cq = RSS_L4K;
 	ricb->flags =
@@ -3486,8 +3486,8 @@ static int ql_start_rss(struct ql_adapte
 	for (i = 0; i < 1024; i++)
 		hash_id[i] = (i & (qdev->rss_ring_count - 1));
 
-	memcpy((void *)&ricb->ipv6_hash_key[0], init_hash_seed, 40);
-	memcpy((void *)&ricb->ipv4_hash_key[0], init_hash_seed, 16);
+	memcpy(&ricb->ipv6_hash_key[0], init_hash_seed, 40);
+	memcpy(&ricb->ipv4_hash_key[0], init_hash_seed, 16);
 
 	status = ql_write_cfg(qdev, ricb, sizeof(*ricb), CFG_LR, 0);
 	if (status) {
@@ -3983,7 +3983,7 @@ static int ql_configure_rings(struct ql_
 
 	for (i = 0; i < qdev->tx_ring_count; i++) {
 		tx_ring = &qdev->tx_ring[i];
-		memset((void *)tx_ring, 0, sizeof(*tx_ring));
+		memset(tx_ring, 0, sizeof(*tx_ring));
 		tx_ring->qdev = qdev;
 		tx_ring->wq_id = i;
 		tx_ring->wq_len = qdev->tx_ring_size;
@@ -3999,7 +3999,7 @@ static int ql_configure_rings(struct ql_
 
 	for (i = 0; i < qdev->rx_ring_count; i++) {
 		rx_ring = &qdev->rx_ring[i];
-		memset((void *)rx_ring, 0, sizeof(*rx_ring));
+		memset(rx_ring, 0, sizeof(*rx_ring));
 		rx_ring->qdev = qdev;
 		rx_ring->cq_id = i;
 		rx_ring->cpu = i % cpu_cnt;	/* CPU to run handler on. */
@@ -4414,7 +4414,7 @@ static int ql_init_device(struct pci_dev
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	int err = 0;
 
-	memset((void *)qdev, 0, sizeof(*qdev));
+	memset(qdev, 0, sizeof(*qdev));
 	err = pci_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "PCI device enable failed.\n");
diff -u -p a/rtl8188eu/core/rtw_ieee80211.c b/rtl8188eu/core/rtw_ieee80211.c
--- a/rtl8188eu/core/rtw_ieee80211.c
+++ b/rtl8188eu/core/rtw_ieee80211.c
@@ -132,7 +132,7 @@ u8 *rtw_set_ie
 	*(pbuf + 1) = (u8)len;
 
 	if (len > 0)
-		memcpy((void *)(pbuf + 2), (void *)source, len);
+		memcpy((pbuf + 2), source, len);
 
 	*frlen = *frlen + (len + 2);
 
diff -u -p a/rtl8188eu/core/rtw_mlme_ext.c b/rtl8188eu/core/rtw_mlme_ext.c
--- a/rtl8188eu/core/rtw_mlme_ext.c
+++ b/rtl8188eu/core/rtw_mlme_ext.c
@@ -2858,7 +2858,7 @@ static unsigned int OnAuthClient(struct
 			if (p == NULL)
 				goto authclnt_fail;
 
-			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
+			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
 			pmlmeinfo->auth_seq = 3;
 			issue_auth(padapter, NULL, 0);
 			set_link_timer(pmlmeext, REAUTH_TO);
diff -u -p a/rtl8192e/rtl819x_HTProc.c b/rtl8192e/rtl819x_HTProc.c
--- a/rtl8192e/rtl819x_HTProc.c
+++ b/rtl8192e/rtl819x_HTProc.c
@@ -655,13 +655,13 @@ void HTInitializeHTInfo(struct rtllib_de
 	pHTInfo->CurrentMPDUDensity = pHTInfo->MPDU_Density;
 	pHTInfo->CurrentAMPDUFactor = pHTInfo->AMPDU_Factor;
 
-	memset((void *)(&(pHTInfo->SelfHTCap)), 0,
+	memset((&(pHTInfo->SelfHTCap)), 0,
 		sizeof(pHTInfo->SelfHTCap));
-	memset((void *)(&(pHTInfo->SelfHTInfo)), 0,
+	memset((&(pHTInfo->SelfHTInfo)), 0,
 		sizeof(pHTInfo->SelfHTInfo));
-	memset((void *)(&(pHTInfo->PeerHTCapBuf)), 0,
+	memset((&(pHTInfo->PeerHTCapBuf)), 0,
 		sizeof(pHTInfo->PeerHTCapBuf));
-	memset((void *)(&(pHTInfo->PeerHTInfoBuf)), 0,
+	memset((&(pHTInfo->PeerHTInfoBuf)), 0,
 		sizeof(pHTInfo->PeerHTInfoBuf));
 
 	pHTInfo->bSwBwInProgress = false;
diff -u -p a/rtl8192u/r819xU_cmdpkt.c b/rtl8192u/r819xU_cmdpkt.c
--- a/rtl8192u/r819xU_cmdpkt.c
+++ b/rtl8192u/r819xU_cmdpkt.c
@@ -373,7 +373,7 @@ static void cmpk_handle_tx_status(struct
 {
 	cmpk_tx_status_t	rx_tx_sts;
 
-	memcpy((void *)&rx_tx_sts, (void *)pmsg, sizeof(cmpk_tx_status_t));
+	memcpy(&rx_tx_sts, pmsg, sizeof(cmpk_tx_status_t));
 	/* 2. Use tx feedback info to count TX statistics. */
 	cmpk_count_tx_status(dev, &rx_tx_sts);
 }
diff -u -p a/rtl8712/ieee80211.c b/rtl8712/ieee80211.c
--- a/rtl8712/ieee80211.c
+++ b/rtl8712/ieee80211.c
@@ -90,7 +90,7 @@ u8 *r8712_set_ie(u8 *pbuf, sint index, u
 	*pbuf = (u8)index;
 	*(pbuf + 1) = (u8)len;
 	if (len > 0)
-		memcpy((void *)(pbuf + 2), (void *)source, len);
+		memcpy((pbuf + 2), source, len);
 	*frlen = *frlen + (len + 2);
 	return pbuf + len + 2;
 }
diff -u -p a/rtl8723bs/core/rtw_ieee80211.c b/rtl8723bs/core/rtw_ieee80211.c
--- a/rtl8723bs/core/rtw_ieee80211.c
+++ b/rtl8723bs/core/rtw_ieee80211.c
@@ -112,7 +112,7 @@ int rtw_check_network_type(unsigned char
 u8 *rtw_set_fixed_ie(unsigned char *pbuf, unsigned int len, unsigned char *source,
 				unsigned int *frlen)
 {
-	memcpy((void *)pbuf, (void *)source, len);
+	memcpy(pbuf, source, len);
 	*frlen = *frlen + len;
 	return pbuf + len;
 }
@@ -132,7 +132,7 @@ u8 *rtw_set_ie
 	*(pbuf + 1) = (u8)len;
 
 	if (len > 0)
-		memcpy((void *)(pbuf + 2), (void *)source, len);
+		memcpy((pbuf + 2), source, len);
 
 	*frlen = *frlen + (len + 2);
 
diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
--- a/rtl8723bs/core/rtw_mlme_ext.c
+++ b/rtl8723bs/core/rtw_mlme_ext.c
@@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
 				goto authclnt_fail;
 			}
 
-			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
+			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
 			pmlmeinfo->auth_seq = 3;
 			issue_auth(padapter, NULL, 0);
 			set_link_timer(pmlmeext, REAUTH_TO);
diff -u -p a/rtl8723bs/hal/rtl8723b_hal_init.c b/rtl8723bs/hal/rtl8723b_hal_init.c
--- a/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -2432,13 +2432,13 @@ void Hal_InitPGData(struct adapter *pada
 		if (!pEEPROM->EepromOrEfuse) {
 			/*  Read EFUSE real map to shadow. */
 			EFUSE_ShadowMapUpdate(padapter, EFUSE_WIFI, false);
-			memcpy((void *)PROMContent, (void *)pEEPROM->efuse_eeprom_data, HWSET_MAX_SIZE_8723B);
+			memcpy(PROMContent, (void *)pEEPROM->efuse_eeprom_data, HWSET_MAX_SIZE_8723B);
 		}
 	} else {/* autoload fail */
 		RT_TRACE(_module_hci_hal_init_c_, _drv_notice_, ("AutoLoad Fail reported from CR9346!!\n"));
 		if (!pEEPROM->EepromOrEfuse)
 			EFUSE_ShadowMapUpdate(padapter, EFUSE_WIFI, false);
-		memcpy((void *)PROMContent, (void *)pEEPROM->efuse_eeprom_data, HWSET_MAX_SIZE_8723B);
+		memcpy(PROMContent, (void *)pEEPROM->efuse_eeprom_data, HWSET_MAX_SIZE_8723B);
 	}
 }
 
diff -u -p a/rtl8723bs/os_dep/ioctl_cfg80211.c b/rtl8723bs/os_dep/ioctl_cfg80211.c
--- a/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -98,7 +98,7 @@ static struct ieee80211_channel rtw_2ghz
 
 static void rtw_2g_channels_init(struct ieee80211_channel *channels)
 {
-	memcpy((void*)channels, (void*)rtw_2ghz_channels,
+	memcpy(channels, (void*)rtw_2ghz_channels,
 		sizeof(struct ieee80211_channel)*RTW_2G_CHANNELS_NUM
 	);
 }
@@ -2548,7 +2548,7 @@ static netdev_tx_t rtw_cfg80211_monitor_
 
 		pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
 
-		memcpy(pframe, (void*)buf, len);
+		memcpy(pframe, buf, len);
 		pattrib->pktlen = len;
 
 		pwlanhdr = (struct ieee80211_hdr *)pframe;
@@ -2750,7 +2750,7 @@ static int rtw_add_beacon(struct adapter
 		return -ENOMEM;
 
 	memcpy(pbuf, (void *)head+24, head_len-24);/*  24 =beacon header len. */
-	memcpy(pbuf+head_len-24, (void *)tail, tail_len);
+	memcpy(pbuf+head_len-24, tail, tail_len);
 
 	len = head_len+tail_len-24;
 
@@ -3041,7 +3041,7 @@ static int _cfg80211_rtw_mgmt_tx(struct
 
 	pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
 
-	memcpy(pframe, (void*)buf, len);
+	memcpy(pframe, buf, len);
 	pattrib->pktlen = len;
 
 	pwlanhdr = (struct ieee80211_hdr *)pframe;
diff -u -p a/vt6655/rxtx.c b/vt6655/rxtx.c
--- a/vt6655/rxtx.c
+++ b/vt6655/rxtx.c
@@ -1170,7 +1170,7 @@ s_cbFillTxBufHead(struct vnt_private *pD
 
 	td_info->mic_hdr = pMICHDR;
 
-	memset((void *)(pbyTxBufferAddr + wTxBufSize), 0, (cbHeaderLength - wTxBufSize));
+	memset((pbyTxBufferAddr + wTxBufSize), 0, (cbHeaderLength - wTxBufSize));
 
 	/* Fill FIFO,RrvTime,RTS,and CTS */
 	s_vGenerateTxParameter(pDevice, byPktType, tx_buffer_head, pvRrvTime, pvRTS, pvCTS,

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Dan Carpenter]

On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote:
> diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
> --- a/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/rtl8723bs/core/rtw_mlme_ext.c
> @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
>  				goto authclnt_fail;
>  			}
>  
> -			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
> +			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);

I wonder why it didn't remove the first void cast?

[ The rest of the email is bonus comments for outreachy developers ].

And someone needs to check the final patch probably to remove the extra
parentheses around "(p + 2)".  Those were necessary when for the cast
but not required after the cast is gone.

>  			pmlmeinfo->auth_seq = 3;
>  			issue_auth(padapter, NULL, 0);
>  			set_link_timer(pmlmeext, REAUTH_TO);

It's sort of tricky to know what "one thing per patch means".

-       memset((void *)(&(pHTInfo->SelfHTCap)), 0,
+       memset((&(pHTInfo->SelfHTCap)), 0,
                sizeof(pHTInfo->SelfHTCap));

Here the parentheses were never related to the cast so we should leave
them as is.  In other words, in the first example, if we didn't remove
the cast that would be "half a thing per patch" and in the second
example that would be "two things in one patch".

regards,
dan carpenter

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

On Sun, 20 Oct 2019, Dan Carpenter wrote:

> On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote:
> > diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
> > --- a/rtl8723bs/core/rtw_mlme_ext.c
> > +++ b/rtl8723bs/core/rtw_mlme_ext.c
> > @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
> >  				goto authclnt_fail;
> >  			}
> >
> > -			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
> > +			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
>
> I wonder why it didn't remove the first void cast?

If "it" is a semantic patch, it is probably because Coccinelle wasn't able
to find the definition of the type of pmlmeinfo.  By default, Coccinelle
only consults a few header files (ones with the same names as the .c file
or ones included with "" and located in the same directory).

>
> [ The rest of the email is bonus comments for outreachy developers ].
>
> And someone needs to check the final patch probably to remove the extra
> parentheses around "(p + 2)".  Those were necessary when for the cast
> but not required after the cast is gone.

The rule could have contained

- (void *)(e)
+ e

That should also match cases with no parentheses.  I think there is even
something to put the parentheses back if they are needed, but overall the
final patch should be checked carefully in any case.

julia

>
> >  			pmlmeinfo->auth_seq = 3;
> >  			issue_auth(padapter, NULL, 0);
> >  			set_link_timer(pmlmeext, REAUTH_TO);
>
> It's sort of tricky to know what "one thing per patch means".
>
> -       memset((void *)(&(pHTInfo->SelfHTCap)), 0,
> +       memset((&(pHTInfo->SelfHTCap)), 0,
>                 sizeof(pHTInfo->SelfHTCap));
>
> Here the parentheses were never related to the cast so we should leave
> them as is.  In other words, in the first example, if we didn't remove
> the cast that would be "half a thing per patch" and in the second
> example that would be "two things in one patch".
>
> regards,
> dan carpenter
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20191020191759.GJ24678%40kadam.
>

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Joe Perches]

On Sun, 2019-10-20 at 22:17 +0300, Dan Carpenter wrote:
> On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote:
> > diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
[]
> > @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
> >  				goto authclnt_fail;
> >  			}
> >  
> > -			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
> > +			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
> 
> I wonder why it didn't remove the first void cast?

drivers/staging/rtl8723bs/include/sta_info.h:151:       unsigned char chg_txt[128];

I think the cocci transforms for an array do not match a pointer
and I wrote the cocci script without much care.

btw;

There's probably a generic cocci mechanism to check function
prototypes and then remove uses of unnecessary void pointer casts
in function calls.  I'm not going to try to figure out that syntax.

> [ The rest of the email is bonus comments for outreachy developers ].
> 
> And someone needs to check the final patch probably to remove the extra
> parentheses around "(p + 2)".  Those were necessary when for the cast
> but not required after the cast is gone.
> 
> >  			pmlmeinfo->auth_seq = 3;
> >  			issue_auth(padapter, NULL, 0);
> >  			set_link_timer(pmlmeext, REAUTH_TO);
> 
> It's sort of tricky to know what "one thing per patch means".

It seems somewhat arbitrary and based on Greg's understanding
of the experience of the patch submitter and also the language
of the potential commit message.

> -       memset((void *)(&(pHTInfo->SelfHTCap)), 0,
> +       memset((&(pHTInfo->SelfHTCap)), 0,
>                 sizeof(pHTInfo->SelfHTCap));
> 
> Here the parentheses were never related to the cast so we should leave
> them as is.  In other words, in the first example, if we didn't remove
> the cast that would be "half a thing per patch" and in the second
> example that would be "two things in one patch".

For style patches, it's frequently easier and better to 
do all the code transformation at once.

IMO the last should be:

	memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));

like it is here:

drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c:1056:       memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));

btw2:

I really dislike all the code inconsistencies and
unnecessary code duplication with miscellaneous changes
in the rtl staging drivers....

Horrid stuff.

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

On Sun, 20 Oct 2019, Joe Perches wrote:

> On Sun, 2019-10-20 at 22:17 +0300, Dan Carpenter wrote:
> > On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote:
> > > diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
> []
> > > @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
> > >  				goto authclnt_fail;
> > >  			}
> > >
> > > -			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
> > > +			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
> >
> > I wonder why it didn't remove the first void cast?
>
> drivers/staging/rtl8723bs/include/sta_info.h:151:       unsigned char chg_txt[128];
>
> I think the cocci transforms for an array do not match a pointer

This is also correct.

julia

> and I wrote the cocci script without much care.
>
> btw;
>
> There's probably a generic cocci mechanism to check function
> prototypes and then remove uses of unnecessary void pointer casts
> in function calls.  I'm not going to try to figure out that syntax.
>
> > [ The rest of the email is bonus comments for outreachy developers ].
> >
> > And someone needs to check the final patch probably to remove the extra
> > parentheses around "(p + 2)".  Those were necessary when for the cast
> > but not required after the cast is gone.
> >
> > >  			pmlmeinfo->auth_seq = 3;
> > >  			issue_auth(padapter, NULL, 0);
> > >  			set_link_timer(pmlmeext, REAUTH_TO);
> >
> > It's sort of tricky to know what "one thing per patch means".
>
> It seems somewhat arbitrary and based on Greg's understanding
> of the experience of the patch submitter and also the language
> of the potential commit message.
>
> > -       memset((void *)(&(pHTInfo->SelfHTCap)), 0,
> > +       memset((&(pHTInfo->SelfHTCap)), 0,
> >                 sizeof(pHTInfo->SelfHTCap));
> >
> > Here the parentheses were never related to the cast so we should leave
> > them as is.  In other words, in the first example, if we didn't remove
> > the cast that would be "half a thing per patch" and in the second
> > example that would be "two things in one patch".
>
> For style patches, it's frequently easier and better to
> do all the code transformation at once.
>
> IMO the last should be:
>
> 	memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));
>
> like it is here:
>
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c:1056:       memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));
>
> btw2:
>
> I really dislike all the code inconsistencies and
> unnecessary code duplication with miscellaneous changes
> in the rtl staging drivers....
>
> Horrid stuff.
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6e6bc92cac0858fe5bd37b28f688d3da043f4bef.camel%40perches.com.
>

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

On Sun, 20 Oct 2019, Joe Perches wrote:

> On Sun, 2019-10-20 at 22:17 +0300, Dan Carpenter wrote:
> > On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote:
> > > diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c
> []
> > > @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter
> > >  				goto authclnt_fail;
> > >  			}
> > >
> > > -			memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len);
> > > +			memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len);
> >
> > I wonder why it didn't remove the first void cast?
>
> drivers/staging/rtl8723bs/include/sta_info.h:151:       unsigned char chg_txt[128];
>
> I think the cocci transforms for an array do not match a pointer
> and I wrote the cocci script without much care.
>
> btw;
>
> There's probably a generic cocci mechanism to check function
> prototypes and then remove uses of unnecessary void pointer casts
> in function calls.  I'm not going to try to figure out that syntax.

With the --recursive-includes option, perhaps:

@r@
identifier f;
parameter list[n] ps;
type T;
identifier i;
@@

T f(ps, void *i, ...);

@@
expression e;
identifier r.f;
expression list[r.n] es;
@@

f(es,
- (void *)(e)
+ e
  ,...)

This of course only works for functions that have prototypes, and not for
macros.  It will also run slowly.

julia


>
> > [ The rest of the email is bonus comments for outreachy developers ].
> >
> > And someone needs to check the final patch probably to remove the extra
> > parentheses around "(p + 2)".  Those were necessary when for the cast
> > but not required after the cast is gone.
> >
> > >  			pmlmeinfo->auth_seq = 3;
> > >  			issue_auth(padapter, NULL, 0);
> > >  			set_link_timer(pmlmeext, REAUTH_TO);
> >
> > It's sort of tricky to know what "one thing per patch means".
>
> It seems somewhat arbitrary and based on Greg's understanding
> of the experience of the patch submitter and also the language
> of the potential commit message.
>
> > -       memset((void *)(&(pHTInfo->SelfHTCap)), 0,
> > +       memset((&(pHTInfo->SelfHTCap)), 0,
> >                 sizeof(pHTInfo->SelfHTCap));
> >
> > Here the parentheses were never related to the cast so we should leave
> > them as is.  In other words, in the first example, if we didn't remove
> > the cast that would be "half a thing per patch" and in the second
> > example that would be "two things in one patch".
>
> For style patches, it's frequently easier and better to
> do all the code transformation at once.
>
> IMO the last should be:
>
> 	memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));
>
> like it is here:
>
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c:1056:       memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap));
>
> btw2:
>
> I really dislike all the code inconsistencies and
> unnecessary code duplication with miscellaneous changes
> in the rtl staging drivers....
>
> Horrid stuff.
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6e6bc92cac0858fe5bd37b28f688d3da043f4bef.camel%40perches.com.
>

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Joe Perches]

On Sun, 2019-10-20 at 21:52 +0200, Julia Lawall wrote:
> On Sun, 20 Oct 2019, Joe Perches wrote:
[]
> > There's probably a generic cocci mechanism to check function
> > prototypes and then remove uses of unnecessary void pointer casts
> > in function calls.  I'm not going to try to figure out that syntax.
> 
> With the --recursive-includes option, perhaps:
> 
> @r@
> identifier f;
> parameter list[n] ps;
> type T;
> identifier i;
> @@
> 
> T f(ps, void *i, ...);
> 
> @@
> expression e;
> identifier r.f;
> expression list[r.n] es;
> @@
> 
> f(es,
> - (void *)(e)
> + e
>   ,...)
> 
> This of course only works for functions that have prototypes, and not for
> macros.  It will also run slowly.

You are not kidding about slow, but it doesn't seem to work
for mem<foo>, maybe because system includes aren't analyzed.

Single file processing time on an XPS13 averages more than
100 seconds per file.

Also:

	expression e;

could probably be better as:

	type T;
	T *p;

as some of the expressions cast to void are int or size_t
and it's probably better to restrict the conversions to
just pointer or array types.

Re: [Outreachy kernel] Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

On Sun, 20 Oct 2019, Joe Perches wrote:

> On Sun, 2019-10-20 at 21:52 +0200, Julia Lawall wrote:
> > On Sun, 20 Oct 2019, Joe Perches wrote:
> []
> > > There's probably a generic cocci mechanism to check function
> > > prototypes and then remove uses of unnecessary void pointer casts
> > > in function calls.  I'm not going to try to figure out that syntax.
> >
> > With the --recursive-includes option, perhaps:
> >
> > @r@
> > identifier f;
> > parameter list[n] ps;
> > type T;
> > identifier i;
> > @@
> >
> > T f(ps, void *i, ...);
> >
> > @@
> > expression e;
> > identifier r.f;
> > expression list[r.n] es;
> > @@
> >
> > f(es,
> > - (void *)(e)
> > + e
> >   ,...)
> >
> > This of course only works for functions that have prototypes, and not for
> > macros.  It will also run slowly.
>
> You are not kidding about slow, but it doesn't seem to work
> for mem<foo>, maybe because system includes aren't analyzed.

No they are not.

> Single file processing time on an XPS13 averages more than
> 100 seconds per file.

Not surprising.

Actually, --include-headers-for-types should provide some benefit.  That
discards the header files after the type inference.

> Also:
>
> 	expression e;
>
> could probably be better as:
>
> 	type T;
> 	T *p;

Good point.  expression *e; would be sufficient.

julia

>
> as some of the expressions cast to void are int or size_t
> and it's probably better to restrict the conversions to
> just pointer or array types.
>
>
>

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Julia Lawall]

> btw2:
>
> I really dislike all the code inconsistencies and
> unnecessary code duplication with miscellaneous changes
> in the rtl staging drivers....
>
> Horrid stuff.

I'm not sure what you mean by "miscellaneous changes".  Do you mean that
all issues should be fixed for one file before moving on to another one?

Or that there are code clones, and all of the clones should be updated at
the same time?

julia

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Joe Perches]

On Mon, 2019-10-21 at 08:52 +0200, Julia Lawall wrote:
> > btw2:
> > 
> > I really dislike all the code inconsistencies and
> > unnecessary code duplication with miscellaneous changes
> > in the rtl staging drivers....
> > 
> > Horrid stuff.
> 
> I'm not sure what you mean by "miscellaneous changes".  Do you mean that
> all issues should be fixed for one file before moving on to another one?
> 
> Or that there are code clones, and all of the clones should be updated at
> the same time?

Neither really.

But realtek drivers are basically code clones where
realtek should prefer to have a single library used
for multiple drivers.

And staging is basically a dumping ground for realtek
wireless drivers.

https://lkml.org/lkml/2019/5/15/1405

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Dan Carpenter]

On Sun, Oct 20, 2019 at 12:36:50PM -0700, Joe Perches wrote:
> > It's sort of tricky to know what "one thing per patch means".
> 
> It seems somewhat arbitrary and based on Greg's understanding
> of the experience of the patch submitter and also the language
> of the potential commit message.

Of course the language of the commit message matters.  You have to
"sell" your commit and convince us why we should apply it.  That's a
life lesson right there...  :P

regards,
dan carpenter

Re: [PATCH v1 1/5] staging: wfx: fix warnings of no space is necessary

[Jerome Pouiller]

On Saturday 19 October 2019 16:24:43 CEST Dan Carpenter wrote:
> On Sat, Oct 19, 2019 at 03:07:15PM +0100, Jules Irenge wrote:
> > diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
> > index 3355183fc86c..573216b08042 100644
> > --- a/drivers/staging/wfx/bh.c
> > +++ b/drivers/staging/wfx/bh.c
> > @@ -69,13 +69,13 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
> >       if (wfx_data_read(wdev, skb->data, alloc_len))
> >               goto err;
> >
> > -     piggyback = le16_to_cpup((u16 *) (skb->data + alloc_len - 2));
> > +     piggyback = le16_to_cpup((u16 *)(skb->data + alloc_len - 2));
> >       _trace_piggyback(piggyback, false);
> >
> > -     hif = (struct hif_msg *) skb->data;
> > +     hif = (struct hif_msg *)skb->data;
> >       WARN(hif->encrypted & 0x1, "unsupported encryption type");
> >       if (hif->encrypted == 0x2) {
> > -             if (wfx_sl_decode(wdev, (void *) hif)) {
> > +             if (wfx_sl_decode(wdev, (void *)hif)) {
> 
> In the future you may want to go through and remove the (void *) casts.
> It's not required here.
> 
> > diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> > index f65f7d75e731..effd07957753 100644
> > --- a/drivers/staging/wfx/bus_spi.c
> > +++ b/drivers/staging/wfx/bus_spi.c
> > @@ -90,7 +90,7 @@ static int wfx_spi_copy_to_io(void *priv, unsigned int addr,
> >       struct wfx_spi_priv *bus = priv;
> >       u16 regaddr = (addr << 12) | (count / 2);
> >       // FIXME: use a bounce buffer
> > -     u16 *src16 = (void *) src;
> > +     u16 *src16 = (void *)src;
> 
> Here we are just getting rid of the constness.  Apparently we are doing
> that so we can modify it without GCC pointing out the bug!!  I don't
> know the code but this seems very wrong.

Hello Dan, Jules,

Indeed, this code should be improved.

Each u16 from src is byte-swapped before to be sent to SPI and restored
before to return from the function:

	for (i = 0; i < count / 2; i++)
		swab16s(&src16[i]);
	[...]
	spi_sync(bus->func, &m);
   [...]
	for (i = 0; i < count / 2; i++)
		swab16s(&src16[i]);

So, src is same than original, but it is not const.

This is exactly the purpose of the FIXME just before the cast: "use a
bounce buffer". However, I did not yet make this change because I worry
about a possible performance penalty.

-- 
Jérôme Pouiller

[PATCH v1 2/5] staging: wfx: fix warning of line over 80 characters

[Jules Irenge]

Fix warning of lines over 80 characters.
Issue detected by checkpatch tool.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/wfx/bh.c       |  17 ++++--
 drivers/staging/wfx/bus.h      |   6 +-
 drivers/staging/wfx/bus_sdio.c |   3 +-
 drivers/staging/wfx/bus_spi.c  |   9 ++-
 drivers/staging/wfx/data_rx.c  |  15 +++--
 drivers/staging/wfx/data_tx.c  | 101 ++++++++++++++++++++++-----------
 6 files changed, 102 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 573216b08042..955ed3a1dd73 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -32,7 +32,8 @@ static void device_wakeup(struct wfx_dev *wdev)
 		// completion without consume it (a kind of
 		// wait_for_completion_done_timeout()). So we have to emulate
 		// it.
-		if (wait_for_completion_timeout(&wdev->hif.ctrl_ready, msecs_to_jiffies(2) + 1))
+		if (wait_for_completion_timeout(&wdev->hif.ctrl_ready,
+						msecs_to_jiffies(2) + 1))
 			complete(&wdev->hif.ctrl_ready);
 		else
 			dev_err(wdev->dev, "timeout while wake up chip\n");
@@ -179,8 +180,9 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif)
 	wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1);
 
 	if (wfx_is_secure_command(wdev, hif->id)) {
-		len = round_up(len - sizeof(hif->len), 16) + sizeof(hif->len)
-		      + sizeof(struct hif_sl_msg_hdr) + sizeof(struct hif_sl_tag);
+		len = round_up(len - sizeof(hif->len), 16) + sizeof(hif->len) +
+			sizeof(struct hif_sl_msg_hdr) +
+			sizeof(struct hif_sl_tag);
 		// AES support encryption in-place. However, mac80211 access to
 		// 802.11 header after frame was sent (to get MAC addresses).
 		// So, keep origin buffer clear.
@@ -241,7 +243,8 @@ static void ack_sdio_data(struct wfx_dev *wdev)
 
 	config_reg_read(wdev, &cfg_reg);
 	if (cfg_reg & 0xFF) {
-		dev_warn(wdev->dev, "chip reports errors: %02x\n", cfg_reg & 0xFF);
+		dev_warn(wdev->dev, "chip reports errors: %02x\n",
+			 cfg_reg & 0xFF);
 		config_reg_write_bits(wdev, 0xFF, 0x00);
 	}
 }
@@ -268,11 +271,13 @@ static void bh_work(struct work_struct *work)
 
 	if (last_op_is_rx)
 		ack_sdio_data(wdev);
-	if (!wdev->hif.tx_buffers_used && !work_pending(work) && !atomic_read(&wdev->scan_in_progress)) {
+	if (!wdev->hif.tx_buffers_used && !work_pending(work) &&
+	    !atomic_read(&wdev->scan_in_progress)) {
 		device_release(wdev);
 		release_chip = true;
 	}
-	_trace_bh_stats(stats_ind, stats_req, stats_cnf, wdev->hif.tx_buffers_used, release_chip);
+	_trace_bh_stats(stats_ind, stats_req, stats_cnf,
+			wdev->hif.tx_buffers_used, release_chip);
 }
 
 /*
diff --git a/drivers/staging/wfx/bus.h b/drivers/staging/wfx/bus.h
index eb77abc09ec2..62d6ecabe4cb 100644
--- a/drivers/staging/wfx/bus.h
+++ b/drivers/staging/wfx/bus.h
@@ -21,8 +21,10 @@
 #define WFX_REG_FRAME_OUT     0x7
 
 struct hwbus_ops {
-	int (*copy_from_io)(void *bus_priv, unsigned int addr, void *dst, size_t count);
-	int (*copy_to_io)(void *bus_priv, unsigned int addr, const void *src, size_t count);
+	int (*copy_from_io)(void *bus_priv, unsigned int addr,
+			    void *dst, size_t count);
+	int (*copy_to_io)(void *bus_priv, unsigned int addr,
+			  const void *src, size_t count);
 	void (*lock)(void *bus_priv);
 	void (*unlock)(void *bus_priv);
 	size_t (*align_size)(void *bus_priv, size_t size);
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index 184e20dfdd62..375e07d6d9ae 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -180,7 +180,8 @@ static int wfx_sdio_probe(struct sdio_func *func,
 		}
 		bus->of_irq = irq_of_parse_and_map(np, 0);
 	} else {
-		dev_warn(&func->dev, "device is not declared in DT, features will be limited\n");
+		dev_warn(&func->dev,
+			 "device is not declared in DT, features will be limited\n");
 		// FIXME: ignore VID/PID and only rely on device tree
 		// return -ENODEV;
 	}
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index effd07957753..ab0cda1e124f 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -178,11 +178,14 @@ static int wfx_spi_probe(struct spi_device *func)
 		return ret;
 	// Trace below is also displayed by spi_setup() if compiled with DEBUG
 	dev_dbg(&func->dev, "SPI params: CS=%d, mode=%d bits/word=%d speed=%d\n",
-		func->chip_select, func->mode, func->bits_per_word, func->max_speed_hz);
+		func->chip_select, func->mode, func->bits_per_word,
+		func->max_speed_hz);
 	if (func->bits_per_word != 16 && func->bits_per_word != 8)
-		dev_warn(&func->dev, "unusual bits/word value: %d\n", func->bits_per_word);
+		dev_warn(&func->dev, "unusual bits/word value: %d\n",
+			 func->bits_per_word);
 	if (func->max_speed_hz > 49000000)
-		dev_warn(&func->dev, "%dHz is a very high speed\n", func->max_speed_hz);
+		dev_warn(&func->dev, "%dHz is a very high speed\n",
+			 func->max_speed_hz);
 
 	bus = devm_kzalloc(&func->dev, sizeof(*bus), GFP_KERNEL);
 	if (!bus)
diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 3a79ab93e97e..522592d71aac 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -98,7 +98,8 @@ static int wfx_drop_encrypt_data(struct wfx_dev *wdev, struct hif_ind_rx *arg, s
 
 }
 
-void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb)
+void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg,
+	       struct sk_buff *skb)
 {
 	int link_id = arg->rx_flags.peer_sta_id;
 	struct ieee80211_rx_status *hdr = IEEE80211_SKB_RXCB(skb);
@@ -118,7 +119,8 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 	if (link_id && link_id <= WFX_MAX_STA_IN_AP_MODE) {
 		entry = &wvif->link_id_db[link_id - 1];
 		entry->timestamp = jiffies;
-		if (entry->status == WFX_LINK_SOFT && ieee80211_is_data(frame->frame_control))
+		if (entry->status == WFX_LINK_SOFT &&
+		    ieee80211_is_data(frame->frame_control))
 			early_data = true;
 	}
 
@@ -137,7 +139,8 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 			goto drop;
 
 	hdr->band = NL80211_BAND_2GHZ;
-	hdr->freq = ieee80211_channel_to_frequency(arg->channel_number, hdr->band);
+	hdr->freq = ieee80211_channel_to_frequency(arg->channel_number,
+						   hdr->band);
 
 	if (arg->rxed_rate >= 14) {
 		hdr->encoding = RX_ENC_HT;
@@ -166,7 +169,8 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 		goto drop;
 	if (ieee80211_is_beacon(frame->frame_control)
 	    && !arg->status && wvif->vif
-	    && ether_addr_equal(ieee80211_get_SA(frame), wvif->vif->bss_conf.bssid)) {
+	    && ether_addr_equal(ieee80211_get_SA(frame),
+				wvif->vif->bss_conf.bssid)) {
 		const u8 *tim_ie;
 		u8 *ies = mgmt->u.beacon.variable;
 		size_t ies_len = skb->len - (ies - skb->data);
@@ -183,7 +187,8 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg, struct sk_buff *skb
 
 		/* Disable beacon filter once we're associated... */
 		if (wvif->disable_beacon_filter &&
-		    (wvif->vif->bss_conf.assoc || wvif->vif->bss_conf.ibss_joined)) {
+		    (wvif->vif->bss_conf.assoc ||
+		     wvif->vif->bss_conf.ibss_joined)) {
 			wvif->disable_beacon_filter = false;
 			schedule_work(&wvif->update_filtering_work);
 		}
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index cf73b83ccc9e..619ab2cac5fc 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -20,7 +20,8 @@
 #define WFX_LINK_ID_NO_ASSOC   15
 #define WFX_LINK_ID_GC_TIMEOUT ((unsigned long)(10 * HZ))
 
-static int wfx_get_hw_rate(struct wfx_dev *wdev, const struct ieee80211_tx_rate *rate)
+static int wfx_get_hw_rate(struct wfx_dev *wdev,
+			   const struct ieee80211_tx_rate *rate)
 {
 	if (rate->idx < 0)
 		return -1;
@@ -120,12 +121,14 @@ static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
 	}
 }
 
-static bool tx_policy_is_equal(const struct tx_policy *a, const struct tx_policy *b)
+static bool tx_policy_is_equal(const struct tx_policy *a,
+			       const struct tx_policy *b)
 {
 	return !memcmp(a->rates, b->rates, sizeof(a->rates));
 }
 
-static int wfx_tx_policy_find(struct tx_policy_cache *cache, struct tx_policy *wanted)
+static int wfx_tx_policy_find(struct tx_policy_cache *cache,
+			      struct tx_policy *wanted)
 {
 	struct tx_policy *it;
 
@@ -138,13 +141,15 @@ static int wfx_tx_policy_find(struct tx_policy_cache *cache, struct tx_policy *w
 	return -1;
 }
 
-static void wfx_tx_policy_use(struct tx_policy_cache *cache, struct tx_policy *entry)
+static void wfx_tx_policy_use(struct tx_policy_cache *cache,
+			      struct tx_policy *entry)
 {
 	++entry->usage_count;
 	list_move(&entry->link, &cache->used);
 }
 
-static int wfx_tx_policy_release(struct tx_policy_cache *cache, struct tx_policy *entry)
+static int wfx_tx_policy_release(struct tx_policy_cache *cache,
+				 struct tx_policy *entry)
 {
 	int ret = --entry->usage_count;
 
@@ -153,8 +158,9 @@ static int wfx_tx_policy_release(struct tx_policy_cache *cache, struct tx_policy
 	return ret;
 }
 
-static int wfx_tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
-			 bool *renew)
+static int wfx_tx_policy_get(struct wfx_vif *wvif,
+			     struct ieee80211_tx_rate *rates,
+			     bool *renew)
 {
 	int idx;
 	struct tx_policy_cache *cache = &wvif->tx_policy_cache;
@@ -211,7 +217,10 @@ static int wfx_tx_policy_upload(struct wfx_vif *wvif)
 	int i;
 	struct tx_policy_cache *cache = &wvif->tx_policy_cache;
 	struct hif_mib_set_tx_rate_retry_policy *arg =
-		kzalloc(struct_size(arg, tx_rate_retry_policy, HIF_MIB_NUM_TX_RATE_RETRY_POLICIES), GFP_KERNEL);
+		kzalloc(struct_size(arg,
+				    tx_rate_retry_policy,
+				    HIF_MIB_NUM_TX_RATE_RETRY_POLICIES),
+			GFP_KERNEL);
 	struct hif_mib_tx_rate_retry_policy *dst;
 
 	spin_lock_bh(&cache->lock);
@@ -220,7 +229,8 @@ static int wfx_tx_policy_upload(struct wfx_vif *wvif)
 		struct tx_policy *src = &cache->cache[i];
 
 		if (!src->uploaded && memzcmp(src->rates, sizeof(src->rates))) {
-			dst = arg->tx_rate_retry_policy + arg->num_tx_rate_policies;
+			dst = arg->tx_rate_retry_policy +
+				arg->num_tx_rate_policies;
 
 			dst->policy_index = i;
 			dst->short_retry_count = 255;
@@ -326,7 +336,8 @@ int wfx_find_link_id(struct wfx_vif *wvif, const u8 *mac)
 	return ret;
 }
 
-static int wfx_map_link(struct wfx_vif *wvif, struct wfx_link_entry *link_entry, int sta_id)
+static int wfx_map_link(struct wfx_vif *wvif,
+			struct wfx_link_entry *link_entry, int sta_id)
 {
 	int ret;
 
@@ -437,7 +448,8 @@ static bool ieee80211_is_action_back(struct ieee80211_hdr *hdr)
 }
 
 static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
-			     struct wfx_tx_priv *tx_priv, struct ieee80211_sta *sta)
+			     struct wfx_tx_priv *tx_priv,
+			     struct ieee80211_sta *sta)
 {
 	u32 mask = ~BIT(tx_priv->raw_link_id);
 
@@ -447,7 +459,8 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 		wvif->pspoll_mask &= mask;
 	}
 
-	if (tx_priv->link_id == WFX_LINK_ID_AFTER_DTIM && !wvif->mcast_buffered) {
+	if (tx_priv->link_id == WFX_LINK_ID_AFTER_DTIM &&
+	    !wvif->mcast_buffered) {
 		wvif->mcast_buffered = true;
 		if (wvif->sta_asleep_mask)
 			schedule_work(&wvif->mcast_start_work);
@@ -464,9 +477,12 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 		ieee80211_sta_set_buffered(sta, tx_priv->tid, true);
 }
 
-static uint8_t wfx_tx_get_raw_link_id(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct ieee80211_hdr *hdr)
+static uint8_t wfx_tx_get_raw_link_id(struct wfx_vif *wvif,
+				      struct ieee80211_sta *sta,
+				      struct ieee80211_hdr *hdr)
 {
-	struct wfx_sta_priv *sta_priv = sta ? (struct wfx_sta_priv *) &sta->drv_priv : NULL;
+	struct wfx_sta_priv *sta_priv =
+		sta ? (struct wfx_sta_priv *) &sta->drv_priv : NULL;
 	const u8 *da = ieee80211_get_DA(hdr);
 	int ret;
 
@@ -505,8 +521,11 @@ static void wfx_tx_fixup_rates(struct ieee80211_tx_rate *rates)
 	do {
 		finished = true;
 		for (i = 0; i < IEEE80211_TX_MAX_RATES - 1; i++) {
-			if (rates[i + 1].idx == rates[i].idx && rates[i].idx != -1) {
-				rates[i].count =  max_t(int, rates[i].count, rates[i + 1].count);
+			if (rates[i + 1].idx == rates[i].idx &&
+			    rates[i].idx != -1) {
+				rates[i].count =
+					max_t(int, rates[i].count,
+					      rates[i + 1].count);
 				rates[i + 1].idx = -1;
 				rates[i + 1].count = 0;
 
@@ -523,12 +542,14 @@ static void wfx_tx_fixup_rates(struct ieee80211_tx_rate *rates)
 		rates[i].flags &= ~IEEE80211_TX_RC_SHORT_GI;
 }
 
-static uint8_t wfx_tx_get_rate_id(struct wfx_vif *wvif, struct ieee80211_tx_info *tx_info)
+static uint8_t wfx_tx_get_rate_id(struct wfx_vif *wvif,
+				  struct ieee80211_tx_info *tx_info)
 {
 	bool tx_policy_renew = false;
 	uint8_t rate_id;
 
-	rate_id = wfx_tx_policy_get(wvif, tx_info->driver_rates, &tx_policy_renew);
+	rate_id = wfx_tx_policy_get(wvif,
+				    tx_info->driver_rates, &tx_policy_renew);
 	WARN(rate_id == WFX_INVALID_RATE_ID, "unable to get a valid Tx policy");
 
 	if (tx_policy_renew) {
@@ -584,7 +605,8 @@ static int wfx_tx_get_icv_len(struct ieee80211_key_conf *hw_key)
 	return hw_key->icv_len + mic_space;
 }
 
-static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct sk_buff *skb)
+static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
+			struct sk_buff *skb)
 {
 	struct hif_msg *hif_msg;
 	struct hif_req_tx *req;
@@ -594,7 +616,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int queue_id = tx_info->hw_queue;
 	size_t offset = (size_t) skb->data & 3;
-	int wmsg_len = sizeof(struct hif_msg) + sizeof(struct hif_req_tx) + offset;
+	int wmsg_len = sizeof(struct hif_msg) +
+			sizeof(struct hif_req_tx) + offset;
 
 	WARN(queue_id >= IEEE80211_NUM_ACS, "unsupported queue_id");
 	wfx_tx_fixup_rates(tx_info->driver_rates);
@@ -632,7 +655,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 
 	// Fill tx request
 	req = (struct hif_req_tx *)hif_msg->body;
-	req->packet_id = queue_id << 16 | IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
+	req->packet_id = queue_id << 16 |
+			 IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 	req->data_flags.fc_offset = offset;
 	req->queue_id.peer_sta_id = tx_priv->raw_link_id;
 	// Queue index are inverted between firmware and Linux
@@ -655,7 +679,8 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	struct ieee80211_sta *sta = control ? control->sta : NULL;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	size_t driver_data_room = FIELD_SIZEOF(struct ieee80211_tx_info, rate_driver_data);
+	size_t driver_data_room = FIELD_SIZEOF(struct ieee80211_tx_info,
+					       rate_driver_data);
 
 	compiletime_assert(sizeof(struct wfx_tx_priv) <= driver_data_room,
 			   "struct tx_priv is too large");
@@ -692,12 +717,15 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, struct hif_cnf_tx *arg)
 
 	skb = wfx_pending_get(wvif->wdev, arg->packet_id);
 	if (!skb) {
-		dev_warn(wvif->wdev->dev, "received unknown packet_id (%#.8x) from chip\n", arg->packet_id);
+		dev_warn(wvif->wdev->dev,
+			 "received unknown packet_id (%#.8x) from chip\n",
+			 arg->packet_id);
 		return;
 	}
 	tx_info = IEEE80211_SKB_CB(skb);
 	tx_priv = wfx_skb_tx_priv(skb);
-	_trace_tx_stats(arg, skb, wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
+	_trace_tx_stats(arg, skb,
+			wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
 
 	// You can touch to tx_priv, but don't touch to tx_info->status.
 	tx_count = arg->ack_failures;
@@ -710,9 +738,12 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, struct hif_cnf_tx *arg)
 		if (tx_count < rate->count && arg->status && arg->ack_failures)
 			dev_dbg(wvif->wdev->dev, "all retries were not consumed: %d != %d\n",
 				rate->count, tx_count);
-		if (tx_count <= rate->count && tx_count && arg->txed_rate != wfx_get_hw_rate(wvif->wdev, rate))
-			dev_dbg(wvif->wdev->dev, "inconsistent tx_info rates: %d != %d\n",
-				arg->txed_rate, wfx_get_hw_rate(wvif->wdev, rate));
+		if (tx_count <= rate->count && tx_count &&
+		    arg->txed_rate != wfx_get_hw_rate(wvif->wdev, rate))
+			dev_dbg(wvif->wdev->dev,
+				"inconsistent tx_info rates: %d != %d\n",
+				arg->txed_rate,
+				wfx_get_hw_rate(wvif->wdev, rate));
 		if (tx_count > rate->count) {
 			tx_count -= rate->count;
 		} else if (!tx_count) {
@@ -724,7 +755,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, struct hif_cnf_tx *arg)
 		}
 	}
 	if (tx_count)
-		dev_dbg(wvif->wdev->dev, "%d more retries than expected\n", tx_count);
+		dev_dbg(wvif->wdev->dev,
+			"%d more retries than expected\n", tx_count);
 	skb_trim(skb, skb->len - wfx_tx_get_icv_len(tx_priv->hw_key));
 
 	// From now, you can touch to tx_info->status, but do not touch to
@@ -734,9 +766,11 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, struct hif_cnf_tx *arg)
 	memset(tx_info->pad, 0, sizeof(tx_info->pad));
 
 	if (!arg->status) {
-		if (wvif->bss_loss_state && arg->packet_id == wvif->bss_loss_confirm_id)
+		if (wvif->bss_loss_state &&
+		    arg->packet_id == wvif->bss_loss_confirm_id)
 			wfx_cqm_bssloss_sm(wvif, 0, 1, 0);
-		tx_info->status.tx_time = arg->media_delay - arg->tx_queue_delay;
+		tx_info->status.tx_time =
+		arg->media_delay - arg->tx_queue_delay;
 		if (tx_info->flags & IEEE80211_TX_CTL_NO_ACK)
 			tx_info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
 		else
@@ -752,7 +786,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, struct hif_cnf_tx *arg)
 		wfx_suspend_resume(wvif, &suspend);
 		tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
 	} else {
-		if (wvif->bss_loss_state && arg->packet_id == wvif->bss_loss_confirm_id)
+		if (wvif->bss_loss_state &&
+		    arg->packet_id == wvif->bss_loss_confirm_id)
 			wfx_cqm_bssloss_sm(wvif, 0, 0, 1);
 	}
 	wfx_pending_remove(wvif->wdev, skb);
@@ -790,7 +825,9 @@ void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
 	struct hif_msg *hif = (struct hif_msg *)skb->data;
 	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
 	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
-	unsigned int offset = sizeof(struct hif_req_tx) + sizeof(struct hif_msg) + req->data_flags.fc_offset;
+	unsigned int offset = sizeof(struct hif_req_tx) +
+				sizeof(struct hif_msg) +
+				req->data_flags.fc_offset;
 
 	WARN_ON(!wvif);
 	skb_pull(skb, offset);
-- 
2.21.0

[PATCH v1 3/5] staging: wfx: fix warnings of logical continuation

[Jules Irenge]

Fix check warnings of logical continuations
should be on the previous line.
Issue detected by checkpatch tool.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/wfx/data_rx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 522592d71aac..52fb0f255dcd 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -163,14 +163,14 @@ void wfx_rx_cb(struct wfx_vif *wvif, struct hif_ind_rx *arg,
 	}
 
 	/* Filter block ACK negotiation: fully controlled by firmware */
-	if (ieee80211_is_action(frame->frame_control)
-	    && arg->rx_flags.match_uc_addr
-	    && mgmt->u.action.category == WLAN_CATEGORY_BACK)
+	if (ieee80211_is_action(frame->frame_control) &&
+	    arg->rx_flags.match_uc_addr &&
+	    mgmt->u.action.category == WLAN_CATEGORY_BACK)
 		goto drop;
-	if (ieee80211_is_beacon(frame->frame_control)
-	    && !arg->status && wvif->vif
-	    && ether_addr_equal(ieee80211_get_SA(frame),
-				wvif->vif->bss_conf.bssid)) {
+	if (ieee80211_is_beacon(frame->frame_control) &&
+	    !arg->status && wvif->vif &&
+	    ether_addr_equal(ieee80211_get_SA(frame),
+			     wvif->vif->bss_conf.bssid)) {
 		const u8 *tim_ie;
 		u8 *ies = mgmt->u.beacon.variable;
 		size_t ies_len = skb->len - (ies - skb->data);
-- 
2.21.0

[PATCH v1 4/5] staging: wfx: correct misspelled words

[Jules Irenge]

Correct misspelled words: retrieved and auxiliary.
Issue detected by checkpatch tool.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/wfx/data_tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 619ab2cac5fc..a02692f3210d 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -32,7 +32,7 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
 		}
 		return rate->idx + 14;
 	}
-	// WFx only support 2GHz, else band information should be retreived
+	// WFx only support 2GHz, else band information should be retrieved
 	// from ieee80211_tx_info
 	return wdev->hw->wiphy->bands[NL80211_BAND_2GHZ]->bitrates[rate->idx].hw_value;
 }
@@ -664,7 +664,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	req->ht_tx_parameters = wfx_tx_get_tx_parms(wvif->wdev, tx_info);
 	req->tx_flags.retry_policy_index = wfx_tx_get_rate_id(wvif, tx_info);
 
-	// Auxilliary operations
+	// Auxiliary operations
 	wfx_tx_manage_pm(wvif, hdr, tx_priv, sta);
 	wfx_tx_queue_put(wvif->wdev, &wvif->wdev->tx_queue[queue_id], skb);
 	wfx_bh_request_tx(wvif->wdev);
-- 
2.21.0

[PATCH v1 5/5] staging: wfx: fix warnings of alignment should match open parenthesis

[Jules Irenge]

: Fix warnings of alignment should match open parenthesis.
Issue detected by checkpatch tool.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/wfx/data_rx.c |  2 +-
 drivers/staging/wfx/data_tx.c |  2 +-
 drivers/staging/wfx/debug.c   | 14 ++++++++------
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 52fb0f255dcd..e7fcce8d0cc4 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -77,7 +77,7 @@ static int wfx_drop_encrypt_data(struct wfx_dev *wdev, struct hif_ind_rx *arg, s
 		break;
 	default:
 		dev_err(wdev->dev, "unknown encryption type %d\n",
-			 arg->rx_flags.encryp);
+			arg->rx_flags.encryp);
 		return -EIO;
 	}
 
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index a02692f3210d..ea4205ac2149 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -40,7 +40,7 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
 /* TX policy cache implementation */
 
 static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
-			    struct ieee80211_tx_rate *rates)
+				struct ieee80211_tx_rate *rates)
 {
 	int i;
 	size_t count;
diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 761ad9b4f27e..0a9ca109039c 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -141,10 +141,11 @@ static int wfx_rx_stats_show(struct seq_file *seq, void *v)
 	mutex_lock(&wdev->rx_stats_lock);
 	seq_printf(seq, "Timestamp: %dus\n", st->date);
 	seq_printf(seq, "Low power clock: frequency %uHz, external %s\n",
-		st->pwr_clk_freq,
-		st->is_ext_pwr_clk ? "yes" : "no");
-	seq_printf(seq, "Num. of frames: %d, PER (x10e4): %d, Throughput: %dKbps/s\n",
-		st->nb_rx_frame, st->per_total, st->throughput);
+		   st->pwr_clk_freq,
+		   st->is_ext_pwr_clk ? "yes" : "no");
+	seq_printf(seq,
+		   "N. of frames: %d, PER (x10e4): %d, Throughput: %dKbps/s\n",
+		   st->nb_rx_frame, st->per_total, st->throughput);
 	seq_puts(seq, "       Num. of      PER     RSSI      SNR      CFO\n");
 	seq_puts(seq, "        frames  (x10e4)    (dBm)     (dB)    (kHz)\n");
 	for (i = 0; i < ARRAY_SIZE(channel_names); i++) {
@@ -160,8 +161,9 @@ static int wfx_rx_stats_show(struct seq_file *seq, void *v)
 }
 DEFINE_SHOW_ATTRIBUTE(wfx_rx_stats);
 
-static ssize_t wfx_send_pds_write(struct file *file, const char __user *user_buf,
-			     size_t count, loff_t *ppos)
+static ssize_t wfx_send_pds_write(struct file *file,
+				  const char __user *user_buf,
+				  size_t count, loff_t *ppos)
 {
 	struct wfx_dev *wdev = file->private_data;
 	char *buf;
-- 
2.21.0