lkml.org 
[lkml]   [2015]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] staging: rtl8188eu: hal: phy: Removed variables that is never used
> I have also removed all the code that thereby serves no purpose.

[ snip ]

> @@ -743,7 +743,7 @@ static u8 phy_path_a_iqk(struct adapter *adapt, bool config_pathb)
> reg_eac = phy_query_bb_reg(adapt, rRx_Power_After_IQK_A_2, bMaskDWord);
> reg_e94 = phy_query_bb_reg(adapt, rTx_Power_Before_IQK_A, bMaskDWord);
> reg_e9c = phy_query_bb_reg(adapt, rTx_Power_After_IQK_A, bMaskDWord);
> - reg_ea4 = phy_query_bb_reg(adapt, rRx_Power_Before_IQK_A_2, bMaskDWord);
> + phy_query_bb_reg(adapt, rRx_Power_Before_IQK_A_2, bMaskDWord);
>
> if (!(reg_eac & BIT28) &&
> (((reg_e94 & 0x03FF0000)>>16) != 0x142) &&

You clearly didn't remove *all* the "code that thereby serves no
purpose".

Could we call a cease fire on these and stop sending them until Greg or
someone else weighs in? It's hard for me to comment on 50 patches that
I don't like.

To re-iterate:

1) Removing part of the line is 90% likely to be wrong. We can't know
without knowing the code better, knowing the hardware, or testing.

2) Silencing static checker warnings in the wrong way, negates the
usefulness of static checkers and makes bad code hard to find. Some
of these "unused" variable warnings could easily be bugs so we are
hiding bugs.

3) From an aesthetic point of view the code was easier to understand
before. The line you have deleted was clearly copy and pasted from
the line before. But now it looks deliberate and the information we
need to understand the original intent is hidden in the git log.

regards,
dan carpenter



\
 
 \ /
  Last update: 2015-01-31 17:21    [W:0.782 / U:2.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site