Thread (18 messages) 18 messages, 4 authors, 2009-10-01

RE: [PATCH] ks8851_ml ethernet network driver

From: Choi, David <hidden>
Date: 2009-09-17 19:31:09

Hello David Miller,

Sorry to resend it. In my previous e-mail, I included a part of patch,
not a complete patch.

My fix ups are as followings;
	-Remove DEBUG definition
	-Remove MALLOC definition
	-Intent to fix compile warnings, which I can not reproduce
	 in my test environment(linux-2.6.31-rc3 source tree and
gcc4.2.1.)
	 If you have still warnings with my fix ups, give me brief
description 
	 To reproduce the warnings.

===================
--- linux-2.6.31-rc3/drivers/net/ks8851_mll.c.orig	2009-09-17
10:18:56.000000000 -0700
+++ linux-2.6.31-rc3/drivers/net/ks8851_mll.c	2009-09-17
10:09:37.000000000 -0700
@@ -21,8 +21,6 @@
  * KS8851 16bit MLL chip from Micrel Inc.
  */
 
-#define DEBUG
-
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
@@ -419,7 +417,6 @@ union ks_tx_hdr {
  * or one of the work queues.
  *
  */
-#define MALLOC(x)		kmalloc(x, GFP_KERNEL)
 
 /* Receive multiplex framer header info */
 struct type_frame_head {
@@ -552,11 +549,9 @@ static void ks_wrreg16(struct ks_net *ks
  */
 static inline void ks_inblk(struct ks_net *ks, u16 *wptr, u32 len)
 {
-	u32 data_port = (u32)ks->hw_addr;
 	len >>= 1;
-	do {
-		*wptr++ = (u16)ioread16(data_port);
-	} while (--len);
+	while (len--)
+		*wptr++ = (u16)ioread16(ks->hw_addr);
 }


 /**
@@ -568,11 +563,9 @@ static inline void ks_inblk(struct ks_ne
  */
 static inline void ks_outblk(struct ks_net *ks, u16 *wptr, u32 len)
 {
-	u32 data_port = (u32)ks->hw_addr;
 	len >>= 1;
-	do {
-		iowrite16(*wptr++, data_port);
-	} while (--len);
+	while (len--)
+		iowrite16(*wptr++, ks->hw_addr);
 }
@@ -1515,12 +1510,13 @@ void ks_enable(struct ks_net *ks)
 
 static int ks_hw_init(struct ks_net *ks)
 {
+#define	MHEADER_SIZE	(sizeof(struct type_frame_head) *
MAX_RECV_FRAMES)
 	ks->promiscuous = 0;
 	ks->all_mcast = 0;
 	ks->mcast_lst_size = 0;
 
 	ks->frame_head_info = (struct type_frame_head *) \
-		MALLOC(sizeof(struct type_frame_head) *
MAX_RECV_FRAMES);
+		kmalloc(MHEADER_SIZE, GFP_KERNEL);
 	if (!ks->frame_head_info) {
 		printk(KERN_ERR "Error: Fail to allocate frame
memory\n");
 		return false;



Regards,
David J. Choi



-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Wednesday, September 16, 2009 8:48 PM
To: greg@kroah.com
Cc: netdev@vger.kernel.org; Li, Charles; Choi@kroah.com; Choi, David;
jgarzik@redhat.com; shemminger@vyatta.com
Subject: Re: [PATCH] ks8851_ml ethernet network driver

From: Greg KH <redacted>
Date: Wed, 16 Sep 2009 19:38:36 -0700
From: Choi, David <redacted>

This is a network driver for the ks8851 16bit MLL ethernet device.

Signed-off-by: David J. Choi <redacted>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This doesn't even build cleanly:

drivers/net/ks8851_mll.c: In function 'ks_inblk':
drivers/net/ks8851_mll.c:555: warning: cast from pointer to integer of
different size
drivers/net/ks8851_mll.c:558: warning: passing argument 1 of '_readw'
makes pointer from integer without a cast
drivers/net/ks8851_mll.c: In function 'ks_outblk':
drivers/net/ks8851_mll.c:571: warning: cast from pointer to integer of
different size
drivers/net/ks8851_mll.c:574: warning: passing argument 2 of '_writew'
makes pointer from integer without a cast

It also has a big "#define DEBUG" at the beginning of the driver.

And it also has stuff like:

+#define MALLOC(x)		kmalloc(x, GFP_KERNEL)

which actually decreases the readability of this driver.

Please fix this up.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help