From: Pavel Skripkin <paskripkin@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Phillip Potter <phil@philpotter.co.uk>,
dan.carpenter@oracle.com, Larry.Finger@lwfinger.net,
straube.linux@gmail.com, martin@kaiser.cx,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 06/10] staging: r8188eu: remove DBG_88E calls from os_dep/ioctl_linux.c
Date: Wed, 26 Jan 2022 13:58:52 +0300 [thread overview]
Message-ID: <b6da2cde-b489-0b9d-819f-c36cd64f9531@gmail.com> (raw)
In-Reply-To: <YfEm4t+fhO4I2uDW@kroah.com>
Hi Greg,
On 1/26/22 13:48, Greg KH wrote:
>> IMHO the best thing you can do is to leave these reads and leave a comment
>> like "hey, please remove me and test". One day useless reads should be
>> anyway removed, since ideally rtw_read family must get __must_check
>> annotation + normal error handling.
>
> No, if these were never getting called in normal operation, there's no
> need to add them back.
>
I guess, I was not clear, sorry. I mean leave reads that were called
during normal operations, but used only for printing debug info. (As
Phillip has already done in v1)
Reads inside R88_DBG() and other debug macros of course should be
removed, but other places seems dangerous without good testing. There is
al least one place with following comment:
> /* Although lenc is only used in a debug statement,
> * do not remove it as the rtw_read16() call consumes
> * 2 bytes from the EEPROM source.
> */
> u16 lenc = rtw_read16(adapter, REG_PKTBUF_DBG_DATA_L);
There is a chance that other places have same problem, but don't have a
comment above it. That's why I suggested to leave all these "debug"
reads and leave a comment for further work. It will help to easily spot
them in future and remove or leave them with explanations why.
With regards,
Pavel Skripkin
next prev parent reply other threads:[~2022-01-26 10:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-24 22:44 [PATCH 00/10] Cleanup and removal of DBG_88E macro Phillip Potter
2022-01-24 22:44 ` [PATCH 01/10] staging: r8188eu: remove previously added dev_dbg and netdev_dbg calls Phillip Potter
2022-01-24 22:44 ` [PATCH 02/10] staging: r8188eu: remove smaller sets of DBG_88E calls from core dir Phillip Potter
2022-01-25 9:02 ` Dan Carpenter
2022-01-26 0:54 ` Phillip Potter
2022-01-26 6:00 ` Dan Carpenter
2022-01-25 18:17 ` Pavel Skripkin
2022-01-26 1:00 ` Phillip Potter
2022-01-24 22:44 ` [PATCH 03/10] staging: r8188eu: remove DBG_88E calls from core/rtw_mlme_ext.c Phillip Potter
2022-01-25 7:22 ` kernel test robot
2022-01-25 18:31 ` Pavel Skripkin
2022-01-26 1:02 ` Phillip Potter
2022-01-24 22:44 ` [PATCH 04/10] staging: r8188eu: remove all DBG_88E calls from hal dir Phillip Potter
2022-01-25 8:27 ` kernel test robot
2022-01-24 22:44 ` [PATCH 05/10] staging: r8188eu: remove smaller sets of DBG_88E calls from os_dep dir Phillip Potter
2022-01-25 12:48 ` Pavel Skripkin
2022-01-25 12:59 ` Dan Carpenter
2022-01-25 13:04 ` Pavel Skripkin
2022-01-25 13:07 ` Dan Carpenter
2022-01-26 0:58 ` Phillip Potter
2022-01-24 22:44 ` [PATCH 06/10] staging: r8188eu: remove DBG_88E calls from os_dep/ioctl_linux.c Phillip Potter
2022-01-25 9:31 ` kernel test robot
2022-01-25 19:03 ` Pavel Skripkin
2022-01-26 1:13 ` Phillip Potter
2022-01-26 10:26 ` Pavel Skripkin
2022-01-26 10:48 ` Greg KH
2022-01-26 10:58 ` Pavel Skripkin [this message]
2022-01-26 11:14 ` Greg KH
2022-01-27 8:53 ` Phillip Potter
2022-01-24 22:44 ` [PATCH 07/10] staging: r8188eu: remove DBG_88E call from include/usb_ops.h Phillip Potter
2022-01-24 22:44 ` [PATCH 08/10] staging: r8188eu: remove all remaining aliased DBG_88E calls Phillip Potter
2022-01-24 22:44 ` [PATCH 09/10] staging: r8188eu: remove DBG_88E macro definition Phillip Potter
2022-01-24 22:44 ` [PATCH 10/10] staging: r8188eu: remove GlobalDebugLevel flag Phillip Potter
2022-01-25 15:33 ` [PATCH 00/10] Cleanup and removal of DBG_88E macro Greg KH
2022-01-26 0:45 ` Phillip Potter
2022-01-24 22:50 [RESENT PATCH 00/10] staging: r8188eu: " Phillip Potter
2022-01-24 22:50 ` [PATCH 06/10] staging: r8188eu: remove DBG_88E calls from os_dep/ioctl_linux.c Phillip Potter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b6da2cde-b489-0b9d-819f-c36cd64f9531@gmail.com \
--to=paskripkin@gmail.com \
--cc=Larry.Finger@lwfinger.net \
--cc=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=martin@kaiser.cx \
--cc=phil@philpotter.co.uk \
--cc=straube.linux@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome