diff mbox series

[v1,5/5] Input: edt-ft5x06 - allocate buffer once for debugging

Message ID 20200303180917.12563-5-andriy.shevchenko@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/5] Input: of_touchscreen - explicitly choose axis | expand

Commit Message

Andy Shevchenko March 3, 2020, 6:09 p.m. UTC 
There is no need to allocate buffer each time we switch modes. First of all,
the code is protected by checking the factory_mode state. The size of the
buffer is static and can't be changed after ->probe() anyway.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

Comments

Dmitry Torokhov March 7, 2020, 12:57 a.m. UTC | #1 
Hi Andy,

On Tue, Mar 03, 2020 at 08:09:17PM +0200, Andy Shevchenko wrote:
> There is no need to allocate buffer each time we switch modes. First of all,
> the code is protected by checking the factory_mode state. The size of the
> buffer is static and can't be changed after ->probe() anyway.

Why do we need to keep memory allocated if it is not going to be used
majority of the time? How much is the code savings vs. allocated memory
size (without considering having multiple devices connected to the same
system).

Thanks.
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 1023d4134b8d..3895e194006a 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -544,16 +544,6 @@  static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
 
 	disable_irq(client->irq);
 
-	if (!tsdata->raw_buffer) {
-		tsdata->raw_bufsize = tsdata->num_x * tsdata->num_y *
-				      sizeof(u16);
-		tsdata->raw_buffer = kzalloc(tsdata->raw_bufsize, GFP_KERNEL);
-		if (!tsdata->raw_buffer) {
-			error = -ENOMEM;
-			goto err_out;
-		}
-	}
-
 	/* mode register is 0x3c when in the work mode */
 	error = edt_ft5x06_register_write(tsdata, WORK_REGISTER_OPMODE, 0x03);
 	if (error) {
@@ -581,8 +571,6 @@  static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
 	return 0;
 
 err_out:
-	kfree(tsdata->raw_buffer);
-	tsdata->raw_buffer = NULL;
 	tsdata->factory_mode = false;
 	enable_irq(client->irq);
 
@@ -622,9 +610,6 @@  static int edt_ft5x06_work_mode(struct edt_ft5x06_ts_data *tsdata)
 		return -EIO;
 	}
 
-	kfree(tsdata->raw_buffer);
-	tsdata->raw_buffer = NULL;
-
 	/* restore parameters */
 	edt_ft5x06_register_write(tsdata, reg_addr->reg_threshold,
 				  tsdata->threshold);
@@ -697,7 +682,7 @@  static ssize_t edt_ft5x06_debugfs_raw_data_read(struct file *file,
 
 	mutex_lock(&tsdata->mutex);
 
-	if (!tsdata->factory_mode || !tsdata->raw_buffer) {
+	if (!tsdata->factory_mode) {
 		error = -EIO;
 		goto out;
 	}
@@ -774,6 +759,12 @@  edt_ft5x06_ts_prepare_debugfs(struct edt_ft5x06_ts_data *tsdata,
 
 	debugfs_create_file("mode", S_IRUSR | S_IWUSR,
 			    tsdata->debug_dir, tsdata, &debugfs_mode_fops);
+
+	tsdata->raw_bufsize = tsdata->num_x * tsdata->num_y * sizeof(u16);
+	tsdata->raw_buffer = kzalloc(tsdata->raw_bufsize, GFP_KERNEL);
+	if (!tsdata->raw_buffer)
+		return;
+
 	debugfs_create_file("raw_data", S_IRUSR,
 			    tsdata->debug_dir, tsdata, &debugfs_raw_data_fops);
 }