Re: [PATCH 06/10] staging: r8188eu: remove DBG_88E calls from os_dep/ioctl_linux.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx>
---

[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




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux