[PATCH] Silence r8712u info and add verbose option v2

[PATCH] Silence r8712u info and add verbose option v2

[Alexandre Demers]

r8712u pollutes dmesg and logs. Silence it and add a verbose option to KConfig if we ever 
really want to hear about it.

v2: keep netdev_info instead of replacing it by a KERN_NOTICE

---
 drivers/staging/rtl8712/Kconfig        | 7 +++++++
 drivers/staging/rtl8712/rtl871x_mlme.c | 7 ++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/Kconfig b/drivers/staging/rtl8712/Kconfig
index 6a43312..7cae8f8 100644
--- a/drivers/staging/rtl8712/Kconfig
+++ b/drivers/staging/rtl8712/Kconfig
@@ -16,4 +16,11 @@ config R8712_TX_AGGR
 	---help---
 	This option provides transmit aggregation for the Realtek RTL8712 USB device.
 
+config R8712U_DEBUG
+	bool "Realtek RTL8712U Wireless verbose debug"
+	depends on R8712U
+	help
+	  Say Y here in order to have the rtl8712u code generate
+	  verbose debugging messages.
+
 
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 6596154..1226009 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1043,9 +1043,12 @@ void r8712_got_addbareq_event_callback(struct _adapter *adapter, u8 *pbuf)
 	struct	sta_priv *pstapriv = &adapter->stapriv;
 	struct	recv_reorder_ctrl *precvreorder_ctrl = NULL;
 
+#ifdef CONFIG_R8712U_DEBUG
	netdev_info(adapter->pnetdev, "%s: mac = %pM, seq = %d, tid = %d\n",
		    __func__, pAddbareq_pram->MacAddress,
	    pAddbareq_pram->StartSeqNum, pAddbareq_pram->tid);
+#endif
+
 	psta = r8712_get_stainfo(pstapriv, pAddbareq_pram->MacAddress);
 	if (psta) {
 		precvreorder_ctrl =
-- 
1.8.4.2

Re: [PATCH] Silence r8712u info and add verbose option v2

[Larry Finger]

On 11/12/2013 12:43 PM, Alexandre Demers wrote:
> r8712u pollutes dmesg and logs. Silence it and add a verbose option to KConfig if we ever
> really want to hear about it.
>
> v2: keep netdev_info instead of replacing it by a KERN_NOTICE
>
> ---
>   drivers/staging/rtl8712/Kconfig        | 7 +++++++
>   drivers/staging/rtl8712/rtl871x_mlme.c | 7 ++++---
>   2 files changed, 11 insertions(+), 3 deletions(-)

Do you really want the "v2" to be part of the patch title in the git repos? I 
expect not, thus you should put it inside the []. All that stuff is stripped off 
when the patch is merged.

Similarly, do you want the v2 line in the patch description to be part of the 
permanent record? If not, move it below the --- line.

You are also missing a Signed-off-by: line. Have you read 
Documentation/SubmittingPatches? At least you should run every patch through 
scripts/checkpatch.pl before submitting it.

Finally, why do you want to add another Kconfig variable? I suggest either 
deleting the logging statement, or commenting it out. I'm not sure that this 
info would ever need logging.

I NACK this patch.

Larry

Re: [PATCH] Silence r8712u info and add verbose option v2

[Greg Kroah-Hartman]

On Tue, Nov 12, 2013 at 01:14:30PM -0600, Larry Finger wrote:
> On 11/12/2013 12:43 PM, Alexandre Demers wrote:
> > r8712u pollutes dmesg and logs. Silence it and add a verbose option to KConfig if we ever
> > really want to hear about it.
> >
> > v2: keep netdev_info instead of replacing it by a KERN_NOTICE
> >
> > ---
> >   drivers/staging/rtl8712/Kconfig        | 7 +++++++
> >   drivers/staging/rtl8712/rtl871x_mlme.c | 7 ++++---
> >   2 files changed, 11 insertions(+), 3 deletions(-)
> 
> Do you really want the "v2" to be part of the patch title in the git repos? I 
> expect not, thus you should put it inside the []. All that stuff is stripped off 
> when the patch is merged.
> 
> Similarly, do you want the v2 line in the patch description to be part of the 
> permanent record? If not, move it below the --- line.
> 
> You are also missing a Signed-off-by: line. Have you read 
> Documentation/SubmittingPatches? At least you should run every patch through 
> scripts/checkpatch.pl before submitting it.
> 
> Finally, why do you want to add another Kconfig variable? I suggest either 
> deleting the logging statement, or commenting it out. I'm not sure that this 
> info would ever need logging.

To be a bit more clear here, we are activly _removing_ driver and
subsystem specific DEBUG configuration flags, and moving everything to
using the dynamic debug infrastructure in the kernel, as no user ever
rebuilds their kernels/drivers, and we want to be able to get debugging
information from them at times.

thanks,

greg k-h

Re: [PATCH] Silence r8712u info and add verbose option v2

[Alexandre Demers]

Hi Larry and Greg,
You are right about the v2 thing and the Signed-off-by. It's the first
time I submit a patch directly to the kernel list, I didn't go to the
bottom of the SubmittingPatches, I should have read the whole document
and not stop in the middle of it. I'll finish reading it and do what
is expected. Greg, I understand your comment: I would have had the
answer if I had read the whole document (it's in there, I've just seen
it).

However, I'll take the opportunity you are offering me, Larry, with
your last question: I was asking myself the same question about simply
deleting the statement which seems pretty useless to me. On the other
hand, should we continue writing patches against r8712u? I'm not aware
of the conclusion about replacing r8712u by r92su. Was there any
decision taken on the subject?

Thank you,
Alexandre D


On Tue, Nov 12, 2013 at 4:06 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 12, 2013 at 01:14:30PM -0600, Larry Finger wrote:
>> On 11/12/2013 12:43 PM, Alexandre Demers wrote:
>> > r8712u pollutes dmesg and logs. Silence it and add a verbose option to KConfig if we ever
>> > really want to hear about it.
>> >
>> > v2: keep netdev_info instead of replacing it by a KERN_NOTICE
>> >
>> > ---
>> >   drivers/staging/rtl8712/Kconfig        | 7 +++++++
>> >   drivers/staging/rtl8712/rtl871x_mlme.c | 7 ++++---
>> >   2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> Do you really want the "v2" to be part of the patch title in the git repos? I
>> expect not, thus you should put it inside the []. All that stuff is stripped off
>> when the patch is merged.
>>
>> Similarly, do you want the v2 line in the patch description to be part of the
>> permanent record? If not, move it below the --- line.
>>
>> You are also missing a Signed-off-by: line. Have you read
>> Documentation/SubmittingPatches? At least you should run every patch through
>> scripts/checkpatch.pl before submitting it.
>>
>> Finally, why do you want to add another Kconfig variable? I suggest either
>> deleting the logging statement, or commenting it out. I'm not sure that this
>> info would ever need logging.
>
> To be a bit more clear here, we are activly _removing_ driver and
> subsystem specific DEBUG configuration flags, and moving everything to
> using the dynamic debug infrastructure in the kernel, as no user ever
> rebuilds their kernels/drivers, and we want to be able to get debugging
> information from them at times.
>
> thanks,
>
> greg k-h

Re: [PATCH] Silence r8712u info and add verbose option v2

[Larry Finger]

On 11/12/2013 04:38 PM, Alexandre Demers wrote:
> Hi Larry and Greg,
> You are right about the v2 thing and the Signed-off-by. It's the first
> time I submit a patch directly to the kernel list, I didn't go to the
> bottom of the SubmittingPatches, I should have read the whole document
> and not stop in the middle of it. I'll finish reading it and do what
> is expected. Greg, I understand your comment: I would have had the
> answer if I had read the whole document (it's in there, I've just seen
> it).
>
> However, I'll take the opportunity you are offering me, Larry, with
> your last question: I was asking myself the same question about simply
> deleting the statement which seems pretty useless to me. On the other
> hand, should we continue writing patches against r8712u? I'm not aware
> of the conclusion about replacing r8712u by r92su. Was there any
> decision taken on the subject?

I am not involved in the r92su project. My impression is that the latest 
proponent of that change was not happy with the information that r92su would 
still be in staging, but without mac80211 there is no other choice.

If there are parts of r8712u that are problems for you, then patch them. I will 
review them, but I am not likely to contribute other than for things that are bugs.

Larry