From: Pavel Skripkin <paskripkin@gmail.com>
To: Phillip Potter <phil@philpotter.co.uk>, gregkh@linuxfoundation.org
Cc: 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: Tue, 25 Jan 2022 22:03:06 +0300 [thread overview]
Message-ID: <a681a562-eb41-9dc8-703a-5b235c15a3a1@gmail.com> (raw)
In-Reply-To: <20220124224415.831-7-phil@philpotter.co.uk>
Hi Phillip,
On 1/25/22 01:44, Phillip Potter wrote:
> Remove all DBG_88E calls from os_dep/ioctl_linux.c, as they do not
> conform to kernel coding standards and are superfluous. Also restructure
> where appropriate to remove no longer needed code left behind by removal
> of these calls. This will allow the eventual removal of the DBG_88E macro
> itself.
>
> Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
> ---
[code snip]
> @@ -3746,7 +3541,6 @@ static int rtw_dbg_port(struct net_device *dev,
> u32 write_num = extra_arg;
>
> int i;
> - u16 final;
> struct xmit_frame *xmit_frame;
>
> xmit_frame = rtw_IOL_accquire_xmit_frame(padapter);
> @@ -3760,11 +3554,7 @@ static int rtw_dbg_port(struct net_device *dev,
> if (rtl8188e_IOL_exec_cmds_sync(padapter, xmit_frame, 5000, 0) != _SUCCESS)
> ret = -EPERM;
>
> - final = rtw_read16(padapter, reg);
> - if (start_value + write_num - 1 == final)
> - DBG_88E("continuous IOL_CMD_WW_REG to 0x%x %u times Success, start:%u, final:%u\n", reg, write_num, start_value, final);
> - else
> - DBG_88E("continuous IOL_CMD_WW_REG to 0x%x %u times Fail, start:%u, final:%u\n", reg, write_num, start_value, final);
> + rtw_read16(padapter, reg);
> }
> break;
I see, that you somewhere removes reads and somewhere leaves them.
What's the difference? I saw, that one of the places has the comment,
that asks not to remove the read, but others do not have such comment
I can point to few places in 2 and 4 patches where you have removed reads.
[code snip]
> @@ -4014,16 +3664,8 @@ static int rtw_dbg_port(struct net_device *dev,
> {
> u32 odm_flag;
>
> - if (0xf == extra_arg) {
> + if (extra_arg == 0xf) {
> GetHalDefVar8188EUsb(padapter, HAL_DEF_DBG_DM_FUNC, &odm_flag);
> - DBG_88E(" === DMFlag(0x%08x) ===\n", odm_flag);
> - DBG_88E("extra_arg = 0 - disable all dynamic func\n");
> - DBG_88E("extra_arg = 1 - disable DIG- BIT(0)\n");
> - DBG_88E("extra_arg = 2 - disable High power - BIT(1)\n");
> - DBG_88E("extra_arg = 3 - disable tx power tracking - BIT(2)\n");
> - DBG_88E("extra_arg = 4 - disable BT coexistence - BIT(3)\n");
> - DBG_88E("extra_arg = 5 - disable antenna diversity - BIT(4)\n");
> - DBG_88E("extra_arg = 6 - enable all dynamic func\n");
> } else {
> /* extra_arg = 0 - disable all dynamic func
> extra_arg = 1 - disable DIG
> @@ -4032,51 +3674,17 @@ static int rtw_dbg_port(struct net_device *dev,
> */
> SetHalDefVar8188EUsb(padapter, HAL_DEF_DBG_DM_FUNC, &extra_arg);
> GetHalDefVar8188EUsb(padapter, HAL_DEF_DBG_DM_FUNC, &odm_flag);
> - DBG_88E(" === DMFlag(0x%08x) ===\n", odm_flag);
> }
> }
> break;
>
Is odm_flag needed now? Seems like it was used only for printing random
debug info
> case 0xfd:
> rtw_write8(padapter, 0xc50, arg);
> - DBG_88E("wr(0xc50) = 0x%x\n", rtw_read8(padapter, 0xc50));
> rtw_write8(padapter, 0xc58, arg);
> - DBG_88E("wr(0xc58) = 0x%x\n", rtw_read8(padapter, 0xc58));
> - break;
> - case 0xfe:
> - DBG_88E("rd(0xc50) = 0x%x\n", rtw_read8(padapter, 0xc50));
> - DBG_88E("rd(0xc58) = 0x%x\n", rtw_read8(padapter, 0xc58));
> - break;
> - case 0xff:
> - DBG_88E("dbg(0x210) = 0x%x\n", rtw_read32(padapter, 0x210));
> - DBG_88E("dbg(0x608) = 0x%x\n", rtw_read32(padapter, 0x608));
> - DBG_88E("dbg(0x280) = 0x%x\n", rtw_read32(padapter, 0x280));
> - DBG_88E("dbg(0x284) = 0x%x\n", rtw_read32(padapter, 0x284));
> - DBG_88E("dbg(0x288) = 0x%x\n", rtw_read32(padapter, 0x288));
> -
> - DBG_88E("dbg(0x664) = 0x%x\n", rtw_read32(padapter, 0x664));
> -
> - DBG_88E("\n");
> -
> - DBG_88E("dbg(0x430) = 0x%x\n", rtw_read32(padapter, 0x430));
> - DBG_88E("dbg(0x438) = 0x%x\n", rtw_read32(padapter, 0x438));
> -
> - DBG_88E("dbg(0x440) = 0x%x\n", rtw_read32(padapter, 0x440));
> -
> - DBG_88E("dbg(0x458) = 0x%x\n", rtw_read32(padapter, 0x458));
> -
> - DBG_88E("dbg(0x484) = 0x%x\n", rtw_read32(padapter, 0x484));
> - DBG_88E("dbg(0x488) = 0x%x\n", rtw_read32(padapter, 0x488));
> -
> - DBG_88E("dbg(0x444) = 0x%x\n", rtw_read32(padapter, 0x444));
> - DBG_88E("dbg(0x448) = 0x%x\n", rtw_read32(padapter, 0x448));
> - DBG_88E("dbg(0x44c) = 0x%x\n", rtw_read32(padapter, 0x44c));
> - DBG_88E("dbg(0x450) = 0x%x\n", rtw_read32(padapter, 0x450));
> break;
> }
And here you also removes the reads. I guess, some kind of magic pattern
is used
With regards,
Pavel Skripkin
next prev parent reply other threads:[~2022-01-25 19:03 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 [this message]
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
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=a681a562-eb41-9dc8-703a-5b235c15a3a1@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;
as well as URLs for NNTP newsgroup(s).