[PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[Paulo Miguel Almeida]

Hi,

I'm planning to submit this cleanup patch but I would appreciate if some
of you could consider reviewing it first as this is my first patch.

Also, when it comes to whom to send it to, this is the list I got so far but
please let me know if I should either add or exclude anyone from this
list so that I send this patch to the right people while not spamming others.

./scripts/get_maintainer.pl /tmp/0001-staging-rtl8192u-specify-printk-s-KERN_-LEVEL-in-iee.patch 
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:STAGING SUBSYSTEM,commit_signer:17/18=94%)
Puranjay Mohan <puranjay12@gmail.com> (commit_signer:4/18=22%,authored:4/18=22%)
Stephen Brennan <stephen@brennan.io> (commit_signer:4/18=22%,authored:4/18=22%,added_lines:297/366=81%,removed_lines:388/470=83%)
Bhanusree Pola <bhanusreemahesh@gmail.com> (commit_signer:3/18=17%,authored:3/18=17%)
Sanjana Sanikommu <sanjana99reddy99@gmail.com> (commit_signer:2/18=11%,authored:2/18=11%)
Vatsala Narang <vatsalanarang@gmail.com> (authored:1/18=6%)
devel@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
linux-kernel@vger.kernel.org (open list)

The origin commit message follows

Checkpatch reports 'WARNING: printk() should include KERN_<LEVEL>
facility level'. Fix this by specifying a relevant KERN_<LEVEL> value
for each line in which it was missing.

Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
 .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 00fea127bdc3..f38986d74005 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -810,11 +810,11 @@ static u8 parse_subframe(struct sk_buff *skb,
 			nSubframe_Length = (nSubframe_Length >> 8) + (nSubframe_Length << 8);
 
 			if (skb->len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
-				printk("%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",\
+				printk(KERN_DEBUG "%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",
 						__func__, rxb->nr_subframes);
-				printk("%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
-				printk("nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
-				printk("The Packet SeqNum is %d\n", SeqNum);
+				printk(KERN_DEBUG "%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
+				printk(KERN_DEBUG "nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
+				printk(KERN_DEBUG "The Packet SeqNum is %d\n", SeqNum);
 				return 0;
 			}
 
@@ -919,7 +919,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 
 	if (HTCCheck(ieee, skb->data)) {
 		if (net_ratelimit())
-			printk("find HTCControl\n");
+			printk(KERN_WARNING "find HTCControl\n");
 		hdrlen += 4;
 		rx_stats->bContainHTC = true;
 	}
@@ -1113,7 +1113,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 
 	if (ieee->host_decrypt && (fc & IEEE80211_FCTL_WEP) &&
 	    (keyidx = ieee80211_rx_frame_decrypt(ieee, skb, crypt)) < 0) {
-		printk("decrypt frame error\n");
+		printk(KERN_WARNING "decrypt frame error\n");
 		goto rx_dropped;
 	}
 
@@ -1178,7 +1178,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 	 * encrypted/authenticated */
 	if (ieee->host_decrypt && (fc & IEEE80211_FCTL_WEP) &&
 	    ieee80211_rx_frame_decrypt_msdu(ieee, skb, keyidx, crypt)) {
-		printk("==>decrypt msdu error\n");
+		printk(KERN_WARNING "==>decrypt msdu error\n");
 		goto rx_dropped;
 	}
 
@@ -1863,7 +1863,7 @@ int ieee80211_parse_info_param(struct ieee80211_device *ieee,
 				info_element->data[0] == 0x00 &&
 				info_element->data[1] == 0x13 &&
 				info_element->data[2] == 0x74)) {
-				printk("========>%s(): athros AP is exist\n", __func__);
+				printk(KERN_DEBUG "========>%s(): athros AP is exist\n", __func__);
 				network->atheros_cap_exist = true;
 			} else
 				network->atheros_cap_exist = false;
@@ -2357,14 +2357,14 @@ static inline void ieee80211_process_probe_response(
 			if (IS_COUNTRY_IE_VALID(ieee)) {
 				// Case 1: Country code
 				if (!is_legal_channel(ieee, network->channel)) {
-					printk("GetScanInfo(): For Country code, filter probe response at channel(%d).\n", network->channel);
+					printk(KERN_WARNING "GetScanInfo(): For Country code, filter probe response at channel(%d).\n", network->channel);
 					goto out;
 				}
 			} else {
 				// Case 2: No any country code.
 				// Filter over channel ch12~14
 				if (network->channel > 11) {
-					printk("GetScanInfo(): For Global Domain, filter probe response at channel(%d).\n", network->channel);
+					printk(KERN_WARNING "GetScanInfo(): For Global Domain, filter probe response at channel(%d).\n", network->channel);
 					goto out;
 				}
 			}
@@ -2372,14 +2372,14 @@ static inline void ieee80211_process_probe_response(
 			if (IS_COUNTRY_IE_VALID(ieee)) {
 				// Case 1: Country code
 				if (!is_legal_channel(ieee, network->channel)) {
-					printk("GetScanInfo(): For Country code, filter beacon at channel(%d).\n", network->channel);
+					printk(KERN_WARNING "GetScanInfo(): For Country code, filter beacon at channel(%d).\n", network->channel);
 					goto out;
 				}
 			} else {
 				// Case 2: No any country code.
 				// Filter over channel ch12~14
 				if (network->channel > 14) {
-					printk("GetScanInfo(): For Global Domain, filter beacon at channel(%d).\n", network->channel);
+					printk(KERN_WARNING "GetScanInfo(): For Global Domain, filter beacon at channel(%d).\n", network->channel);
 					goto out;
 				}
 			}
-- 
2.24.1


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[Greg KH]

On Mon, Jan 13, 2020 at 06:02:47PM +1300, Paulo Miguel Almeida wrote:
> Hi,
> 
> I'm planning to submit this cleanup patch but I would appreciate if some
> of you could consider reviewing it first as this is my first patch.
> 
> Also, when it comes to whom to send it to, this is the list I got so far but
> please let me know if I should either add or exclude anyone from this
> list so that I send this patch to the right people while not spamming others.
> 
> ./scripts/get_maintainer.pl /tmp/0001-staging-rtl8192u-specify-printk-s-KERN_-LEVEL-in-iee.patch 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:STAGING SUBSYSTEM,commit_signer:17/18=94%)
> Puranjay Mohan <puranjay12@gmail.com> (commit_signer:4/18=22%,authored:4/18=22%)
> Stephen Brennan <stephen@brennan.io> (commit_signer:4/18=22%,authored:4/18=22%,added_lines:297/366=81%,removed_lines:388/470=83%)
> Bhanusree Pola <bhanusreemahesh@gmail.com> (commit_signer:3/18=17%,authored:3/18=17%)
> Sanjana Sanikommu <sanjana99reddy99@gmail.com> (commit_signer:2/18=11%,authored:2/18=11%)
> Vatsala Narang <vatsalanarang@gmail.com> (authored:1/18=6%)
> devel@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
> linux-kernel@vger.kernel.org (open list)

Looks correct.

> The origin commit message follows
> 
> Checkpatch reports 'WARNING: printk() should include KERN_<LEVEL>
> facility level'. Fix this by specifying a relevant KERN_<LEVEL> value
> for each line in which it was missing.
> 
> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
>  .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> index 00fea127bdc3..f38986d74005 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> @@ -810,11 +810,11 @@ static u8 parse_subframe(struct sk_buff *skb,
>  			nSubframe_Length = (nSubframe_Length >> 8) + (nSubframe_Length << 8);
>  
>  			if (skb->len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
> -				printk("%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",\
> +				printk(KERN_DEBUG "%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",
>  						__func__, rxb->nr_subframes);
> -				printk("%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> -				printk("nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> -				printk("The Packet SeqNum is %d\n", SeqNum);
> +				printk(KERN_DEBUG "%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> +				printk(KERN_DEBUG "nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> +				printk(KERN_DEBUG "The Packet SeqNum is %d\n", SeqNum);

As this is a driver, you really should be using dev_dbg() for this, or
better yet, netdev_dbg() as I think checkpatch will ask you to do.

Same for all of the other conversions in this file, try using a "better"
thing than printk.

But you are on the right track, nice job.

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[Bjørn Mork]

Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com> writes:

> Checkpatch reports 'WARNING: printk() should include KERN_<LEVEL>
> facility level'. Fix this by specifying a relevant KERN_<LEVEL> value
> for each line in which it was missing.


Although this is true, there are also additional best practice rules wrt
use of printk in drivers and debug level printks in particular.
Checkpatch does not tell you everything, unfortunately ;-)

You should always use dev_dbg() or netif_dbg() or similar macros instead
of printk in drivers.  This way debug messages can be compiled away when
not needed, or even dynamically enabled/disabled on kernels built with
dynamic debugging.  You should also drop stuff like __func__ since that
can be enabled dynamically as necessary with dev_dbg().

Take a look at
https://www.kernel.org/doc/html/v5.4/admin-guide/dynamic-debug-howto.html
and play it if you haven't already.  This an extremely useful feature.

See some of the drivers in drivers/net/wireless for examples of how to
use dev_dbg().


> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
>  .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> index 00fea127bdc3..f38986d74005 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> @@ -810,11 +810,11 @@ static u8 parse_subframe(struct sk_buff *skb,
>  			nSubframe_Length = (nSubframe_Length >> 8) + (nSubframe_Length << 8);
>  
>  			if (skb->len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
> -				printk("%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",\
> +				printk(KERN_DEBUG "%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",
>  						__func__, rxb->nr_subframes);
> -				printk("%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> -				printk("nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> -				printk("The Packet SeqNum is %d\n", SeqNum);
> +				printk(KERN_DEBUG "%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> +				printk(KERN_DEBUG "nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> +				printk(KERN_DEBUG "The Packet SeqNum is %d\n", SeqNum);
>  				return 0;
>  			}
>  

I see that this function, and many others in this driver, does not
access any device specific data.  So you'll probably have to do
something about that.  A bit more work required here than just setting
the printk level.


Hmm... I was going to suggest that you looked at the driver's TODO file
just to make sure that this work isn't futile e.g because it conflicts
with planned/suggested driver restructuring.  But I see that the TODO
file is missing.  Weird.  I thought it was required for all staging
drivers.  Learning something new every day...



Bjørn

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[Paulo Miguel Almeida]

On Mon, Jan 13, 2020 at 09:53:23AM +0100, Bjørn Mork wrote:
> Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com> writes:
> 
> > Checkpatch reports 'WARNING: printk() should include KERN_<LEVEL>
> > facility level'. Fix this by specifying a relevant KERN_<LEVEL> value
> > for each line in which it was missing.
> 
> 
> Although this is true, there are also additional best practice rules wrt
> use of printk in drivers and debug level printks in particular.
> Checkpatch does not tell you everything, unfortunately ;-)
> 
> You should always use dev_dbg() or netif_dbg() or similar macros instead
> of printk in drivers.  This way debug messages can be compiled away when
> not needed, or even dynamically enabled/disabled on kernels built with
> dynamic debugging.  You should also drop stuff like __func__ since that
> can be enabled dynamically as necessary with dev_dbg().

Done that in newer version of the patch. I also removed references to 
netdevice->name as this is populated automatically when using netdev_*
(Correct me if I'm wrong pleae)

https://elixir.bootlin.com/linux/v5.4.11/source/net/core/dev.c#L9980
https://elixir.bootlin.com/linux/v5.4.11/source/include/linux/netdevice.h#L4682

> 
> Take a look at
> https://www.kernel.org/doc/html/v5.4/admin-guide/dynamic-debug-howto.html
> and play it if you haven't already.  This an extremely useful feature.

Fascinating! I hand't come across this before. Thanks :)

> 
> See some of the drivers in drivers/net/wireless for examples of how to
> use dev_dbg().
> 
> 
> > Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> > ---
> >  .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 24 +++++++++----------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> > index 00fea127bdc3..f38986d74005 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> > @@ -810,11 +810,11 @@ static u8 parse_subframe(struct sk_buff *skb,
> >  			nSubframe_Length = (nSubframe_Length >> 8) + (nSubframe_Length << 8);
> >  
> >  			if (skb->len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
> > -				printk("%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",\
> > +				printk(KERN_DEBUG "%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",
> >  						__func__, rxb->nr_subframes);
> > -				printk("%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> > -				printk("nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> > -				printk("The Packet SeqNum is %d\n", SeqNum);
> > +				printk(KERN_DEBUG "%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
> > +				printk(KERN_DEBUG "nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
> > +				printk(KERN_DEBUG "The Packet SeqNum is %d\n", SeqNum);
> >  				return 0;
> >  			}
> >  
> 
> I see that this function, and many others in this driver, does not
> access any device specific data.  So you'll probably have to do
> something about that.  A bit more work required here than just setting
> the printk level.
 
What I did in this new patch was to receive an additional parameter on
the parse_subframe method so it could have access to a netdevice ref.

Do you agree with this approach or do you suggest me to address it 
differently?
> 
> 
> Hmm... I was going to suggest that you looked at the driver's TODO file
> just to make sure that this work isn't futile e.g because it conflicts
> with planned/suggested driver restructuring.  But I see that the TODO
> file is missing.  Weird.  I thought it was required for all staging
> drivers.  Learning something new every day...
> 
> 
> 
> Bjørn


Hi Greg, Hi Bjørn,

I appreciate the comments that both of you made regarding my original patch.

I implemented those suggestions, however, this time I replaced also the existing 
printk calls (with dbg lvl set) to keep it consistent.  

Please let me know if you think I should change anything else.


The original commit message follows

Checkpatch reports 'WARNING: printk() should include KERN_<LEVEL>
facility level'. Fix this by specifying a relevant KERN_<LEVEL> value
for each line in which it was missing.

Once they are fixed, checkpatch reports 'WARNING: Prefer [subsystem eg:
netdev]_dbg([subsystem]dev, ... then dev_dbg(dev, ... then
pr_debug(...  to printk(KERN_DEBUG ...'. Fix this by replacing
relevant printk_<level> statements with their netdev_<level>
equivalent.

Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
 .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 62 +++++++++----------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 00fea127bdc3..e101f7b13c7e 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -232,8 +232,7 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, struct sk_buff *skb,

 	#ifdef NOT_YET
 	if (ieee->iw_mode == IW_MODE_MASTER) {
-		printk(KERN_DEBUG "%s: Master mode not yet supported.\n",
-		       ieee->dev->name);
+		netdev_dbg(ieee->dev, "Master mode not yet supported.\n");
 		return 0;
 /*
   hostap_update_sta_ps(ieee, (struct hostap_ieee80211_hdr_4addr *)
@@ -261,9 +260,9 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, struct sk_buff *skb,

 	    if (ieee->iw_mode == IW_MODE_MASTER) {
 		if (type != WLAN_FC_TYPE_MGMT && type != WLAN_FC_TYPE_CTRL) {
-			printk(KERN_DEBUG "%s: unknown management frame "
+			netdev_dbg(skb->dev, "unknown management frame "
 			       "(type=0x%02x, stype=0x%02x) dropped\n",
-			       skb->dev->name, type, stype);
+			       type, stype);
 			return -1;
 		}

@@ -271,8 +270,8 @@ ieee80211_rx_frame_mgmt(struct ieee80211_device *ieee, struct sk_buff *skb,
 		return 0;
 	}

-	printk(KERN_DEBUG "%s: hostap_rx_frame_mgmt: management frame "
-	       "received in non-Host AP mode\n", skb->dev->name);
+	netdev_dbg(skb->dev, "hostap_rx_frame_mgmt: management frame "
+	       "received in non-Host AP mode\n");
 	return -1;
 	#endif
 }
@@ -349,9 +348,9 @@ ieee80211_rx_frame_decrypt(struct ieee80211_device *ieee, struct sk_buff *skb,
 	if (ieee->tkip_countermeasures &&
 	    strcmp(crypt->ops->name, "TKIP") == 0) {
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "%s: TKIP countermeasures: dropped "
+			netdev_dbg(ieee->dev, "TKIP countermeasures: dropped "
 			       "received packet from %pM\n",
-			       ieee->dev->name, hdr->addr2);
+			       hdr->addr2);
 		}
 		return -1;
 	}
@@ -397,9 +396,9 @@ ieee80211_rx_frame_decrypt_msdu(struct ieee80211_device *ieee, struct sk_buff *s
 	res = crypt->ops->decrypt_msdu(skb, keyidx, hdrlen, crypt->priv);
 	atomic_dec(&crypt->refcnt);
 	if (res < 0) {
-		printk(KERN_DEBUG "%s: MSDU decryption/MIC verification failed"
+		netdev_dbg(ieee->dev, "MSDU decryption/MIC verification failed"
 		       " (SA=%pM keyidx=%d)\n",
-		       ieee->dev->name, hdr->addr2, keyidx);
+		       hdr->addr2, keyidx);
 		return -1;
 	}

@@ -749,7 +748,8 @@ static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
 	kfree(prxbIndicateArray);
 }

-static u8 parse_subframe(struct sk_buff *skb,
+static u8 parse_subframe(struct ieee80211_device *ieee,
+			 struct sk_buff *skb,
 			 struct ieee80211_rx_stats *rx_stats,
 			 struct ieee80211_rxb *rxb, u8 *src, u8 *dst)
 {
@@ -810,11 +810,11 @@ static u8 parse_subframe(struct sk_buff *skb,
 			nSubframe_Length = (nSubframe_Length >> 8) + (nSubframe_Length << 8);

 			if (skb->len < (ETHERNET_HEADER_SIZE + nSubframe_Length)) {
-				printk("%s: A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",\
-						__func__, rxb->nr_subframes);
-				printk("%s: A-MSDU parse error!! Subframe Length: %d\n", __func__, nSubframe_Length);
-				printk("nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
-				printk("The Packet SeqNum is %d\n", SeqNum);
+				netdev_dbg(ieee->dev, "A-MSDU parse error!! pRfd->nTotalSubframe : %d\n",
+					   rxb->nr_subframes);
+				netdev_dbg(ieee->dev, "A-MSDU parse error!! Subframe Length: %d\n", nSubframe_Length);
+				netdev_dbg(ieee->dev, "nRemain_Length is %d and nSubframe_Length is : %d\n", skb->len, nSubframe_Length);
+				netdev_dbg(ieee->dev, "The Packet SeqNum is %d\n", SeqNum);
 				return 0;
 			}

@@ -904,8 +904,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 	stats = &ieee->stats;

 	if (skb->len < 10) {
-		printk(KERN_INFO "%s: SKB length < 10\n",
-		       dev->name);
+		netdev_info(dev, "SKB length < 10\n");
 		goto rx_dropped;
 	}

@@ -919,7 +918,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,

 	if (HTCCheck(ieee, skb->data)) {
 		if (net_ratelimit())
-			printk("find HTCControl\n");
+			netdev_warn(dev, "find HTCControl\n");
 		hdrlen += 4;
 		rx_stats->bContainHTC = true;
 	}
@@ -1113,7 +1112,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,

 	if (ieee->host_decrypt && (fc & IEEE80211_FCTL_WEP) &&
 	    (keyidx = ieee80211_rx_frame_decrypt(ieee, skb, crypt)) < 0) {
-		printk("decrypt frame error\n");
+		netdev_dbg(ieee->dev, "decrypt frame error\n");
 		goto rx_dropped;
 	}

@@ -1141,9 +1140,8 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 			flen -= hdrlen;

 		if (frag_skb->tail + flen > frag_skb->end) {
-			printk(KERN_WARNING "%s: host decrypted and "
-			       "reassembled frame did not fit skb\n",
-			       dev->name);
+			netdev_warn(dev, "host decrypted and "
+			       "reassembled frame did not fit skb\n");
 			ieee80211_frag_cache_invalidate(ieee, hdr);
 			goto rx_dropped;
 		}
@@ -1178,7 +1176,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 	 * encrypted/authenticated */
 	if (ieee->host_decrypt && (fc & IEEE80211_FCTL_WEP) &&
 	    ieee80211_rx_frame_decrypt_msdu(ieee, skb, keyidx, crypt)) {
-		printk("==>decrypt msdu error\n");
+		netdev_dbg(ieee->dev, "==>decrypt msdu error\n");
 		goto rx_dropped;
 	}

@@ -1250,7 +1248,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 		goto rx_dropped;
 	/* to parse amsdu packets */
 	/* qos data packets & reserved bit is 1 */
-	if (parse_subframe(skb, rx_stats, rxb, src, dst) == 0) {
+	if (parse_subframe(ieee, skb, rx_stats, rxb, src, dst) == 0) {
 		/* only to free rxb, and not submit the packets to upper layer */
 		for (i = 0; i < rxb->nr_subframes; i++) {
 			dev_kfree_skb(rxb->subframes[i]);
@@ -1863,7 +1861,7 @@ int ieee80211_parse_info_param(struct ieee80211_device *ieee,
 				info_element->data[0] == 0x00 &&
 				info_element->data[1] == 0x13 &&
 				info_element->data[2] == 0x74)) {
-				printk("========>%s(): athros AP is exist\n", __func__);
+				netdev_dbg(ieee->dev, "========> athros AP is exist\n");
 				network->atheros_cap_exist = true;
 			} else
 				network->atheros_cap_exist = false;
@@ -1980,8 +1978,8 @@ int ieee80211_parse_info_param(struct ieee80211_device *ieee,
 			}
 			break;
 		case MFIE_TYPE_QOS_PARAMETER:
-			printk(KERN_ERR
-			       "QoS Error need to parse QOS_PARAMETER IE\n");
+			netdev_err(ieee->dev,
+				   "QoS Error need to parse QOS_PARAMETER IE\n");
 			break;

 		case MFIE_TYPE_COUNTRY:
@@ -2357,14 +2355,14 @@ static inline void ieee80211_process_probe_response(
 			if (IS_COUNTRY_IE_VALID(ieee)) {
 				// Case 1: Country code
 				if (!is_legal_channel(ieee, network->channel)) {
-					printk("GetScanInfo(): For Country code, filter probe response at channel(%d).\n", network->channel);
+					netdev_warn(ieee->dev, "GetScanInfo(): For Country code, filter probe response at channel(%d).\n", network->channel);
 					goto out;
 				}
 			} else {
 				// Case 2: No any country code.
 				// Filter over channel ch12~14
 				if (network->channel > 11) {
-					printk("GetScanInfo(): For Global Domain, filter probe response at channel(%d).\n", network->channel);
+					netdev_warn(ieee->dev, "GetScanInfo(): For Global Domain, filter probe response at channel(%d).\n", network->channel);
 					goto out;
 				}
 			}
@@ -2372,14 +2370,14 @@ static inline void ieee80211_process_probe_response(
 			if (IS_COUNTRY_IE_VALID(ieee)) {
 				// Case 1: Country code
 				if (!is_legal_channel(ieee, network->channel)) {
-					printk("GetScanInfo(): For Country code, filter beacon at channel(%d).\n", network->channel);
+					netdev_warn(ieee->dev, "GetScanInfo(): For Country code, filter beacon at channel(%d).\n", network->channel);
 					goto out;
 				}
 			} else {
 				// Case 2: No any country code.
 				// Filter over channel ch12~14
 				if (network->channel > 14) {
-					printk("GetScanInfo(): For Global Domain, filter beacon at channel(%d).\n", network->channel);
+					netdev_warn(ieee->dev, "GetScanInfo(): For Global Domain, filter beacon at channel(%d).\n", network->channel);
 					goto out;
 				}
 			}
--
2.24.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] staging: rtl8192u: specify printk's KERN_<LEVEL> in ieee80211

[Greg KH]

On Wed, Jan 15, 2020 at 04:47:47PM +1300, Paulo Miguel Almeida wrote:
> Please let me know if you think I should change anything else.

Submit it the "normal" way and let the developers there review it all.
That's how everyone else works, no need to worry about having a patch
"pre-reviewed" at all.

And at a quick glance, looks good to me :)

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies