Re: [PATCH] staging: rtl8192e: Remove functions _rtl92e_wx_get_sens and _rtl92e_wx_set_sens

[Philipp Hortmann <philipp.g.hortmann@gmail.com>]

On 4/10/23 10:49, Yogesh Hegde wrote:
> Both of these functions depend on the function rf_set_sens, which is declared
> but never defined. Hence calling this function will cause an oops.
> Because there is no definition of the function priv->rf_set_sens will always be NULL.
> 
> As a result _rtl92e_wx_set_sens and _rtl92e_wx_get_sens will always return -1.
> 
> Hence,
> * Removed function definition rf_set_sens
> * Removed usage of variable priv->rf_set_sens
> * Removed functions _rtl92e_wx_get_sens and _rtl92e_wx_set_sens
> * Cleaned up the variables sens and max_sens used in these functions
> 
> This bug was pointed out by Philipp Hortmann[1].
> 
> [1]: https://lore.kernel.org/linux-staging/004210bd-0ed1-58d5-0315-47499c850444@gmail.com/
> 
> Signed-off-by: Yogesh Hegde <yogi.kernel@gmail.com>
> ---
>   drivers/staging/rtl8192e/rtl8192e/rtl_core.h |  3 --
>   drivers/staging/rtl8192e/rtl8192e/rtl_wx.c   | 45 --------------------
>   2 files changed, 48 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.h b/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
> index 2b2d8af4cf6e..a949a3833cca 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
> @@ -234,7 +234,6 @@ struct r8192_priv {
>   	struct rt_stats stats;
>   	struct iw_statistics			wstats;
>   
> -	short (*rf_set_sens)(struct net_device *dev, short sens);
>   	u8 (*rf_set_chan)(struct net_device *dev, u8 ch);
>   
>   	struct rx_desc *rx_ring[MAX_RX_QUEUE];
> @@ -274,8 +273,6 @@ struct r8192_priv {
>   	short	promisc;
>   
>   	short	chan;
> -	short	sens;
> -	short	max_sens;
>   	bool ps_force;
>   
>   	u32 irq_mask[2];
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> index cb28288a618b..0bb657fda06c 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
> @@ -311,10 +311,6 @@ static int _rtl92e_wx_get_range(struct net_device *dev,
>   	/* ~130 Mb/s real (802.11n) */
>   	range->throughput = 130 * 1000 * 1000;
>   
> -	if (priv->rf_set_sens != NULL)
> -		/* signal level threshold range */
> -		range->sensitivity = priv->max_sens;
> -
>   	range->max_qual.qual = 100;
>   	range->max_qual.level = 0;
>   	range->max_qual.noise = 0;
> @@ -807,45 +803,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
>   	return 0;
>   }
>   
> -static int _rtl92e_wx_get_sens(struct net_device *dev,
> -			       struct iw_request_info *info,
> -			       union iwreq_data *wrqu, char *extra)
> -{
> -	struct r8192_priv *priv = rtllib_priv(dev);
> -
> -	if (priv->rf_set_sens == NULL)
> -		return -1; /* we have not this support for this radio */
> -	wrqu->sens.value = priv->sens;
> -	return 0;
> -}
> -
> -static int _rtl92e_wx_set_sens(struct net_device *dev,
> -			       struct iw_request_info *info,
> -			       union iwreq_data *wrqu, char *extra)
> -{
> -	struct r8192_priv *priv = rtllib_priv(dev);
> -
> -	short err = 0;
> -
> -	if (priv->hw_radio_off)
> -		return 0;
> -
> -	mutex_lock(&priv->wx_mutex);
> -	if (priv->rf_set_sens == NULL) {
> -		err = -1; /* we have not this support for this radio */
> -		goto exit;
> -	}
> -	if (priv->rf_set_sens(dev, wrqu->sens.value) == 0)
> -		priv->sens = wrqu->sens.value;
> -	else
> -		err = -EINVAL;
> -
> -exit:
> -	mutex_unlock(&priv->wx_mutex);
> -
> -	return err;
> -}
> -
>   static int _rtl92e_wx_set_encode_ext(struct net_device *dev,
>   				     struct iw_request_info *info,
>   				     union iwreq_data *wrqu, char *extra)
> @@ -1066,8 +1023,6 @@ static iw_handler r8192_wx_handlers[] = {
>   	[IW_IOCTL(SIOCGIWFREQ)] = _rtl92e_wx_get_freq,
>   	[IW_IOCTL(SIOCSIWMODE)] = _rtl92e_wx_set_mode,
>   	[IW_IOCTL(SIOCGIWMODE)] = _rtl92e_wx_get_mode,
> -	[IW_IOCTL(SIOCSIWSENS)] = _rtl92e_wx_set_sens,
> -	[IW_IOCTL(SIOCGIWSENS)] = _rtl92e_wx_get_sens,
>   	[IW_IOCTL(SIOCGIWRANGE)] = _rtl92e_wx_get_range,
>   	[IW_IOCTL(SIOCSIWAP)] = _rtl92e_wx_set_wap,
>   	[IW_IOCTL(SIOCGIWAP)] = _rtl92e_wx_get_wap,


You need to watch the messages from checkpatch. Two lines are too long 
from your description. Over 75 chars....

I think that the maintainer wants that this patch is a v2 even when 
almost everything of this patch has changed. So if you resend, name the 
patch v3 with revision change history.

Instead of the pointed out by you need to choose from:
13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
Please look for the place as this is critical. Or omit this line...
As I will test this patch and get therefore credit... hopefully...

applied, compiled and tested... all OK.

Thanks for your support,

Bye Philipp