[PATCH 0/4] Staging: rtl8192u: ieee80211: Replace printk(KERN_<LEVEL> ...) with netdev_*() family.

[PATCH 0/4] Staging: rtl8192u: ieee80211: Replace printk(KERN_<LEVEL> ...) with netdev_*() family.

[Sam Muhammed]

Use the netdev_*() family across the driver, since it's specific
for network devices and already been used in multiple cases.
This series replaces the rest.
Checkpatch.pl: WARNINGS:

	Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then
	dev_dbg(dev, ... then pr_debug(...  to printk(KERN_DEBUG ...

	Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then
	dev_warn(dev, ... then pr_warn(...  to printk(KERN_WARNING ...

	WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ...
	then dev_info(dev, ... then pr_info(...  to printk(KERN_INFO ...

	Prefer [subsystem eg: netdev]_alert([subsystem]dev, ... then
	dev_alert(dev, ... then pr_alert(...  to printk(KERN_ALERT ...

Sam Muhammed (4):
  Staging: rtl8192u: ieee80211: Use netdev_dbg() for debug messages.
  Staging: rtl8192u: ieee80211: Use netdev_warn() for network devices.
  Staging: rtl8192u: ieee80211: Use netdev_info() with network devices.
  Staging: rtl8192u: ieee80211: Use netdev_alert().

 .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 19 ++++++------
 .../rtl8192u/ieee80211/ieee80211_module.c     |  3 +-
 .../rtl8192u/ieee80211/ieee80211_softmac.c    | 30 +++++++++++--------
 .../rtl8192u/ieee80211/ieee80211_softmac_wx.c |  4 +--
 .../staging/rtl8192u/ieee80211/ieee80211_tx.c | 26 +++++++---------
 .../staging/rtl8192u/ieee80211/ieee80211_wx.c |  7 ++---
 6 files changed, 43 insertions(+), 46 deletions(-)

---
2.20.1

[PATCH 1/4] Staging: rtl8192u: ieee80211: Use netdev_dbg() for debug messages.

[Sam Muhammed]

Replace printk(KERN_DEBUG ...) with netdev_dbg() across the driver.
since netdev_dbg() is preferable and specific for
printing debug messages for network devices.

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 19 +++++++++----------
 .../staging/rtl8192u/ieee80211/ieee80211_tx.c |  5 ++---
 .../staging/rtl8192u/ieee80211/ieee80211_wx.c |  2 +-
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 6f4710171151..ffe624ed0c0c 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -388,20 +388,20 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	keyidx = pos[3];
 	if (!(keyidx & BIT(5))) {
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "TKIP: received packet without ExtIV"
+			netdev_dbg(skb->dev, "TKIP: received packet without ExtIV"
 			       " flag from %pM\n", hdr->addr2);
 		}
 		return -2;
 	}
 	keyidx >>= 6;
 	if (tkey->key_idx != keyidx) {
-		printk(KERN_DEBUG "TKIP: RX tkey->key_idx=%d frame "
+		netdev_dbg(skb->dev, "TKIP: RX tkey->key_idx=%d frame "
 		       "keyidx=%d priv=%p\n", tkey->key_idx, keyidx, priv);
 		return -6;
 	}
 	if (!tkey->key_set) {
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "TKIP: received packet from %pM"
+			netdev_dbg(skb->dev, "TKIP: received packet from %pM"
 			       " with keyid=%d that does not have a configured"
 			       " key\n", hdr->addr2, keyidx);
 		}
@@ -417,7 +417,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 		if (iv32 < tkey->rx_iv32 ||
 		(iv32 == tkey->rx_iv32 && iv16 <= tkey->rx_iv16)) {
 			if (net_ratelimit()) {
-				printk(KERN_DEBUG "TKIP: replay detected: STA=%pM"
+				netdev_dbg(skb->dev, "TKIP: replay detected: STA=%pM"
 				" previous TSC %08x%04x received TSC "
 				"%08x%04x\n", hdr->addr2,
 				tkey->rx_iv32, tkey->rx_iv16, iv32, iv16);
@@ -445,7 +445,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 		skcipher_request_zero(req);
 		if (err) {
 			if (net_ratelimit()) {
-				printk(KERN_DEBUG ": TKIP: failed to decrypt "
+				netdev_dbg(skb->dev, "TKIP: failed to decrypt "
 						"received packet from %pM\n",
 						hdr->addr2);
 			}
@@ -468,7 +468,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 				tkey->rx_phase1_done = 0;
 			}
 			if (net_ratelimit()) {
-				printk(KERN_DEBUG "TKIP: ICV error detected: STA="
+				netdev_dbg(skb->dev, "TKIP: ICV error detected: STA="
 				"%pM\n", hdr->addr2);
 			}
 			tkey->dot11RSNAStatsTKIPICVErrors++;
@@ -559,7 +559,7 @@ static int ieee80211_michael_mic_add(struct sk_buff *skb, int hdr_len, void *pri
 	hdr = (struct rtl_80211_hdr_4addr *)skb->data;

 	if (skb_tailroom(skb) < 8 || skb->len < hdr_len) {
-		printk(KERN_DEBUG "Invalid packet for Michael MIC add "
+		netdev_dbg(skb->dev, "Invalid packet for Michael MIC add "
 		       "(tailroom=%d hdr_len=%d skb->len=%d)\n",
 		       skb_tailroom(skb), hdr_len, skb->len);
 		return -1;
@@ -628,10 +628,9 @@ static int ieee80211_michael_mic_verify(struct sk_buff *skb, int keyidx,
 		struct rtl_80211_hdr_4addr *hdr;
 		hdr = (struct rtl_80211_hdr_4addr *)skb->data;

-		printk(KERN_DEBUG "%s: Michael MIC verification failed for "
+		netdev_dbg(skb->dev, "Michael MIC verification failed for "
 		       "MSDU from %pM keyidx=%d\n",
-		       skb->dev ? skb->dev->name : "N/A", hdr->addr2,
-		       keyidx);
+		       hdr->addr2, keyidx);
 		if (skb->dev)
 			ieee80211_michael_mic_failure(skb->dev, hdr, keyidx);
 		tkey->dot11RSNAStatsTKIPLocalMICFailures++;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
index f0b6b8372f91..95cea2f56149 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
@@ -180,9 +180,8 @@ int ieee80211_encrypt_fragment(
 			struct rtl_80211_hdr_3addrqos *header;

 			header = (struct rtl_80211_hdr_3addrqos *)frag->data;
-			printk(KERN_DEBUG "%s: TKIP countermeasures: dropped "
-			       "TX packet to %pM\n",
-			       ieee->dev->name, header->addr1);
+			netdev_dbg(ieee->dev, "TKIP countermeasures: dropped "
+			       "TX packet to %pM\n", header->addr1);
 		}
 		return -1;
 	}
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
index 33c596f9ec96..17d9700dbf01 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
@@ -436,7 +436,7 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
 	if (ieee->reset_on_keychange &&
 	    ieee->iw_mode != IW_MODE_INFRA &&
 	    ieee->reset_port && ieee->reset_port(dev)) {
-		printk(KERN_DEBUG "%s: reset_port failed\n", dev->name);
+		netdev_dbg(ieee->dev, "reset_port failed\n");
 		return -EINVAL;
 	}
 	return 0;
---
2.20.1

[PATCH 2/4] Staging: rtl8192u: ieee80211: Use netdev_warn() for network devices.

[Sam Muhammed]

Use netdev_warn() over printk().
netdev_warn() is specific for printing warning
messages for network devices, and preferable
over printk(KERN_WARNING ...).

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 .../rtl8192u/ieee80211/ieee80211_module.c       |  3 +--
 .../staging/rtl8192u/ieee80211/ieee80211_tx.c   | 17 +++++++----------
 .../staging/rtl8192u/ieee80211/ieee80211_wx.c   |  5 ++---
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index aa9dab8a4ae8..a5a1b14f5a40 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -68,8 +68,7 @@ static inline int ieee80211_networks_allocate(struct ieee80211_device *ieee)
 				 sizeof(struct ieee80211_network),
 				 GFP_KERNEL);
 	if (!ieee->networks) {
-		printk(KERN_WARNING "%s: Out of memory allocating beacons\n",
-		       ieee->dev->name);
+		netdev_warn(ieee->dev, "Out of memory allocating beacons\n");
 		return -ENOMEM;
 	}

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
index 95cea2f56149..44684b7fae72 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
@@ -556,16 +556,15 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	if ((!ieee->hard_start_xmit && !(ieee->softmac_features & IEEE_SOFTMAC_TX_QUEUE)) ||
 	   ((!ieee->softmac_data_hard_start_xmit && (ieee->softmac_features & IEEE_SOFTMAC_TX_QUEUE)))) {
-		printk(KERN_WARNING "%s: No xmit handler.\n",
-		       ieee->dev->name);
+		netdev_warn(ieee->dev, "No xmit handler.\n");
 		goto success;
 	}


 	if (likely(ieee->raw_tx == 0)) {
 		if (unlikely(skb->len < SNAP_SIZE + sizeof(u16))) {
-			printk(KERN_WARNING "%s: skb too small (%d).\n",
-			ieee->dev->name, skb->len);
+			netdev_warn(ieee->dev, "skb too small (%d).\n",
+				    skb->len);
 			goto success;
 		}

@@ -684,8 +683,7 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
 		 */
 		txb = ieee80211_alloc_txb(nr_frags, frag_size + ieee->tx_headroom, GFP_ATOMIC);
 		if (unlikely(!txb)) {
-			printk(KERN_WARNING "%s: Could not allocate TXB\n",
-			ieee->dev->name);
+			netdev_warn(ieee->dev, "Could not allocate TXB\n");
 			goto failed;
 		}
 		txb->encrypted = encrypt;
@@ -778,15 +776,14 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	} else {
 		if (unlikely(skb->len < sizeof(struct rtl_80211_hdr_3addr))) {
-			printk(KERN_WARNING "%s: skb too small (%d).\n",
-			ieee->dev->name, skb->len);
+			netdev_warn(ieee->dev, "skb too small (%d).\n",
+				    skb->len);
 			goto success;
 		}

 		txb = ieee80211_alloc_txb(1, skb->len, GFP_ATOMIC);
 		if (!txb) {
-			printk(KERN_WARNING "%s: Could not allocate TXB\n",
-			ieee->dev->name);
+			netdev_warn(ieee->dev, "Could not allocate TXB\n");
 			goto failed;
 		}

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
index 17d9700dbf01..22373c0afebc 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
@@ -356,9 +356,8 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
 			kfree(new_crypt);
 			new_crypt = NULL;

-			printk(KERN_WARNING "%s: could not initialize WEP: "
-			       "load module ieee80211_crypt_wep\n",
-			       dev->name);
+			netdev_warn(dev, "could not initialize WEP: "
+				    "load module ieee80211_crypt_wep\n");
 			return -EOPNOTSUPP;
 		}
 		*crypt = new_crypt;
---
2.20.1

[PATCH 3/4] Staging: rtl8192u: ieee80211: Use netdev_info() with network devices.

[Sam Muhammed]

netdev_info() should be used instead of printk(KERN_INFO ...)
since it's specific to and preferable for printing messages
for network devices.

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 .../rtl8192u/ieee80211/ieee80211_softmac.c    | 28 +++++++++++--------
 .../rtl8192u/ieee80211/ieee80211_softmac_wx.c |  4 +--
 .../staging/rtl8192u/ieee80211/ieee80211_tx.c |  4 +--
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index 90692db81b71..1e79bc771fce 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -1270,14 +1270,15 @@ static void ieee80211_associate_step2(struct ieee80211_device *ieee)
 static void ieee80211_associate_complete_wq(struct work_struct *work)
 {
 	struct ieee80211_device *ieee = container_of(work, struct ieee80211_device, associate_complete_wq);
-	printk(KERN_INFO "Associated successfully\n");
+
+	netdev_info(ieee->dev, "Associated successfully\n");
 	if (ieee80211_is_54g(&ieee->current_network) &&
 	    (ieee->modulation & IEEE80211_OFDM_MODULATION)) {
 		ieee->rate = 108;
-		printk(KERN_INFO"Using G rates:%d\n", ieee->rate);
+		netdev_info(ieee->dev, "Using G rates:%d\n", ieee->rate);
 	} else {
 		ieee->rate = 22;
-		printk(KERN_INFO"Using B rates:%d\n", ieee->rate);
+		netdev_info(ieee->dev, "Using B rates:%d\n", ieee->rate);
 	}
 	if (ieee->pHTInfo->bCurrentHTSupport && ieee->pHTInfo->bEnableHT) {
 		printk("Successfully associated, ht enabled\n");
@@ -1391,12 +1392,13 @@ inline void ieee80211_softmac_new_net(struct ieee80211_device *ieee, struct ieee

 			strncpy(ieee->current_network.ssid, tmp_ssid, IW_ESSID_MAX_SIZE);
 			ieee->current_network.ssid_len = tmp_ssid_len;
-			printk(KERN_INFO"Linking with %s,channel:%d, qos:%d, myHT:%d, networkHT:%d\n",
-			       ieee->current_network.ssid,
-			       ieee->current_network.channel,
-			       ieee->current_network.qos_data.supported,
-			       ieee->pHTInfo->bEnableHT,
-			       ieee->current_network.bssht.bdSupportHT);
+			netdev_info(ieee->dev,
+				    "Linking with %s,channel:%d, qos:%d, myHT:%d, networkHT:%d\n",
+				    ieee->current_network.ssid,
+				    ieee->current_network.channel,
+				    ieee->current_network.qos_data.supported,
+				    ieee->pHTInfo->bEnableHT,
+				    ieee->current_network.bssht.bdSupportHT);

 			//ieee->pHTInfo->IOTAction = 0;
 			HTResetIOTSetting(ieee->pHTInfo);
@@ -1421,11 +1423,13 @@ inline void ieee80211_softmac_new_net(struct ieee80211_device *ieee, struct ieee
 				    (ieee->modulation & IEEE80211_OFDM_MODULATION)) {
 					ieee->rate = 108;
 					ieee->SetWirelessMode(ieee->dev, IEEE_G);
-					printk(KERN_INFO"Using G rates\n");
+					netdev_info(ieee->dev,
+						    "Using G rates\n");
 				} else {
 					ieee->rate = 22;
 					ieee->SetWirelessMode(ieee->dev, IEEE_B);
-					printk(KERN_INFO"Using B rates\n");
+					netdev_info(ieee->dev,
+						    "Using B rates\n");
 				}
 				memset(ieee->dot11HTOperationalRateSet, 0, 16);
 				//HTSetConnectBwMode(ieee, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
@@ -1622,7 +1626,7 @@ ieee80211_rx_assoc_rq(struct ieee80211_device *ieee, struct sk_buff *skb)
 	if (assoc_rq_parse(skb, dest) != -1)
 		ieee80211_resp_to_assoc_rq(ieee, dest);

-	printk(KERN_INFO"New client associated: %pM\n", dest);
+	netdev_info(ieee->dev, "New client associated: %pM\n", dest);
 	//FIXME
 }

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac_wx.c
index b1baaa18b129..f434a26cdb2f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac_wx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac_wx.c
@@ -459,8 +459,8 @@ int ieee80211_wx_set_rawtx(struct ieee80211_device *ieee,
 	else
 		ieee->raw_tx = 0;

-	printk(KERN_INFO"raw TX is %s\n",
-	      ieee->raw_tx ? "enabled" : "disabled");
+	netdev_info(ieee->dev, "raw TX is %s\n",
+		    ieee->raw_tx ? "enabled" : "disabled");

 	if (ieee->iw_mode == IW_MODE_MONITOR) {
 		if (prev == 0 && ieee->raw_tx) {
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
index 44684b7fae72..0ee054d82832 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
@@ -203,8 +203,8 @@ int ieee80211_encrypt_fragment(

 	atomic_dec(&crypt->refcnt);
 	if (res < 0) {
-		printk(KERN_INFO "%s: Encryption failed: len=%d.\n",
-		       ieee->dev->name, frag->len);
+		netdev_info(ieee->dev, "Encryption failed: len=%d.\n",
+			    frag->len);
 		ieee->ieee_stats.tx_discards++;
 		return -1;
 	}
---
2.20.1

[PATCH 4/4] Staging: rtl8192u: ieee80211: Use netdev_alert().

[Sam Muhammed]

Replace printk(KERN_ALERT ...) with netdev_alert()
when a network device is available.

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index 1e79bc771fce..d8eb907ff301 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -131,7 +131,7 @@ static void ieee80211_TURBO_Info(struct ieee80211_device *ieee, u8 **tag_p)
 	*tag++ = 0x00;

 	*tag_p = tag;
-	printk(KERN_ALERT "This is enable turbo mode IE process\n");
+	netdev_alert(ieee->dev, "This is enable turbo mode IE process\n");
 }
 #endif

---
2.20.1

Re: [Outreachy kernel] [PATCH 1/4] Staging: rtl8192u: ieee80211: Use netdev_dbg() for debug messages.

[Stefano Brivio]

On Thu, 26 Mar 2020 11:02:35 -0400
Sam Muhammed <jane.pnx9@gmail.com> wrote:

> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> @@ -436,7 +436,7 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
>  	if (ieee->reset_on_keychange &&
>  	    ieee->iw_mode != IW_MODE_INFRA &&
>  	    ieee->reset_port && ieee->reset_port(dev)) {
> -		printk(KERN_DEBUG "%s: reset_port failed\n", dev->name);
> +		netdev_dbg(ieee->dev, "reset_port failed\n");

I guess you should stick to 'dev' here -- it's the same as ieee->dev,
but reset_port() is invoked on 'dev', so it would be less confusing.

-- 
Stefano

Re: [Outreachy kernel] [PATCH 1/4] Staging: rtl8192u: ieee80211: Use netdev_dbg() for debug messages.

[Sam Muhammed]

On Mon, 2020-03-30 at 09:59 +0200, Stefano Brivio wrote:
> On Thu, 26 Mar 2020 11:02:35 -0400
> Sam Muhammed <jane.pnx9@gmail.com> wrote:
> 
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > @@ -436,7 +436,7 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
> >  	if (ieee->reset_on_keychange &&
> >  	    ieee->iw_mode != IW_MODE_INFRA &&
> >  	    ieee->reset_port && ieee->reset_port(dev)) {
> > -		printk(KERN_DEBUG "%s: reset_port failed\n", dev->name);
> > +		netdev_dbg(ieee->dev, "reset_port failed\n");
> 
> I guess you should stick to 'dev' here -- it's the same as ieee->dev,
> but reset_port() is invoked on 'dev', so it would be less confusing.
> 

Thank You for pointing this out! this got me hesitant but i thought i
could be more explicit?

Now since this change is already been made, i guess i should make this
a new patch?

is there any information i should say to tie it with this series? or
should be treated like a new separate patch? 

Thank You.

Sam

> -- 
> Stefano
> 

Re: [Outreachy kernel] [PATCH 1/4] Staging: rtl8192u: ieee80211: Use netdev_dbg() for debug messages.

[Stefano Brivio]

On Mon, 30 Mar 2020 09:01:19 -0400
Sam Muhammed <jane.pnx9@gmail.com> wrote:

> On Mon, 2020-03-30 at 09:59 +0200, Stefano Brivio wrote:
> > On Thu, 26 Mar 2020 11:02:35 -0400
> > Sam Muhammed <jane.pnx9@gmail.com> wrote:
> >   
> > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > > @@ -436,7 +436,7 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
> > >  	if (ieee->reset_on_keychange &&
> > >  	    ieee->iw_mode != IW_MODE_INFRA &&
> > >  	    ieee->reset_port && ieee->reset_port(dev)) {
> > > -		printk(KERN_DEBUG "%s: reset_port failed\n", dev->name);
> > > +		netdev_dbg(ieee->dev, "reset_port failed\n");  
> > 
> > I guess you should stick to 'dev' here -- it's the same as ieee->dev,
> > but reset_port() is invoked on 'dev', so it would be less confusing.
> 
> Thank You for pointing this out! this got me hesitant but i thought i
> could be more explicit?

Hmm, I see. Look, these would be my points, in order of priority:

1. ieee->reset_port(dev), port reset failed on dev, nothing else

2. it's nice to have a 'dev' pointer representing the device instead of
   having to spell out ieee->dev every time in this function,
   especially as the container is called 'ieee', which has a very weak
   meaning

2. having a 'dev' variable is useless, ieee->dev leaves no room for
   interpretation and wondering (when was 'dev' assigned? where does it
   come from?)

So if I understand correctly you are referring to the second "2."
point. About those two points, I really have no preference. I also
couldn't decide myself. I guess I'd instinctively go for "ieee->dev"
everywhere.

However, I guess point 1. is arguably the most important here.

> Now since this change is already been made, i guess i should make this
> a new patch?

Ouch, I missed that. Yes.

> is there any information i should say to tie it with this series? or
> should be treated like a new separate patch?

It's a separate patch. You can use the Fixes: tag as documented in
submitting-patches.rst, but this is really subject to interpretation.

My personal interpretation is that it's not a functional fix and the
Fixes: tag should be omitted, even though you should refer to the merged
commit in the message of the new change.

Some maintainers want it no matter what. I don't recall having ever
sent a patch directly to Greg, so I don't know :) Try without :)

-- 
Stefano