[PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Larry Finger]

As soon as debugging is turned on, the logs are filled with messages
reporting the interrupt status. As this quantity is usually zero, this
output is not needed. In fact, there will be a report if the status is
not zero, thus the debug line in question could probably be deleted.
Rather than taking that action, I have changed it to only be printed
when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 9f6dbb4..236f33c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5260,7 +5260,8 @@ static void rtl8xxxu_int_complete(struct urb *urb)
 	struct device *dev = &priv->udev->dev;
 	int ret;
 
-	dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
+	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_USB)
+		dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
 	if (urb->status == 0) {
 		usb_anchor_urb(urb, &priv->int_anchor);
 		ret = usb_submit_urb(urb, GFP_ATOMIC);
-- 
2.6.6

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Joe Perches]

On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
> As soon as debugging is turned on, the logs are filled with messages
> reporting the interrupt status. As this quantity is usually zero, this
> output is not needed. In fact, there will be a report if the status is
> not zero, thus the debug line in question could probably be deleted.
> Rather than taking that action, I have changed it to only be printed
> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.

There are many uses of
	if (rtl8xxxu_debug & <DEFINE>) {
		dev_info(dev, ...)

Emitting debugging information at KERN_INFO is odd.

I think it'd be nicer to use dev_dbg for all these cases
and as well use some new macro that includes the test

Something like:

#define rtl8xxxu_dbg(type, fmt, ...)			\
do {							\
	if (rtl8xxxu_debug & (type))			\
		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
} while (0)

> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 9f6dbb4..236f33c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5260,7 +5260,8 @@ static void rtl8xxxu_int_complete(struct urb *urb)
>  	struct device *dev = &priv->udev->dev;
>  	int ret;
>  
> -	dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
> +	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_USB)
> +		dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
>  	if (urb->status == 0) {
>  		usb_anchor_urb(urb, &priv->int_anchor);
>  		ret = usb_submit_urb(urb, GFP_ATOMIC);
> 
> 

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Jes Sorensen]

Joe Perches <joe@perches.com> writes:
> On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
>> As soon as debugging is turned on, the logs are filled with messages
>> reporting the interrupt status. As this quantity is usually zero, this
>> output is not needed. In fact, there will be a report if the status is
>> not zero, thus the debug line in question could probably be deleted.
>> Rather than taking that action, I have changed it to only be printed
>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>
> There are many uses of
> 	if (rtl8xxxu_debug & <DEFINE>) {
> 		dev_info(dev, ...)
>
> Emitting debugging information at KERN_INFO is odd.

Not at all, it's a pain to enable it in debug fs post loading the
driver, especially if you need the output immediately during driver
init. That is why the flags are there.

> I think it'd be nicer to use dev_dbg for all these cases
> and as well use some new macro that includes the test
>
> Something like:
>
> #define rtl8xxxu_dbg(type, fmt, ...)			\
> do {							\
> 	if (rtl8xxxu_debug & (type))			\
> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
> } while (0)

Yuck yuck yuck, no thanks!

Any attempt of adding that kinda grossness to the driver will get a
NACK.

There is a reason the debug flag is there - it allows you to enable
specific items on driver load. If you enable that flag, expect to see
stuff in your log.

Jes

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Kalle Valo]

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> Joe Perches <joe@perches.com> writes:
>> On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
>>> As soon as debugging is turned on, the logs are filled with messages
>>> reporting the interrupt status. As this quantity is usually zero, this
>>> output is not needed. In fact, there will be a report if the status is
>>> not zero, thus the debug line in question could probably be deleted.
>>> Rather than taking that action, I have changed it to only be printed
>>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>>
>> There are many uses of
>> 	if (rtl8xxxu_debug & <DEFINE>) {
>> 		dev_info(dev, ...)
>>
>> Emitting debugging information at KERN_INFO is odd.
>
> Not at all, it's a pain to enable it in debug fs post loading the
> driver, especially if you need the output immediately during driver
> init. That is why the flags are there.
>
>> I think it'd be nicer to use dev_dbg for all these cases
>> and as well use some new macro that includes the test
>>
>> Something like:
>>
>> #define rtl8xxxu_dbg(type, fmt, ...)			\
>> do {							\
>> 	if (rtl8xxxu_debug & (type))			\
>> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
>> } while (0)
>
> Yuck yuck yuck, no thanks!
>
> Any attempt of adding that kinda grossness to the driver will get a
> NACK.

Huh, how is that ugly? To me it's the opposite, original code is ugly
and Joes' proposal makes sense. Lots of wireless drivers have something
similar.

-- 
Kalle Valo

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Jes Sorensen]

Kalle Valo <kvalo@codeaurora.org> writes:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>
>> Joe Perches <joe@perches.com> writes:
>>> I think it'd be nicer to use dev_dbg for all these cases
>>> and as well use some new macro that includes the test
>>>
>>> Something like:
>>>
>>> #define rtl8xxxu_dbg(type, fmt, ...)			\
>>> do {							\
>>> 	if (rtl8xxxu_debug & (type))			\
>>> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
>>> } while (0)
>>
>> Yuck yuck yuck, no thanks!
>>
>> Any attempt of adding that kinda grossness to the driver will get a
>> NACK.
>
> Huh, how is that ugly? To me it's the opposite, original code is ugly
> and Joes' proposal makes sense. Lots of wireless drivers have something
> similar.

Sorry it's a classic case of obfuscating the code for zero gain. If
someone else likes this kinda wrapper in their code, by all means go
ahead. In my book it's just bad coding taste.

Jes

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Jes Sorensen]

Larry Finger <Larry.Finger@lwfinger.net> writes:
> As soon as debugging is turned on, the logs are filled with messages
> reporting the interrupt status. As this quantity is usually zero, this
> output is not needed. In fact, there will be a report if the status is
> not zero, thus the debug line in question could probably be deleted.
> Rather than taking that action, I have changed it to only be printed
> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.

Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
that.

Which device do you see this with?

Thanks,
Jes

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Larry Finger]

On 09/17/2016 03:59 PM, Jes Sorensen wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>> As soon as debugging is turned on, the logs are filled with messages
>> reporting the interrupt status. As this quantity is usually zero, this
>> output is not needed. In fact, there will be a report if the status is
>> not zero, thus the debug line in question could probably be deleted.
>> Rather than taking that action, I have changed it to only be printed
>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>
> Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
> that.
>
> Which device do you see this with?

OK. I will change the flag.

I found this with a TP-Link TL-MN8200ND, which has some variant of the RTL8188CU 
chip. It transmits, but I see no evidence that the receiver is functioning at 
all. The same is true for driver rtl8192cu. Only the driver from Realtek's web 
site actually works.

One other problem that I have found is that the debug option on module load 
seems to be ignored. So far, I've had to hard wire the flags. Once I find the 
reason, I'll send a patch for that as well.

Larry

Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

[Jes Sorensen]

Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 09/17/2016 03:59 PM, Jes Sorensen wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>> As soon as debugging is turned on, the logs are filled with messages
>>> reporting the interrupt status. As this quantity is usually zero, this
>>> output is not needed. In fact, there will be a report if the status is
>>> not zero, thus the debug line in question could probably be deleted.
>>> Rather than taking that action, I have changed it to only be printed
>>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>>
>> Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
>> that.
>>
>> Which device do you see this with?
>
> OK. I will change the flag.
>
> I found this with a TP-Link TL-MN8200ND, which has some variant of the
> RTL8188CU chip. It transmits, but I see no evidence that the receiver
> is functioning at all. The same is true for driver rtl8192cu. Only the
> driver from Realtek's web site actually works.

I assume you mean TL-WN8200ND? That device is 'interesting' in the least
positive sense of the word. It seems abandoned by the manufacturer
too. I have one of them but never managed to get it working, not with
any driver under Linux nor Windows.

TP-Link shipped a driver disc with it, but you cannot install that in
any recent version of Windows because the OS ships with it's own driver
for the 8192cu/8188cu series and the device uses the common USB ID. I
have been meaning to see if I could find a box with Vista on it to
install their driver and run a USB trace on it.

> One other problem that I have found is that the debug option on module
> load seems to be ignored. So far, I've had to hard wire the
> flags. Once I find the reason, I'll send a patch for that as well.

That is odd - I use it regularly and haven't had problems with it.

Cheers,
Jes