[PATCH net-next] rtlwifi: 8192de: correct checking of IQK reload

[PATCH net-next] rtlwifi: 8192de: correct checking of IQK reload

[Ping-Ke Shih]

This mistake is existing since initial commit, and I recall the vendor
driver to find correctness.

Fixes: ee3db469dd31 ("wifi: rtlwifi: remove always-true condition pointed out by GCC 12")
Cc: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
This patch should go through net-next tree, because this one is going to fix
"wifi: rtlwifi: remove always-true condition pointed out by GCC 12" that only
exits in that tree.
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
index 15e6a6aded319..f6ff32658863d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
@@ -2386,7 +2386,10 @@ void rtl92d_phy_reload_iqk_setting(struct ieee80211_hw *hw, u8 channel)
 			rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
 				"Just Read IQK Matrix reg for channel:%d....\n",
 				channel);
-			_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
+			if ((rtlphy->iqk_matrix[indexforchannel].
+			     value[0][0] != 0)
+				/*&&(regea4 != 0) */)
+				_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
 					rtlphy->iqk_matrix[
 					indexforchannel].value,	0,
 					(rtlphy->iqk_matrix[
-- 
2.25.1

Re: [PATCH net-next] rtlwifi: 8192de: correct checking of IQK reload

[Jakub Kicinski]

On Tue, 24 May 2022 08:37:50 +0800 Ping-Ke Shih wrote:
> This mistake is existing since initial commit, and I recall the vendor
> driver to find correctness.
> 
> Fixes: ee3db469dd31 ("wifi: rtlwifi: remove always-true condition pointed out by GCC 12")

Not fair, I did not break this code. My change did not alter 
the functionality.

> Cc: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
> This patch should go through net-next tree, because this one is going to fix
> "wifi: rtlwifi: remove always-true condition pointed out by GCC 12" that only
> exits in that tree.

If you want it to go via net-next you must CC netdev@.

But I don't see any reason to rush it, AFAICS the code was added in
2011 - commit 7274a8c22980 ("rtlwifi: rtl8192de: Merge phy routines")
or so. We waited 11 years to notice, we can wait a few weeks for the 
patch to propagate thru wireless tree.

> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> index 15e6a6aded319..f6ff32658863d 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> @@ -2386,7 +2386,10 @@ void rtl92d_phy_reload_iqk_setting(struct ieee80211_hw *hw, u8 channel)
>  			rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
>  				"Just Read IQK Matrix reg for channel:%d....\n",
>  				channel);
> -			_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
> +			if ((rtlphy->iqk_matrix[indexforchannel].
> +			     value[0][0] != 0)
> +				/*&&(regea4 != 0) */)

Please don't add the extra brackets around the condition
and the commented out code back in.

> +				_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
>  					rtlphy->iqk_matrix[
>  					indexforchannel].value,	0,
>  					(rtlphy->iqk_matrix[

RE: [PATCH net-next] rtlwifi: 8192de: correct checking of IQK reload

[Ping-Ke Shih]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, May 24, 2022 9:41 AM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: kvalo@kernel.org; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH net-next] rtlwifi: 8192de: correct checking of IQK reload
> 
> On Tue, 24 May 2022 08:37:50 +0800 Ping-Ke Shih wrote:
> > This mistake is existing since initial commit, and I recall the vendor
> > driver to find correctness.
> >
> > Fixes: ee3db469dd31 ("wifi: rtlwifi: remove always-true condition pointed out by GCC 12")
> 
> Not fair, I did not break this code. My change did not alter
> the functionality.

Will annotate the commit 11 years ago.

> 
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> > This patch should go through net-next tree, because this one is going to fix
> > "wifi: rtlwifi: remove always-true condition pointed out by GCC 12" that only
> > exits in that tree.
> 
> If you want it to go via net-next you must CC netdev@.
> 
> But I don't see any reason to rush it, AFAICS the code was added in
> 2011 - commit 7274a8c22980 ("rtlwifi: rtl8192de: Merge phy routines")
> or so. We waited 11 years to notice, we can wait a few weeks for the
> patch to propagate thru wireless tree.
> 
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> > index 15e6a6aded319..f6ff32658863d 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> > @@ -2386,7 +2386,10 @@ void rtl92d_phy_reload_iqk_setting(struct ieee80211_hw *hw, u8 channel)
> >  			rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
> >  				"Just Read IQK Matrix reg for channel:%d....\n",
> >  				channel);
> > -			_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
> > +			if ((rtlphy->iqk_matrix[indexforchannel].
> > +			     value[0][0] != 0)
> > +				/*&&(regea4 != 0) */)
> 
> Please don't add the extra brackets around the condition
> and the commented out code back in.

My patch is to revert your patch and correct single one line.
I will correct the strange coding style as well until your patch appears
in wireless-next.

> 
> > +				_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
> >  					rtlphy->iqk_matrix[
> >  					indexforchannel].value,	0,
> >  					(rtlphy->iqk_matrix[
> ------Please consider the environment before printing this e-mail.