[PATCH 00/16] staging: ks7010: next cleanups

[PATCH 00/16] staging: ks7010: next cleanups

[Sergio Paracuellos]

This patch series includes some new cleanups in code.

It also includes a remaining patch (PATCH 1/16) about 
SPDX identifiers in files which wasn't be applied.

Sergio Paracuellos (16):
  staging: ks7010: add SPDX identifiers to all files
  staging: ks7010: refactor ks_wlan_set_beacon_lost function
  staging: ks7010: refactor ks_wlan_set_scan_type function
  staging: ks7010: refactor ks_wlan_set_power_mgmt function
  staging: ks7010: refactor ks_wlan_set_preamble function
  staging: ks7010: refactor ks_wlan_set_cts_mode function
  staging: ks7010: refactor ks_wlan_set_pmksa function
  staging: ks7010: use ether_addr_copy in ks_wlan_set_encode_ext
  staging: ks7010: align values in frequency_list array
  staging: ks7010: refactor ks_wlan_set_wap function
  staging: ks7010: use ether_addr_copy in ks_wlan_get_wap
  staging: ks7010: avoid an 'else' in ks_wlan_set_power
  staging: ks7010: avoid if-else condition in ks_wlan_get_power
  staging: ks7010: use ether_addr_copy in ks_wlan_get_aplist
  staging: ks7010: avoid blank line between declarations in
    ks_wlan_get_aplist
  staging: ks7010: refactor ks_wlan_translate_scan function

 drivers/staging/ks7010/ks7010_sdio.c   |   5 +-
 drivers/staging/ks7010/ks7010_sdio.h   |   5 +-
 drivers/staging/ks7010/ks_hostif.c     |   5 +-
 drivers/staging/ks7010/ks_hostif.h     |   5 +-
 drivers/staging/ks7010/ks_wlan.h       |   5 +-
 drivers/staging/ks7010/ks_wlan_ioctl.h |   5 +-
 drivers/staging/ks7010/ks_wlan_net.c   | 302 ++++++++++++++-------------------
 drivers/staging/ks7010/michael_mic.c   |   5 +-
 drivers/staging/ks7010/michael_mic.h   |   5 +-
 9 files changed, 134 insertions(+), 208 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/16] staging: ks7010: add SPDX identifiers to all files

[Sergio Paracuellos]

It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.

Fix up the all of the staging ks7010 files to have a proper SPDX
identifier, based on the license text in the file itself.  The SPDX
identifier is a legally binding shorthand, which can be used instead of
the full boiler plate text.

Extra GPL text wording can be removed as it is no longer needed at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c   | 5 +----
 drivers/staging/ks7010/ks7010_sdio.h   | 5 +----
 drivers/staging/ks7010/ks_hostif.c     | 5 +----
 drivers/staging/ks7010/ks_hostif.h     | 5 +----
 drivers/staging/ks7010/ks_wlan.h       | 5 +----
 drivers/staging/ks7010/ks_wlan_ioctl.h | 5 +----
 drivers/staging/ks7010/ks_wlan_net.c   | 5 +----
 drivers/staging/ks7010/michael_mic.c   | 5 +----
 drivers/staging/ks7010/michael_mic.h   | 5 +----
 9 files changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index b29f48c..b6cabab 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1,13 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream, KS7010 based SDIO cards.
  *
  *   Copyright (C) 2006-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
  *   Copyright (C) 2016 Sang Engineering, Wolfram Sang
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #include <linux/atomic.h>
diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h
index 95ac86b..831b2f1 100644
--- a/drivers/staging/ks7010/ks7010_sdio.h
+++ b/drivers/staging/ks7010/ks7010_sdio.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream, KS7010 based SDIO cards.
  *
  *   Copyright (C) 2006-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 #ifndef _KS7010_SDIO_H
 #define _KS7010_SDIO_H
diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 9a05374..61d9742 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream wireless LAN cards.
  *
  *   Copyright (C) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #include <linux/if_arp.h>
diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h
index 05ff5ca..172d38f 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream wireless LAN
  *
  *   Copyright (c) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #ifndef _KS_HOSTIF_H_
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index 2894b0c..5070af8 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream IEEE802.11 b/g wireless LAN cards.
  *
  *   Copyright (C) 2006-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #ifndef _KS_WLAN_H
diff --git a/drivers/staging/ks7010/ks_wlan_ioctl.h b/drivers/staging/ks7010/ks_wlan_ioctl.h
index e45a332..97c7d95 100644
--- a/drivers/staging/ks7010/ks_wlan_ioctl.h
+++ b/drivers/staging/ks7010/ks_wlan_ioctl.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream 11b/g wireless LAN
  *
  *   Copyright (c) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #ifndef _KS_WLAN_IOCTL_H
diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index aaa57c2..71fc8c2 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream 11b/g wireless LAN
  *
  *   Copyright (C) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #include <linux/atomic.h>
diff --git a/drivers/staging/ks7010/michael_mic.c b/drivers/staging/ks7010/michael_mic.c
index 2128c84..e66da52 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream wireless LAN
  *
  *   Copyright (C) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 #include <asm/unaligned.h>
diff --git a/drivers/staging/ks7010/michael_mic.h b/drivers/staging/ks7010/michael_mic.h
index eb22fdd..f0ac164 100644
--- a/drivers/staging/ks7010/michael_mic.h
+++ b/drivers/staging/ks7010/michael_mic.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *   Driver for KeyStream wireless LAN
  *
  *   Copyright (C) 2005-2008 KeyStream Corp.
  *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
  */
 
 /* MichaelMIC routine define */
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/16] staging: ks7010: add SPDX identifiers to all files

[Greg KH]

On Wed, Apr 25, 2018 at 04:01:40PM +0200, Sergio Paracuellos wrote:
> It's good to have SPDX identifiers in all files to make it easier to
> audit the kernel tree for correct licenses.
> 
> Fix up the all of the staging ks7010 files to have a proper SPDX
> identifier, based on the license text in the file itself.  The SPDX
> identifier is a legally binding shorthand, which can be used instead of
> the full boiler plate text.
> 
> Extra GPL text wording can be removed as it is no longer needed at all.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c   | 5 +----
>  drivers/staging/ks7010/ks7010_sdio.h   | 5 +----
>  drivers/staging/ks7010/ks_hostif.c     | 5 +----
>  drivers/staging/ks7010/ks_hostif.h     | 5 +----
>  drivers/staging/ks7010/ks_wlan.h       | 5 +----
>  drivers/staging/ks7010/ks_wlan_ioctl.h | 5 +----
>  drivers/staging/ks7010/ks_wlan_net.c   | 5 +----
>  drivers/staging/ks7010/michael_mic.c   | 5 +----
>  drivers/staging/ks7010/michael_mic.h   | 5 +----
>  9 files changed, 9 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index b29f48c..b6cabab 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -1,13 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

{sigh}

No, please read the documentation for how to do this correctly.  You
used the wrong comment marker for this :(

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/16] staging: ks7010: add SPDX identifiers to all files

[Sergio Paracuellos]

On Thu, Apr 26, 2018 at 09:17:08AM +0200, Greg KH wrote:
> On Wed, Apr 25, 2018 at 04:01:40PM +0200, Sergio Paracuellos wrote:
> > It's good to have SPDX identifiers in all files to make it easier to
> > audit the kernel tree for correct licenses.
> > 
> > Fix up the all of the staging ks7010 files to have a proper SPDX
> > identifier, based on the license text in the file itself.  The SPDX
> > identifier is a legally binding shorthand, which can be used instead of
> > the full boiler plate text.
> > 
> > Extra GPL text wording can be removed as it is no longer needed at all.
> > 
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/staging/ks7010/ks7010_sdio.c   | 5 +----
> >  drivers/staging/ks7010/ks7010_sdio.h   | 5 +----
> >  drivers/staging/ks7010/ks_hostif.c     | 5 +----
> >  drivers/staging/ks7010/ks_hostif.h     | 5 +----
> >  drivers/staging/ks7010/ks_wlan.h       | 5 +----
> >  drivers/staging/ks7010/ks_wlan_ioctl.h | 5 +----
> >  drivers/staging/ks7010/ks_wlan_net.c   | 5 +----
> >  drivers/staging/ks7010/michael_mic.c   | 5 +----
> >  drivers/staging/ks7010/michael_mic.h   | 5 +----
> >  9 files changed, 9 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > index b29f48c..b6cabab 100644
> > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > @@ -1,13 +1,10 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> 
> {sigh}
> 
> No, please read the documentation for how to do this correctly.  You
> used the wrong comment marker for this :(

ack! Sorry Greg. It seems that I have to use '//' comments in source files.

Changes have been included in next series.

> 
> greg k-h

Thanks,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/16] staging: ks7010: add SPDX identifiers to all files

[Greg KH]

On Thu, Apr 26, 2018 at 10:35:52AM +0200, Sergio Paracuellos wrote:
> On Thu, Apr 26, 2018 at 09:17:08AM +0200, Greg KH wrote:
> > On Wed, Apr 25, 2018 at 04:01:40PM +0200, Sergio Paracuellos wrote:
> > > It's good to have SPDX identifiers in all files to make it easier to
> > > audit the kernel tree for correct licenses.
> > > 
> > > Fix up the all of the staging ks7010 files to have a proper SPDX
> > > identifier, based on the license text in the file itself.  The SPDX
> > > identifier is a legally binding shorthand, which can be used instead of
> > > the full boiler plate text.
> > > 
> > > Extra GPL text wording can be removed as it is no longer needed at all.
> > > 
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  drivers/staging/ks7010/ks7010_sdio.c   | 5 +----
> > >  drivers/staging/ks7010/ks7010_sdio.h   | 5 +----
> > >  drivers/staging/ks7010/ks_hostif.c     | 5 +----
> > >  drivers/staging/ks7010/ks_hostif.h     | 5 +----
> > >  drivers/staging/ks7010/ks_wlan.h       | 5 +----
> > >  drivers/staging/ks7010/ks_wlan_ioctl.h | 5 +----
> > >  drivers/staging/ks7010/ks_wlan_net.c   | 5 +----
> > >  drivers/staging/ks7010/michael_mic.c   | 5 +----
> > >  drivers/staging/ks7010/michael_mic.h   | 5 +----
> > >  9 files changed, 9 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > > index b29f48c..b6cabab 100644
> > > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > > @@ -1,13 +1,10 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > 
> > {sigh}
> > 
> > No, please read the documentation for how to do this correctly.  You
> > used the wrong comment marker for this :(
> 
> ack! Sorry Greg. It seems that I have to use '//' comments in source files.

For .c files, yes, for .h files, no.  Please read the documentation
well.

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/16] staging: ks7010: add SPDX identifiers to all files

[Sergio Paracuellos]

On Thu, Apr 26, 2018 at 10:55:42AM +0200, Greg KH wrote:
> On Thu, Apr 26, 2018 at 10:35:52AM +0200, Sergio Paracuellos wrote:
> > On Thu, Apr 26, 2018 at 09:17:08AM +0200, Greg KH wrote:
> > > On Wed, Apr 25, 2018 at 04:01:40PM +0200, Sergio Paracuellos wrote:
> > > > It's good to have SPDX identifiers in all files to make it easier to
> > > > audit the kernel tree for correct licenses.
> > > > 
> > > > Fix up the all of the staging ks7010 files to have a proper SPDX
> > > > identifier, based on the license text in the file itself.  The SPDX
> > > > identifier is a legally binding shorthand, which can be used instead of
> > > > the full boiler plate text.
> > > > 
> > > > Extra GPL text wording can be removed as it is no longer needed at all.
> > > > 
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >  drivers/staging/ks7010/ks7010_sdio.c   | 5 +----
> > > >  drivers/staging/ks7010/ks7010_sdio.h   | 5 +----
> > > >  drivers/staging/ks7010/ks_hostif.c     | 5 +----
> > > >  drivers/staging/ks7010/ks_hostif.h     | 5 +----
> > > >  drivers/staging/ks7010/ks_wlan.h       | 5 +----
> > > >  drivers/staging/ks7010/ks_wlan_ioctl.h | 5 +----
> > > >  drivers/staging/ks7010/ks_wlan_net.c   | 5 +----
> > > >  drivers/staging/ks7010/michael_mic.c   | 5 +----
> > > >  drivers/staging/ks7010/michael_mic.h   | 5 +----
> > > >  9 files changed, 9 insertions(+), 36 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > > > index b29f48c..b6cabab 100644
> > > > --- a/drivers/staging/ks7010/ks7010_sdio.c
> > > > +++ b/drivers/staging/ks7010/ks7010_sdio.c
> > > > @@ -1,13 +1,10 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > 
> > > {sigh}
> > > 
> > > No, please read the documentation for how to do this correctly.  You
> > > used the wrong comment marker for this :(
> > 
> > ack! Sorry Greg. It seems that I have to use '//' comments in source files.
> 
> For .c files, yes, for .h files, no.  Please read the documentation
> well.

Yes, that's what I wanted to say, I have to be more explicit :) 

> 
> greg k-h

Thanks,
    Sergio Paracuellos

[PATCH 02/16] staging: ks7010: refactor ks_wlan_set_beacon_lost function

[Sergio Paracuellos]

Change if condition to handle invalid value first and
avoid nonsense else's path improving readability.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 71fc8c2..68aece7 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1970,17 +1970,17 @@ static int ks_wlan_set_beacon_lost(struct net_device *dev,
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
 	/* for SLEEP MODE */
-	if (*uwrq >= BEACON_LOST_COUNT_MIN && *uwrq <= BEACON_LOST_COUNT_MAX)
-		priv->reg.beacon_lost_count = *uwrq;
-	else
+	if (*uwrq < BEACON_LOST_COUNT_MIN || *uwrq > BEACON_LOST_COUNT_MAX)
 		return -EINVAL;
 
+	priv->reg.beacon_lost_count = *uwrq;
+
 	if (priv->reg.operation_mode == MODE_INFRASTRUCTURE) {
 		priv->need_commit |= SME_MODE_SET;
 		return -EINPROGRESS;	/* Call commit handler */
-	} else {
-		return 0;
 	}
+
+	return 0;
 }
 
 static int ks_wlan_get_beacon_lost(struct net_device *dev,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/16] staging: ks7010: refactor ks_wlan_set_scan_type function

[Sergio Paracuellos]

This commit changes logic to handle invalid values first
and just assign valid ones afterwards.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 68aece7..b10c54c 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1937,14 +1937,11 @@ static int ks_wlan_set_scan_type(struct net_device *dev,
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
 	/* for SLEEP MODE */
-	if (*uwrq == ACTIVE_SCAN) {	/* 0 */
-		priv->reg.scan_type = ACTIVE_SCAN;
-	} else if (*uwrq == PASSIVE_SCAN) {	/* 1 */
-		priv->reg.scan_type = PASSIVE_SCAN;
-	} else {
+
+	if (*uwrq != ACTIVE_SCAN && *uwrq != PASSIVE_SCAN)
 		return -EINVAL;
-	}
 
+	priv->reg.scan_type = *uwrq;
 	return 0;
 }
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/16] staging: ks7010: refactor ks_wlan_set_power_mgmt function

[Sergio Paracuellos]

This commit change logic to simplify conditional paths in
ks_wlan_set_power_mgmt function. It handles invalid's first
and just finally assign a valid value.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index b10c54c..ba587c6 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1892,23 +1892,16 @@ static int ks_wlan_set_power_mgmt(struct net_device *dev,
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
 
-	/* for SLEEP MODE */
-	if (*uwrq == POWER_MGMT_ACTIVE) {	/* 0 */
-		priv->reg.power_mgmt = POWER_MGMT_ACTIVE;
-	} else if (*uwrq == POWER_MGMT_SAVE1) {	/* 1 */
-		if (priv->reg.operation_mode == MODE_INFRASTRUCTURE)
-			priv->reg.power_mgmt = POWER_MGMT_SAVE1;
-		else
-			return -EINVAL;
-	} else if (*uwrq == POWER_MGMT_SAVE2) {	/* 2 */
-		if (priv->reg.operation_mode == MODE_INFRASTRUCTURE)
-			priv->reg.power_mgmt = POWER_MGMT_SAVE2;
-		else
-			return -EINVAL;
-	} else {
+	if (*uwrq != POWER_MGMT_ACTIVE &&
+	    *uwrq != POWER_MGMT_SAVE1 &&
+	    *uwrq != POWER_MGMT_SAVE2)
+		return -EINVAL;
+
+	if ((*uwrq == POWER_MGMT_SAVE1 || *uwrq == POWER_MGMT_SAVE2) &&
+	    (priv->reg.operation_mode != MODE_INFRASTRUCTURE))
 		return -EINVAL;
-	}
 
+	priv->reg.power_mgmt = *uwrq;
 	hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST);
 
 	return 0;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/16] staging: ks7010: refactor ks_wlan_set_preamble function

[Sergio Paracuellos]

Change if logic to handle invalid values first and just
assign at the end the valid one if it is the case.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index ba587c6..89d9fda 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1857,14 +1857,10 @@ static int ks_wlan_set_preamble(struct net_device *dev,
 		return -EPERM;
 
 	/* for SLEEP MODE */
-	if (*uwrq == LONG_PREAMBLE) {	/* 0 */
-		priv->reg.preamble = LONG_PREAMBLE;
-	} else if (*uwrq == SHORT_PREAMBLE) {	/* 1 */
-		priv->reg.preamble = SHORT_PREAMBLE;
-	} else {
+	if (*uwrq != LONG_PREAMBLE && *uwrq != SHORT_PREAMBLE)
 		return -EINVAL;
-	}
 
+	priv->reg.preamble = *uwrq;
 	priv->need_commit |= SME_MODE_SET;
 	return -EINPROGRESS;	/* Call commit handler */
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/16] staging: ks7010: refactor ks_wlan_set_cts_mode function

[Sergio Paracuellos]

This commit refactors ks_wlan_set_cts_mode function to
handle invalid values first and then assign the good
one changing a bit logic to use a ternary operator.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 89d9fda..676f9fd 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2027,18 +2027,13 @@ static int ks_wlan_set_cts_mode(struct net_device *dev,
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
 	/* for SLEEP MODE */
-	if (*uwrq == CTS_MODE_FALSE) {	/* 0 */
-		priv->reg.cts_mode = CTS_MODE_FALSE;
-	} else if (*uwrq == CTS_MODE_TRUE) {	/* 1 */
-		if (priv->reg.phy_type == D_11G_ONLY_MODE ||
-		    priv->reg.phy_type == D_11BG_COMPATIBLE_MODE) {
-			priv->reg.cts_mode = CTS_MODE_TRUE;
-		} else {
-			priv->reg.cts_mode = CTS_MODE_FALSE;
-		}
-	} else {
+	if (*uwrq != CTS_MODE_FALSE && *uwrq != CTS_MODE_TRUE)
 		return -EINVAL;
-	}
+
+	priv->reg.cts_mode = (*uwrq == CTS_MODE_FALSE) ? *uwrq :
+			      (priv->reg.phy_type == D_11G_ONLY_MODE ||
+			       priv->reg.phy_type == D_11BG_COMPATIBLE_MODE) ?
+			       *uwrq : !*uwrq;
 
 	priv->need_commit |= SME_MODE_SET;
 	return -EINPROGRESS;	/* Call commit handler */
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/16] staging: ks7010: refactor ks_wlan_set_pmksa function

[Sergio Paracuellos]

This commit cleans a bit ks_wlan_set_pmksa function removing
nonsense comments as well as make use of ether_addr_* family
functions written to not do manually things that were being
here. Minor single if brackets has been removed also.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 45 +++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 676f9fd..5553743 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1691,68 +1691,65 @@ static int ks_wlan_set_pmksa(struct net_device *dev,
 
 	switch (pmksa->cmd) {
 	case IW_PMKSA_ADD:
-		if (list_empty(&priv->pmklist.head)) {	/* new list */
+		if (list_empty(&priv->pmklist.head)) {
 			for (i = 0; i < PMK_LIST_MAX; i++) {
 				pmk = &priv->pmklist.pmk[i];
-				if (memcmp("\x00\x00\x00\x00\x00\x00",
-					   pmk->bssid, ETH_ALEN) == 0)
-					break; /* loop */
+				if (is_zero_ether_addr(pmk->bssid))
+					break;
 			}
-			memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN);
+			ether_addr_copy(pmk->bssid, pmksa->bssid.sa_data);
 			memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
 			list_add(&pmk->list, &priv->pmklist.head);
 			priv->pmklist.size++;
-			break;	/* case */
+			break;
 		}
 		/* search cache data */
 		list_for_each(ptr, &priv->pmklist.head) {
 			pmk = list_entry(ptr, struct pmk, list);
-			if (memcmp(pmksa->bssid.sa_data, pmk->bssid, ETH_ALEN) == 0) {
+			if (ether_addr_equal(pmksa->bssid.sa_data, pmk->bssid)) {
 				memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
 				list_move(&pmk->list, &priv->pmklist.head);
-				break; /* list_for_each */
+				break;
 			}
 		}
-		if (ptr != &priv->pmklist.head)	/* not find address. */
-			break;	/* case */
-
-		if (priv->pmklist.size < PMK_LIST_MAX) {	/* new cache data */
+		/* not find address. */
+		if (ptr != &priv->pmklist.head)
+			break;
+		/* new cache data */
+		if (priv->pmklist.size < PMK_LIST_MAX) {
 			for (i = 0; i < PMK_LIST_MAX; i++) {
 				pmk = &priv->pmklist.pmk[i];
-				if (memcmp("\x00\x00\x00\x00\x00\x00",
-					   pmk->bssid, ETH_ALEN) == 0)
-					break; /* loop */
+				if (is_zero_ether_addr(pmk->bssid))
+					break;
 			}
-			memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN);
+			ether_addr_copy(pmk->bssid, pmksa->bssid.sa_data);
 			memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
 			list_add(&pmk->list, &priv->pmklist.head);
 			priv->pmklist.size++;
-		} else {	/* overwrite old cache data */
+		} else { /* overwrite old cache data */
 			pmk = list_entry(priv->pmklist.head.prev, struct pmk,
 					 list);
-			memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN);
+			ether_addr_copy(pmk->bssid, pmksa->bssid.sa_data);
 			memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
 			list_move(&pmk->list, &priv->pmklist.head);
 		}
 		break;
 	case IW_PMKSA_REMOVE:
-		if (list_empty(&priv->pmklist.head)) {	/* list empty */
+		if (list_empty(&priv->pmklist.head))
 			return -EINVAL;
-		}
 		/* search cache data */
 		list_for_each(ptr, &priv->pmklist.head) {
 			pmk = list_entry(ptr, struct pmk, list);
-			if (memcmp(pmksa->bssid.sa_data, pmk->bssid, ETH_ALEN) == 0) {
+			if (ether_addr_equal(pmksa->bssid.sa_data, pmk->bssid)) {
 				eth_zero_addr(pmk->bssid);
 				memset(pmk->pmkid, 0, IW_PMKID_LEN);
 				list_del_init(&pmk->list);
 				break;
 			}
 		}
-		if (ptr == &priv->pmklist.head) {	/* not find address. */
+		/* not find address. */
+		if (ptr == &priv->pmklist.head)
 			return 0;
-		}
-
 		break;
 	case IW_PMKSA_FLUSH:
 		memset(&priv->pmklist, 0, sizeof(priv->pmklist));
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/16] staging: ks7010: use ether_addr_copy in ks_wlan_set_encode_ext

[Sergio Paracuellos]

Instead of manually memcpy ethernet addresses use ether_addr_copy
function for this purpose.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 5553743..e1f92ce 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1591,7 +1591,7 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
 		memcpy(&key->rx_seq[0], &enc->rx_seq[0], IW_ENCODE_SEQ_MAX_SIZE);
 	}
 
-	memcpy(&key->addr.sa_data[0], &enc->addr.sa_data[0], ETH_ALEN);
+	ether_addr_copy(&key->addr.sa_data[0], &enc->addr.sa_data[0]);
 
 	switch (enc->alg) {
 	case IW_ENCODE_ALG_NONE:
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/16] staging: ks7010: align values in frequency_list array

[Sergio Paracuellos]

To avoid a long line align values in static array
frequency_list array.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index e1f92ce..2fd5b50 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -28,7 +28,8 @@ static int wep_on_off;
 #include <net/iw_handler.h>	/* New driver API */
 
 /* Frequency list (map channels to frequencies) */
-static const long frequency_list[] = { 2412, 2417, 2422, 2427, 2432, 2437, 2442,
+static const long frequency_list[] = {
+	2412, 2417, 2422, 2427, 2432, 2437, 2442,
 	2447, 2452, 2457, 2462, 2467, 2472, 2484
 };
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/16] staging: ks7010: refactor ks_wlan_set_wap function

[Sergio Paracuellos]

Make use of ether_addr_copy instead of memcpy for copying
ethernet address data in ks_wlan_set_wap function and avoid
an 'else' just changing if logic to check invalid values first.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 2fd5b50..690e5c0 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -313,18 +313,16 @@ static int ks_wlan_set_wap(struct net_device *dev, struct iw_request_info *info,
 		return -EPERM;
 
 	/* for SLEEP MODE */
-	if (priv->reg.operation_mode == MODE_ADHOC ||
-	    priv->reg.operation_mode == MODE_INFRASTRUCTURE) {
-		memcpy(priv->reg.bssid, &awrq->ap_addr.sa_data, ETH_ALEN);
-
-		if (is_valid_ether_addr((u8 *)priv->reg.bssid))
-			priv->need_commit |= SME_MODE_SET;
-
-	} else {
+	if (priv->reg.operation_mode != MODE_ADHOC &&
+	    priv->reg.operation_mode != MODE_INFRASTRUCTURE) {
 		eth_zero_addr(priv->reg.bssid);
 		return -EOPNOTSUPP;
 	}
 
+	ether_addr_copy(priv->reg.bssid, awrq->ap_addr.sa_data);
+	if (is_valid_ether_addr((u8 *)priv->reg.bssid))
+		priv->need_commit |= SME_MODE_SET;
+
 	netdev_dbg(dev, "bssid = %pM\n", priv->reg.bssid);
 
 	/* Write it to the card */
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/16] staging: ks7010: use ether_addr_copy in ks_wlan_get_wap

[Sergio Paracuellos]

Use ether_addr_copy to copy ethernet addresses instead
of a custom memcpy. This improves readability.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 690e5c0..6625d57 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -343,7 +343,7 @@ static int ks_wlan_get_wap(struct net_device *dev, struct iw_request_info *info,
 
 	/* for SLEEP MODE */
 	if (is_connect_status(priv->connect_status))
-		memcpy(awrq->ap_addr.sa_data, priv->current_ap.bssid, ETH_ALEN);
+		ether_addr_copy(awrq->ap_addr.sa_data, priv->current_ap.bssid);
 	else
 		eth_zero_addr(awrq->ap_addr.sa_data);
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 12/16] staging: ks7010: avoid an 'else' in ks_wlan_set_power

[Sergio Paracuellos]

Change if logic to handle invalid case for operation_mode
at first avoiding an 'else' path.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 6625d57..1387783 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1042,10 +1042,9 @@ static int ks_wlan_set_power(struct net_device *dev,
 	if (vwrq->power.disabled) {
 		priv->reg.power_mgmt = POWER_MGMT_ACTIVE;
 	} else {
-		if (priv->reg.operation_mode == MODE_INFRASTRUCTURE)
-			priv->reg.power_mgmt = POWER_MGMT_SAVE1;
-		else
+		if (priv->reg.operation_mode != MODE_INFRASTRUCTURE)
 			return -EINVAL;
+		priv->reg.power_mgmt = POWER_MGMT_SAVE1;
 	}
 
 	hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 13/16] staging: ks7010: avoid if-else condition in ks_wlan_get_power

[Sergio Paracuellos]

The if-else code in ks_wlan_get_power function is not needed
at all and can be achieved with a simple boolean assignation.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 1387783..df828d6 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1061,10 +1061,7 @@ static int ks_wlan_get_power(struct net_device *dev,
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
 	/* for SLEEP MODE */
-	if (priv->reg.power_mgmt > 0)
-		vwrq->power.disabled = 0;
-	else
-		vwrq->power.disabled = 1;
+	vwrq->power.disabled = (priv->reg.power_mgmt <= 0);
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 14/16] staging: ks7010: use ether_addr_copy in ks_wlan_get_aplist

[Sergio Paracuellos]

Use ether_addr_copy to copy ethernet addresses instead of
using custom memcpy for that.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index df828d6..b35284c 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1098,8 +1098,7 @@ static int ks_wlan_get_aplist(struct net_device *dev,
 		return -EPERM;
 	/* for SLEEP MODE */
 	for (i = 0; i < priv->aplist.size; i++) {
-		memcpy(address[i].sa_data, &(priv->aplist.ap[i].bssid[0]),
-		       ETH_ALEN);
+		ether_addr_copy(address[i].sa_data, priv->aplist.ap[i].bssid);
 		address[i].sa_family = ARPHRD_ETHER;
 		qual[i].level = 256 - priv->aplist.ap[i].rssi;
 		qual[i].qual = priv->aplist.ap[i].sq;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/16] staging: ks7010: avoid blank line between declarations in ks_wlan_get_aplist

[Sergio Paracuellos]

This commit removes a blank line which is included between
declarations and it has not sense at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index b35284c..b1933b3 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1091,7 +1091,6 @@ static int ks_wlan_get_aplist(struct net_device *dev,
 	struct ks_wlan_private *priv = netdev_priv(dev);
 	struct sockaddr *address = (struct sockaddr *)extra;
 	struct iw_quality qual[LOCAL_APLIST_MAX];
-
 	int i;
 
 	if (priv->sleep_mode == SLP_SLEEP)
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 16/16] staging: ks7010: refactor ks_wlan_translate_scan function

[Sergio Paracuellos]

This commit refactors ks_wlan_translate_scan function with
the following changes:

    - reorder local variables
    - use ether_addr_copy to copy ethernet addresses
    - change style in 'current_ev' variable assignments
    - make use of some ternaries avoiding if-else code
    - use preferred style for comments
    - extract common code into a new ks_wlan_add_leader_event
      function

After this changes readability has been improved.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks_wlan_net.c | 150 +++++++++++++++--------------------
 1 file changed, 66 insertions(+), 84 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index b1933b3..5c2cc75 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1143,6 +1143,33 @@ static int ks_wlan_set_scan(struct net_device *dev,
 	return 0;
 }
 
+static char *ks_wlan_add_leader_event(const char *rsn_leader, char *end_buf,
+				      char *current_ev, struct rsn_ie *rsn,
+				      struct iw_event *iwe,
+				      struct iw_request_info *info)
+{
+	char buffer[RSN_IE_BODY_MAX * 2 + 30];
+	char *pbuf;
+	int i;
+
+	pbuf = &buffer[0];
+	memset(iwe, 0, sizeof(*iwe));
+	iwe->cmd = IWEVCUSTOM;
+	memcpy(buffer, rsn_leader, sizeof(rsn_leader) - 1);
+	iwe->u.data.length += sizeof(rsn_leader) - 1;
+	pbuf += sizeof(rsn_leader) - 1;
+	pbuf += sprintf(pbuf, "%02x", rsn->id);
+	pbuf += sprintf(pbuf, "%02x", rsn->size);
+	iwe->u.data.length += 4;
+
+	for (i = 0; i < rsn->size; i++)
+		pbuf += sprintf(pbuf, "%02x", rsn->body[i]);
+
+	iwe->u.data.length += rsn->size * 2;
+
+	return iwe_stream_add_point(info, current_ev, end_buf, iwe, &buffer[0]);
+}
+
 /*
  * Translate scan data returned from the card to a card independent
  * format that the Wireless Tools will understand - Jean II
@@ -1153,22 +1180,19 @@ static inline char *ks_wlan_translate_scan(struct net_device *dev,
 					   struct local_ap *ap)
 {
 	/* struct ks_wlan_private *priv = (struct ks_wlan_private *)dev->priv; */
+	static const char rsn_leader[] = "rsn_ie=";
+	static const char wpa_leader[] = "wpa_ie=";
 	struct iw_event iwe;	/* Temporary buffer */
 	u16 capabilities;
 	char *current_val;	/* For rates */
 	int i;
-	static const char rsn_leader[] = "rsn_ie=";
-	static const char wpa_leader[] = "wpa_ie=";
-	char buf0[RSN_IE_BODY_MAX * 2 + 30];
-	char buf1[RSN_IE_BODY_MAX * 2 + 30];
-	char *pbuf;
+
 	/* First entry *MUST* be the AP MAC address */
 	iwe.cmd = SIOCGIWAP;
 	iwe.u.ap_addr.sa_family = ARPHRD_ETHER;
-	memcpy(iwe.u.ap_addr.sa_data, ap->bssid, ETH_ALEN);
-	current_ev =
-	    iwe_stream_add_event(info, current_ev, end_buf, &iwe,
-				 IW_EV_ADDR_LEN);
+	ether_addr_copy(iwe.u.ap_addr.sa_data, ap->bssid);
+	current_ev = iwe_stream_add_event(info, current_ev,
+					  end_buf, &iwe, IW_EV_ADDR_LEN);
 
 	/* Other entries will be displayed in the order we give them */
 
@@ -1178,21 +1202,17 @@ static inline char *ks_wlan_translate_scan(struct net_device *dev,
 		iwe.u.data.length = 32;
 	iwe.cmd = SIOCGIWESSID;
 	iwe.u.data.flags = 1;
-	current_ev =
-	    iwe_stream_add_point(info, current_ev, end_buf, &iwe,
-				 ap->ssid.body);
+	current_ev = iwe_stream_add_point(info, current_ev,
+					  end_buf, &iwe, ap->ssid.body);
 
 	/* Add mode */
 	iwe.cmd = SIOCGIWMODE;
 	capabilities = ap->capability;
 	if (capabilities & (WLAN_CAPABILITY_ESS | WLAN_CAPABILITY_IBSS)) {
-		if (capabilities & WLAN_CAPABILITY_ESS)
-			iwe.u.mode = IW_MODE_INFRA;
-		else
-			iwe.u.mode = IW_MODE_ADHOC;
-		current_ev =
-		    iwe_stream_add_event(info, current_ev, end_buf, &iwe,
-					 IW_EV_UINT_LEN);
+		iwe.u.mode = (capabilities & WLAN_CAPABILITY_ESS) ?
+			      IW_MODE_INFRA : IW_MODE_ADHOC;
+		current_ev = iwe_stream_add_event(info, current_ev,
+						  end_buf, &iwe, IW_EV_UINT_LEN);
 	}
 
 	/* Add frequency */
@@ -1200,32 +1220,29 @@ static inline char *ks_wlan_translate_scan(struct net_device *dev,
 	iwe.u.freq.m = ap->channel;
 	iwe.u.freq.m = frequency_list[iwe.u.freq.m - 1] * 100000;
 	iwe.u.freq.e = 1;
-	current_ev =
-	    iwe_stream_add_event(info, current_ev, end_buf, &iwe,
-				 IW_EV_FREQ_LEN);
+	current_ev = iwe_stream_add_event(info, current_ev,
+					  end_buf, &iwe, IW_EV_FREQ_LEN);
 
 	/* Add quality statistics */
 	iwe.cmd = IWEVQUAL;
 	iwe.u.qual.level = 256 - ap->rssi;
 	iwe.u.qual.qual = ap->sq;
 	iwe.u.qual.noise = 0;	/* invalid noise value */
-	current_ev =
-	    iwe_stream_add_event(info, current_ev, end_buf, &iwe,
-				 IW_EV_QUAL_LEN);
+	current_ev = iwe_stream_add_event(info, current_ev, end_buf,
+					  &iwe, IW_EV_QUAL_LEN);
 
 	/* Add encryption capability */
 	iwe.cmd = SIOCGIWENCODE;
-	if (capabilities & WLAN_CAPABILITY_PRIVACY)
-		iwe.u.data.flags = IW_ENCODE_ENABLED | IW_ENCODE_NOKEY;
-	else
-		iwe.u.data.flags = IW_ENCODE_DISABLED;
+	iwe.u.data.flags = (capabilities & WLAN_CAPABILITY_PRIVACY) ?
+			    (IW_ENCODE_ENABLED | IW_ENCODE_NOKEY) :
+			     IW_ENCODE_DISABLED;
 	iwe.u.data.length = 0;
-	current_ev =
-	    iwe_stream_add_point(info, current_ev, end_buf, &iwe,
-				 ap->ssid.body);
+	current_ev = iwe_stream_add_point(info, current_ev, end_buf,
+					  &iwe, ap->ssid.body);
 
-	/* Rate : stuffing multiple values in a single event require a bit
-	 * more of magic - Jean II
+	/*
+	 * Rate : stuffing multiple values in a single event
+	 * require a bit more of magic - Jean II
 	 */
 	current_val = current_ev + IW_EV_LCP_LEN;
 
@@ -1243,9 +1260,9 @@ static inline char *ks_wlan_translate_scan(struct net_device *dev,
 		/* Bit rate given in 500 kb/s units (+ 0x80) */
 		iwe.u.bitrate.value = ((ap->rate_set.body[i] & 0x7f) * 500000);
 		/* Add new value to event */
-		current_val =
-		    iwe_stream_add_value(info, current_ev, current_val, end_buf,
-					 &iwe, IW_EV_PARAM_LEN);
+		current_val = iwe_stream_add_value(info, current_ev,
+						   current_val, end_buf, &iwe,
+						   IW_EV_PARAM_LEN);
 	}
 	/* Check if we added any event */
 	if ((current_val - current_ev) > IW_EV_LCP_LEN)
@@ -1253,53 +1270,18 @@ static inline char *ks_wlan_translate_scan(struct net_device *dev,
 
 #define GENERIC_INFO_ELEM_ID 0xdd
 #define RSN_INFO_ELEM_ID 0x30
-	if (ap->rsn_ie.id == RSN_INFO_ELEM_ID && ap->rsn_ie.size != 0) {
-		pbuf = &buf0[0];
-		memset(&iwe, 0, sizeof(iwe));
-		iwe.cmd = IWEVCUSTOM;
-		memcpy(buf0, rsn_leader, sizeof(rsn_leader) - 1);
-		iwe.u.data.length += sizeof(rsn_leader) - 1;
-		pbuf += sizeof(rsn_leader) - 1;
-
-		pbuf += sprintf(pbuf, "%02x", ap->rsn_ie.id);
-		pbuf += sprintf(pbuf, "%02x", ap->rsn_ie.size);
-		iwe.u.data.length += 4;
-
-		for (i = 0; i < ap->rsn_ie.size; i++)
-			pbuf += sprintf(pbuf, "%02x", ap->rsn_ie.body[i]);
-		iwe.u.data.length += (ap->rsn_ie.size) * 2;
-
-		netdev_dbg(dev, "ap->rsn.size=%d\n", ap->rsn_ie.size);
-
-		current_ev =
-		    iwe_stream_add_point(info, current_ev, end_buf, &iwe,
-					 &buf0[0]);
-	}
-	if (ap->wpa_ie.id == GENERIC_INFO_ELEM_ID && ap->wpa_ie.size != 0) {
-		pbuf = &buf1[0];
-		memset(&iwe, 0, sizeof(iwe));
-		iwe.cmd = IWEVCUSTOM;
-		memcpy(buf1, wpa_leader, sizeof(wpa_leader) - 1);
-		iwe.u.data.length += sizeof(wpa_leader) - 1;
-		pbuf += sizeof(wpa_leader) - 1;
-
-		pbuf += sprintf(pbuf, "%02x", ap->wpa_ie.id);
-		pbuf += sprintf(pbuf, "%02x", ap->wpa_ie.size);
-		iwe.u.data.length += 4;
-
-		for (i = 0; i < ap->wpa_ie.size; i++)
-			pbuf += sprintf(pbuf, "%02x", ap->wpa_ie.body[i]);
-		iwe.u.data.length += (ap->wpa_ie.size) * 2;
-
-		netdev_dbg(dev, "ap->rsn.size=%d\n", ap->wpa_ie.size);
-		netdev_dbg(dev, "iwe.u.data.length=%d\n", iwe.u.data.length);
-
-		current_ev =
-		    iwe_stream_add_point(info, current_ev, end_buf, &iwe,
-					 &buf1[0]);
-	}
-
-	/* The other data in the scan result are not really
+	if (ap->rsn_ie.id == RSN_INFO_ELEM_ID && ap->rsn_ie.size != 0)
+		current_ev = ks_wlan_add_leader_event(rsn_leader, end_buf,
+						      current_ev, &ap->rsn_ie,
+						      &iwe, info);
+
+	if (ap->wpa_ie.id == GENERIC_INFO_ELEM_ID && ap->wpa_ie.size != 0)
+		current_ev = ks_wlan_add_leader_event(wpa_leader, end_buf,
+						      current_ev, &ap->wpa_ie,
+						      &iwe, info);
+
+	/*
+	 * The other data in the scan result are not really
 	 * interesting, so for now drop it - Jean II
 	 */
 	return current_ev;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 16/16] staging: ks7010: refactor ks_wlan_translate_scan function

[Dan Carpenter]

This really should have been broken up into several patches.  I've
reviewed the first 15 patches and they were straight forward.  Then I
postponed because it was just so grizzly to figure out what was going
on...

On Wed, Apr 25, 2018 at 04:01:55PM +0200, Sergio Paracuellos wrote:
> This commit refactors ks_wlan_translate_scan function with
> the following changes:
> 
>     - reorder local variables
>     - use ether_addr_copy to copy ethernet addresses
>     - change style in 'current_ev' variable assignments
>     - make use of some ternaries avoiding if-else code
>     - use preferred style for comments
>     - extract common code into a new ks_wlan_add_leader_event
>       function
> 

I have s script to ignore comment changes and review patches that
extract code into new functions but since this stuff is all jumbled
together then I have to review it manually.  Five of these things take
about 10 seconds to review when they're on their own, so 50 seconds
altogether.  Extracting the code takes maybe a minute to review.  So
let's say a total of two minutes.

But to review it after it's mixed up and then mashed into a pile takes
probably 15 minutes.

regards,
dan carpenter

Re: [PATCH 16/16] staging: ks7010: refactor ks_wlan_translate_scan function

[Sergio Paracuellos]

On Fri, Apr 27, 2018 at 02:00:29PM +0300, Dan Carpenter wrote:
> This really should have been broken up into several patches.  I've
> reviewed the first 15 patches and they were straight forward.  Then I
> postponed because it was just so grizzly to figure out what was going
> on...

Thanks for pointing this out. Because it was all related together
and changes were pretty straighforward I prefer to put them all
in the same patch and resume changes properly in the commit message. 
I'll try to group changes more properly from now. 

> 
> On Wed, Apr 25, 2018 at 04:01:55PM +0200, Sergio Paracuellos wrote:
> > This commit refactors ks_wlan_translate_scan function with
> > the following changes:
> > 
> >     - reorder local variables
> >     - use ether_addr_copy to copy ethernet addresses
> >     - change style in 'current_ev' variable assignments
> >     - make use of some ternaries avoiding if-else code
> >     - use preferred style for comments
> >     - extract common code into a new ks_wlan_add_leader_event
> >       function
> > 
> 
> I have s script to ignore comment changes and review patches that
> extract code into new functions but since this stuff is all jumbled
> together then I have to review it manually.  Five of these things take
> about 10 seconds to review when they're on their own, so 50 seconds
> altogether.  Extracting the code takes maybe a minute to review.  So
> let's say a total of two minutes.
> 
> But to review it after it's mixed up and then mashed into a pile takes
> probably 15 minutes.
> 

Thanks for your time for reviewing this, Dan.

> regards,
> dan carpenter
> 

Best regards,
    Sergio Paracuellos