LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] staging: r8188eu: remove rtw_ioctl function
@ 2021-08-10  7:45 Phillip Potter
  2021-08-10  8:06 ` Dan Carpenter
  2021-08-10  8:08 ` Arnd Bergmann
  0 siblings, 2 replies; 8+ messages in thread
From: Phillip Potter @ 2021-08-10  7:45 UTC (permalink / raw)
  To: gregkh; +Cc: Larry.Finger, linux-staging, linux-kernel, arnd

Remove rtw_ioctl function from os_dep/ioctl_linux.c, its declaration
in include/osdep_intf.h, and finally its inclusion as the value of the
.ndo_do_ioctl member of struct net_device_ops rtw_netdev_ops in
os_dep/os_intfs.c.

The e-mail from Arnd Bergmann at:
https://lore.kernel.org/linux-next/CAK8P3a0WRMNmBmBDerZ0nWPdFZKALnxrj-uUFBkTP-MOtv48vg@mail.gmail.com
explains the justification for this approach. Essentially, changes from
net-next make the existing function and its use of SIOCDEVPRIVATE ioctls
unworkable without changes.

Functions called from rtw_ioctl that are now no longer accessible will
be cleaned up in further patches. Additionally, once these changes have
made their way downstream to the staging branch, we can modify and
reconnect rtw_android_priv_cmd via ndo_siocdevprivate in struct
net_device_ops.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
---
 drivers/staging/r8188eu/include/osdep_intf.h |  2 --
 drivers/staging/r8188eu/os_dep/ioctl_linux.c | 28 --------------------
 drivers/staging/r8188eu/os_dep/os_intfs.c    |  1 -
 3 files changed, 31 deletions(-)

diff --git a/drivers/staging/r8188eu/include/osdep_intf.h b/drivers/staging/r8188eu/include/osdep_intf.h
index 1915c926faac..3ea60feee2db 100644
--- a/drivers/staging/r8188eu/include/osdep_intf.h
+++ b/drivers/staging/r8188eu/include/osdep_intf.h
@@ -48,8 +48,6 @@ u32 rtw_start_drv_threads(struct adapter *padapter);
 void rtw_stop_drv_threads (struct adapter *padapter);
 void rtw_cancel_all_timer(struct adapter *padapter);
 
-int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
-
 int rtw_init_netdev_name(struct net_device *pnetdev, const char *ifname);
 struct net_device *rtw_init_netdev(struct adapter *padapter);
 u16 rtw_recv_select_queue(struct sk_buff *skb);
diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 34dce01e469c..beabbc84232f 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -8097,31 +8097,3 @@ static int rtw_ioctl_wext_private(struct net_device *dev, union iwreq_data *wrq_
 	kfree(output);
 	return err;
 }
-
-#include "../include/rtw_android.h"
-int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	struct iwreq *wrq = (struct iwreq *)rq;
-	int ret = 0;
-
-	switch (cmd) {
-	case RTL_IOCTL_WPA_SUPPLICANT:
-		ret = wpa_supplicant_ioctl(dev, &wrq->u.data);
-		break;
-#ifdef CONFIG_88EU_AP_MODE
-	case RTL_IOCTL_HOSTAPD:
-		ret = rtw_hostapd_ioctl(dev, &wrq->u.data);
-		break;
-#endif /*  CONFIG_88EU_AP_MODE */
-	case SIOCDEVPRIVATE:
-		ret = rtw_ioctl_wext_private(dev, &wrq->u);
-		break;
-	case (SIOCDEVPRIVATE+1):
-		ret = rtw_android_priv_cmd(dev, rq, cmd);
-		break;
-	default:
-		ret = -EOPNOTSUPP;
-		break;
-	}
-	return ret;
-}
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 935e35c82666..7d17c765205c 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -692,7 +692,6 @@ static const struct net_device_ops rtw_netdev_ops = {
 	.ndo_select_queue	= rtw_select_queue,
 	.ndo_set_mac_address = rtw_net_set_mac_address,
 	.ndo_get_stats = rtw_net_get_stats,
-	.ndo_do_ioctl = rtw_ioctl,
 };
 
 int rtw_init_netdev_name(struct net_device *pnetdev, const char *ifname)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  7:45 [PATCH] staging: r8188eu: remove rtw_ioctl function Phillip Potter
@ 2021-08-10  8:06 ` Dan Carpenter
  2021-08-10 22:36   ` Phillip Potter
                     ` (2 more replies)
  2021-08-10  8:08 ` Arnd Bergmann
  1 sibling, 3 replies; 8+ messages in thread
From: Dan Carpenter @ 2021-08-10  8:06 UTC (permalink / raw)
  To: Phillip Potter, Michael Straube, Martin Kaiser
  Cc: gregkh, Larry.Finger, linux-staging, linux-kernel, arnd

Phillip, Michael, and Martin,

You are all working on this this same driver.  Could you Ack each
other's patches?

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  7:45 [PATCH] staging: r8188eu: remove rtw_ioctl function Phillip Potter
  2021-08-10  8:06 ` Dan Carpenter
@ 2021-08-10  8:08 ` Arnd Bergmann
  2021-08-10 10:22   ` gregkh
  1 sibling, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2021-08-10  8:08 UTC (permalink / raw)
  To: Phillip Potter
  Cc: gregkh, Larry Finger, linux-staging, Linux Kernel Mailing List,
	Arnd Bergmann

On Tue, Aug 10, 2021 at 9:45 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> -int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> -{
> -       struct iwreq *wrq = (struct iwreq *)rq;
> -       int ret = 0;
> -
> -       switch (cmd) {
> -       case RTL_IOCTL_WPA_SUPPLICANT:
> -               ret = wpa_supplicant_ioctl(dev, &wrq->u.data);
> -               break;
> -#ifdef CONFIG_88EU_AP_MODE
> -       case RTL_IOCTL_HOSTAPD:
> -               ret = rtw_hostapd_ioctl(dev, &wrq->u.data);
> -               break;
> -#endif /*  CONFIG_88EU_AP_MODE */
> -       case SIOCDEVPRIVATE:
> -               ret = rtw_ioctl_wext_private(dev, &wrq->u);
> -               break;


I think these functions are all defined 'static' in the same file, so
removing the
caller will cause a warning about an unused function. Better remove the
called functions along with the caller.

       Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  8:08 ` Arnd Bergmann
@ 2021-08-10 10:22   ` gregkh
  2021-08-10 22:24     ` Phillip Potter
  0 siblings, 1 reply; 8+ messages in thread
From: gregkh @ 2021-08-10 10:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Phillip Potter, Larry Finger, linux-staging, Linux Kernel Mailing List

On Tue, Aug 10, 2021 at 10:08:31AM +0200, Arnd Bergmann wrote:
> On Tue, Aug 10, 2021 at 9:45 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> > -int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> > -{
> > -       struct iwreq *wrq = (struct iwreq *)rq;
> > -       int ret = 0;
> > -
> > -       switch (cmd) {
> > -       case RTL_IOCTL_WPA_SUPPLICANT:
> > -               ret = wpa_supplicant_ioctl(dev, &wrq->u.data);
> > -               break;
> > -#ifdef CONFIG_88EU_AP_MODE
> > -       case RTL_IOCTL_HOSTAPD:
> > -               ret = rtw_hostapd_ioctl(dev, &wrq->u.data);
> > -               break;
> > -#endif /*  CONFIG_88EU_AP_MODE */
> > -       case SIOCDEVPRIVATE:
> > -               ret = rtw_ioctl_wext_private(dev, &wrq->u);
> > -               break;
> 
> 
> I think these functions are all defined 'static' in the same file, so
> removing the
> caller will cause a warning about an unused function. Better remove the
> called functions along with the caller.

I get no build warnings/errors with this patch applied, which is odd.

So I'll take this for now, but a follow-on patch to remove these unused
functions would be great to have.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10 10:22   ` gregkh
@ 2021-08-10 22:24     ` Phillip Potter
  0 siblings, 0 replies; 8+ messages in thread
From: Phillip Potter @ 2021-08-10 22:24 UTC (permalink / raw)
  To: gregkh
  Cc: Arnd Bergmann, Larry Finger, linux-staging, Linux Kernel Mailing List

On Tue, 10 Aug 2021 at 11:23, gregkh <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 10, 2021 at 10:08:31AM +0200, Arnd Bergmann wrote:
> > On Tue, Aug 10, 2021 at 9:45 AM Phillip Potter <phil@philpotter.co.uk> wrote:
> > > -int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> > > -{
> > > -       struct iwreq *wrq = (struct iwreq *)rq;
> > > -       int ret = 0;
> > > -
> > > -       switch (cmd) {
> > > -       case RTL_IOCTL_WPA_SUPPLICANT:
> > > -               ret = wpa_supplicant_ioctl(dev, &wrq->u.data);
> > > -               break;
> > > -#ifdef CONFIG_88EU_AP_MODE
> > > -       case RTL_IOCTL_HOSTAPD:
> > > -               ret = rtw_hostapd_ioctl(dev, &wrq->u.data);
> > > -               break;
> > > -#endif /*  CONFIG_88EU_AP_MODE */
> > > -       case SIOCDEVPRIVATE:
> > > -               ret = rtw_ioctl_wext_private(dev, &wrq->u);
> > > -               break;
> >
> >
> > I think these functions are all defined 'static' in the same file, so
> > removing the
> > caller will cause a warning about an unused function. Better remove the
> > called functions along with the caller.
>
> I get no build warnings/errors with this patch applied, which is odd.
>
> So I'll take this for now, but a follow-on patch to remove these unused
> functions would be great to have.
>
> thanks,
>
> greg k-h

Dear Greg,

Many thanks, and of course, I shall prepare one now.

Regards,
Phil

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  8:06 ` Dan Carpenter
@ 2021-08-10 22:36   ` Phillip Potter
  2021-08-11 19:42   ` Martin Kaiser
  2021-08-12 12:19   ` Michael Straube
  2 siblings, 0 replies; 8+ messages in thread
From: Phillip Potter @ 2021-08-10 22:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Michael Straube, Martin Kaiser, Greg KH, Larry Finger,
	linux-staging, Linux Kernel Mailing List, Arnd Bergmann

On Tue, 10 Aug 2021 at 09:07, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Phillip, Michael, and Martin,
>
> You are all working on this this same driver.  Could you Ack each
> other's patches?
>
> regards,
> dan carpenter
>
>

Dear Dan,

Certainly, will do. Will keep a look out for Michael and Martin's
future patches in particular.

Regards,
Phil

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  8:06 ` Dan Carpenter
  2021-08-10 22:36   ` Phillip Potter
@ 2021-08-11 19:42   ` Martin Kaiser
  2021-08-12 12:19   ` Michael Straube
  2 siblings, 0 replies; 8+ messages in thread
From: Martin Kaiser @ 2021-08-11 19:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Phillip Potter, Michael Straube, gregkh, Larry.Finger,
	linux-staging, linux-kernel, arnd

Hi Dan and all,

Thus wrote Dan Carpenter (dan.carpenter@oracle.com):

> Phillip, Michael, and Martin,

> You are all working on this this same driver.  Could you Ack each
> other's patches?

sure, no problem. I'll try to look at r8188 patches from Phil, Michael
and others as time permits.

Best regards,
Martin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: r8188eu: remove rtw_ioctl function
  2021-08-10  8:06 ` Dan Carpenter
  2021-08-10 22:36   ` Phillip Potter
  2021-08-11 19:42   ` Martin Kaiser
@ 2021-08-12 12:19   ` Michael Straube
  2 siblings, 0 replies; 8+ messages in thread
From: Michael Straube @ 2021-08-12 12:19 UTC (permalink / raw)
  To: Dan Carpenter, Phillip Potter, Martin Kaiser
  Cc: gregkh, Larry.Finger, linux-staging, linux-kernel, arnd

On 8/10/21 10:06 AM, Dan Carpenter wrote:
> Phillip, Michael, and Martin,
> 
> You are all working on this this same driver.  Could you Ack each
> other's patches?
> 
> regards,
> dan carpenter
> 
> 

Sure, I will look at future patches from Phillip and Martin
if time permits.

Regards,
Michael

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-12 12:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10  7:45 [PATCH] staging: r8188eu: remove rtw_ioctl function Phillip Potter
2021-08-10  8:06 ` Dan Carpenter
2021-08-10 22:36   ` Phillip Potter
2021-08-11 19:42   ` Martin Kaiser
2021-08-12 12:19   ` Michael Straube
2021-08-10  8:08 ` Arnd Bergmann
2021-08-10 10:22   ` gregkh
2021-08-10 22:24     ` Phillip Potter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).