[PATCH 0/5] staging/wlags49_h2: fixed up wl_cs.c

[PATCH 0/5] staging/wlags49_h2: fixed up wl_cs.c

[Stephan Gabert]

The appended patches fix up wl_cs.c from the wireless device driver wlags49
of Agere Systems Inc.

Most modifications resulted from checkpatch.pl.

Stephan Gabert (5):
  staging/wlags49_h2: checkpatch: including <linux/*.h> instead of
    <asm/*.h>
  staging/wlags49_h2: checkpatch: added spaces around equal sign
  staging/wlags49_h2: correct check of the return value of
    register_netdev()
  staging/wlags49_h2: convert printk calls to netdev_* calls
  staging/wlags49_h2: checkpatch: line over 80 characters

 drivers/staging/wlags49_h2/wl_cs.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

-- 
1.9.1

[PATCH 1/5] staging/wlags49_h2: checkpatch: including <linux/*.h> instead of <asm/*.h>

[Stephan Gabert]

As warned by checkpatch.pl, one should use #include <linux/io.h>
instead of #include <asm/io.h> and #include <linux/bitops.h>
instead of #include <asm/bitops.h>.

Signed-off-by: Stephan Gabert <stephan.gabert@fau.de>
Signed-off-by: Nicolas Pfeiffer <nicolas.pfeiffer@fau.de>
---
 drivers/staging/wlags49_h2/wl_cs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index 3f7cf41..5c6e18c 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -73,8 +73,8 @@
 #include <linux/interrupt.h>
 #include <linux/in.h>
 #include <linux/delay.h>
-#include <asm/io.h>
-#include <asm/bitops.h>
+#include <linux/io.h>
+#include <linux/bitops.h>
 
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
-- 
1.9.1

[PATCH 2/5] staging/wlags49_h2: checkpatch: added spaces around equal sign

[Stephan Gabert]

Hereby the checkpatch message
  ERROR: spaces required around that '=' (ctx:VxV)
gets resolved
and the neighboring equal signs are positioned in the same column.

Signed-off-by: Stephan Gabert <stephan.gabert@fau.de>
Signed-off-by: Nicolas Pfeiffer <nicolas.pfeiffer@fau.de>
---
 drivers/staging/wlags49_h2/wl_cs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index 5c6e18c..c79f1ed 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -131,11 +131,11 @@ static int wl_adapter_attach(struct pcmcia_device *link)
 		return -ENOMEM;
 	}
 
-	link->resource[0]->end  = HCF_NUM_IO_PORTS;
-	link->resource[0]->flags= IO_DATA_PATH_WIDTH_16;
-	link->config_flags     |= CONF_ENABLE_IRQ;
-	link->config_index      = 5;
-	link->config_regs       = PRESENT_OPTION;
+	link->resource[0]->end   = HCF_NUM_IO_PORTS;
+	link->resource[0]->flags = IO_DATA_PATH_WIDTH_16;
+	link->config_flags      |= CONF_ENABLE_IRQ;
+	link->config_index       = 5;
+	link->config_regs        = PRESENT_OPTION;
 
 	link->priv = dev;
 	lp = wl_priv(dev);
-- 
1.9.1

[PATCH 3/5] staging/wlags49_h2: correct check of the return value of register_netdev()

[Stephan Gabert]

As mentioned in net/core/dev.c register_netdev() explicitly returns a
negative errno code on failure.

So in case of failure, one should rather test whether ret is negative
than just unlike 0.

Signed-off-by: Stephan Gabert <stephan.gabert@fau.de>
Signed-off-by: Nicolas Pfeiffer <nicolas.pfeiffer@fau.de>
---
 drivers/staging/wlags49_h2/wl_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index c79f1ed..764f547 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -232,7 +232,7 @@ int wl_adapter_insert(struct pcmcia_device *link)
 
 	SET_NETDEV_DEV(dev, &link->dev);
 	ret = register_netdev(dev);
-	if (ret != 0) {
+	if (ret < 0) {
 		printk("%s: register_netdev() failed\n", KBUILD_MODNAME);
 		goto failed;
 	}
-- 
1.9.1

[PATCH 4/5] staging/wlags49_h2: convert printk calls to netdev_* calls

[Stephan Gabert]

Use netdev_* where applicable.

Signed-off-by: Stephan Gabert <stephan.gabert@fau.de>
Signed-off-by: Nicolas Pfeiffer <nicolas.pfeiffer@fau.de>
---
 drivers/staging/wlags49_h2/wl_cs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index 764f547..e658000 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -233,12 +233,12 @@ int wl_adapter_insert(struct pcmcia_device *link)
 	SET_NETDEV_DEV(dev, &link->dev);
 	ret = register_netdev(dev);
 	if (ret < 0) {
-		printk("%s: register_netdev() failed\n", KBUILD_MODNAME);
+		netdev_err(dev, "register_netdev() failed\n");
 		goto failed;
 	}
 
-	printk(KERN_INFO "%s: Wireless, io_addr %#03lx, irq %d, mac_address"
-		" %pM\n", dev->name, dev->base_addr, dev->irq, dev->dev_addr);
+	netdev_info(dev, "io_addr %#03lx, irq %d, mac_address %pM\n",
+			dev->base_addr, dev->irq, dev->dev_addr);
 
 	return 0;
 
-- 
1.9.1

[PATCH 5/5] staging/wlags49_h2: checkpatch: line over 80 characters

[Stephan Gabert]

Signed-off-by: Stephan Gabert <stephan.gabert@fau.de>
Signed-off-by: Nicolas Pfeiffer <nicolas.pfeiffer@fau.de>
---
 drivers/staging/wlags49_h2/wl_cs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index e658000..4239f0c 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -340,8 +340,9 @@ static const struct pcmcia_device_id wl_adapter_ids[] = {
 				0x33103a9b, 0xe175b0dd),
 #else
 	PCMCIA_DEVICE_MANF_CARD(0x0156, 0x0004),
-	PCMCIA_DEVICE_PROD_ID12("Linksys", "WCF54G_Wireless-G_CompactFlash_Card",
-				0x0733cc81, 0x98a599e1),
+	PCMCIA_DEVICE_PROD_ID12("Linksys",
+			"WCF54G_Wireless-G_CompactFlash_Card", 0x0733cc81,
+			0x98a599e1),
 #endif  /* (HCF_TYPE) & HCF_TYPE_HII5 */
 	PCMCIA_DEVICE_NULL,
 };
-- 
1.9.1

Re: [PATCH 3/5] staging/wlags49_h2: correct check of the return value of register_netdev()

[Dan Carpenter]

On Mon, Jun 16, 2014 at 04:50:51PM +0200, Stephan Gabert wrote:
> As mentioned in net/core/dev.c register_netdev() explicitly returns a
> negative errno code on failure.
> 
> So in case of failure, one should rather test whether ret is negative
> than just unlike 0.

No.  In the kernel the normal way is to say:

	if (ret)
		return ret;

Zero is succes and non-zero is error code.

	if (ret != 0)
		return ret;

That's a double negative and pointlessly confusing.

	if (ret != 0 != 0 != 0)

That's a hextuple negative and awesomely confusing.

There are times where a double negative is ok.  When you are talking
about numbers specifically:

	if (ret != 0 && ret != 3) {

That means ret is not zero or three, but zero doesn't mean success or
failure, it's just a number.

For strcmp() functions you should always compare against zero
because that is the idiom.

	if (strcmp(foo, bar) < 0)
	if (strcmp(foo, bar) != 0)

The first "<" means "foo" is before "bar" and "!=" means not equal.

	if (ret < 0)
		return ret;

Probably means that now ret is either zero or a positive value???  It is
ambiguous.

regards,
dan carpenter