Messages in this thread | | | Date | Tue, 25 Jan 2022 22:03:06 +0300 | Subject | Re: [PATCH 06/10] staging: r8188eu: remove DBG_88E calls from os_dep/ioctl_linux.c | From | Pavel Skripkin <> |
| |
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
| |