[i2c] [PATCH] Lifebook apanel driver

[Jean Delvare]

Hi Stephen,

On Wed, 10 Jan 2007 11:50:59 -0800, Stephen Hemminger wrote:
> On Wed, 10 Jan 2007 09:22:09 +0100 Jean Delvare wrote:
> > Why isn't this structure dynamically allocated as all other i2c chip
> > drivers do? This is a requiement if you ever need to support more than
> > one device on a given system, and this is also much nicer IMHO.
> 
> There can only be one of these devices just like there can only be a single
> i801 adapter (similar to wistron buttons). Likewise the whole discovery process
> is tied up with looking at the BIOS to see what is there, and the hardware
> chips involved.

This is true at the moment, but might as well change in the future.

> Because of all this, it makes more sense to skip the allocation and have smaller code.

Actually, not really, which is why I was asking. OK, you have smaller
code, i.e. a smaller .text section, however, the static struct
apanel ends up in the .data section, and it's not exactly small:
approximatively 500 bytes on x86. The dynamic allocation and free code
takes like 50 bytes, so that's still a net benefit of ~450 bytes, or
10% of the driver binary size.

Your driver:
   text    data     bss     dec     hex filename
   2823    1472      56    4351    10ff drivers/input/misc/apanel.o
With my (untested) patch below:
   text    data     bss     dec     hex filename
   2879     968      56    3903     f3f drivers/input/misc/apanel.o

But well that's your driver, your decision.

* * * * *

Allocate the device structure dynamically, this cuts 10% of the driver
binary size.

Signed-off-by: Jean Delvare <khali at linux-fr.org>
---
 drivers/input/misc/apanel.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

--- linux-2.6.20-rc4.orig/drivers/input/misc/apanel.c	2007-01-10 20:59:02.000000000 +0100
+++ linux-2.6.20-rc4/drivers/input/misc/apanel.c	2007-01-10 21:09:29.000000000 +0100
@@ -245,6 +245,7 @@ static int apanel_detach_client(struct i
 	input_unregister_device(ap->input);
 
 	i2c_detach_client(&ap->client);
+	kfree(ap);
 	return 0;
 }
 
@@ -279,13 +280,6 @@ static int apanel_probe(struct i2c_adapt
 	const struct keymap *key;
 	struct input_dev *input;
 	int err = -ENOMEM;
-	static struct apanel apanel = {
-		.client = {
-			.name = "apanel",
-			.driver = &apanel_driver,
-		}
-
-	};
 
 	pr_debug("apanel: probe adapter %p addr %d kind %d\n",
 		 adap, addr, kind);
@@ -296,12 +290,16 @@ static int apanel_probe(struct i2c_adapt
 		return -EIO;
 	}
 
-	ap = &apanel;
+	ap = kzalloc(sizeof(struct apanel), GFP_KERNEL);
+	if (!ap)
+		goto out0;
 	input = input_allocate_device();
 	if (!input)
 		goto out1;
 
 	ap->input = input;
+	strlcpy(ap->client.name, "apanel", I2C_NAME_SIZE);
+	ap->client.driver = &apanel_driver,
 	ap->client.adapter = adap;
 	ap->client.addr = addr;
 	INIT_DELAYED_WORK(&ap->poll_timer, apanel_poll);
@@ -340,6 +338,8 @@ out3:
 out2:
 	input_free_device(input);
 out1:
+	kfree(ap);
+out0:
 	return err;
 }
 



-- 
Jean Delvare