mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Jiri Kosina <jikos@kernel.org>,
	Andrew Duggan <aduggan@synaptics.com>,
	Lyude Paul <thatslyude@gmail.com>, Nick Dyer <nick@shmanahar.org>,
	Dennis Wassenberg <dennis.wassenberg@secunet.com>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 04/13] Input: synaptics-rmi4 - remove mutex calls while updating the firmware
Date: Tue, 29 Nov 2016 11:08:15 +0100	[thread overview]
Message-ID: <1480414104-8524-5-git-send-email-benjamin.tissoires@redhat.com> (raw)
In-Reply-To: <1480414104-8524-1-git-send-email-benjamin.tissoires@redhat.com>

This partially reverts commit 29fd0ec2bdbe ("Input: synaptics-rmi4 -
add support for F34 device reflash")

irq_mutex should be used only to protect data->current_irq_mask, not
preventing incoming input to be processed while the upgrade of the
firmware is happening. We can simply disable the irqs when we don't
want them to interfere with the upgrade process.

Tested on S7300 and S7800 (with F34 v7 patch added)

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Nick Dyer <nick@shmanahar.org>
---
 drivers/input/rmi4/rmi_driver.c | 40 ++++++++--------------------------------
 drivers/input/rmi4/rmi_f34.c    | 19 ++++++++++++++-----
 2 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index f04fc41..27c731a 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -42,8 +42,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 
 	rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev, "Freeing function list\n");
 
-	mutex_lock(&data->irq_mutex);
-
 	devm_kfree(&rmi_dev->dev, data->irq_memory);
 	data->irq_memory = NULL;
 	data->irq_status = NULL;
@@ -60,8 +58,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 		list_del(&fn->node);
 		rmi_unregister_function(fn);
 	}
-
-	mutex_unlock(&data->irq_mutex);
 }
 EXPORT_SYMBOL_GPL(rmi_free_function_list);
 
@@ -160,25 +156,24 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	if (!data)
 		return 0;
 
-	mutex_lock(&data->irq_mutex);
-	if (!data->irq_status || !data->f01_container) {
-		mutex_unlock(&data->irq_mutex);
-		return 0;
-	}
-
 	if (!rmi_dev->xport->attn_data) {
 		error = rmi_read_block(rmi_dev,
 				data->f01_container->fd.data_base_addr + 1,
 				data->irq_status, data->num_of_irq_regs);
 		if (error < 0) {
 			dev_err(dev, "Failed to read irqs, code=%d\n", error);
-			mutex_unlock(&data->irq_mutex);
 			return error;
 		}
 	}
 
+	mutex_lock(&data->irq_mutex);
 	bitmap_and(data->irq_status, data->irq_status, data->current_irq_mask,
 	       data->irq_count);
+	/*
+	 * At this point, irq_status has all bits that are set in the
+	 * interrupt status register and are enabled.
+	 */
+	mutex_unlock(&data->irq_mutex);
 
 	/*
 	 * It would be nice to be able to use irq_chip to handle these
@@ -194,8 +189,6 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	if (data->input)
 		input_sync(data->input);
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -263,18 +256,12 @@ static int rmi_suspend_functions(struct rmi_device *rmi_dev)
 	struct rmi_function *entry;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	list_for_each_entry(entry, &data->function_list, node) {
 		retval = suspend_one_function(entry);
-		if (retval < 0) {
-			mutex_unlock(&data->irq_mutex);
+		if (retval < 0)
 			return retval;
-		}
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -303,18 +290,12 @@ static int rmi_resume_functions(struct rmi_device *rmi_dev)
 	struct rmi_function *entry;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	list_for_each_entry(entry, &data->function_list, node) {
 		retval = resume_one_function(entry);
-		if (retval < 0) {
-			mutex_unlock(&data->irq_mutex);
+		if (retval < 0)
 			return retval;
-		}
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -1043,8 +1024,6 @@ int rmi_init_functions(struct rmi_driver_data *data)
 	int irq_count;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	irq_count = 0;
 	rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Creating functions.\n", __func__);
 	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
@@ -1069,13 +1048,10 @@ int rmi_init_functions(struct rmi_driver_data *data)
 		goto err_destroy_functions;
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 
 err_destroy_functions:
 	rmi_free_function_list(rmi_dev);
-	mutex_unlock(&data->irq_mutex);
 	return retval;
 }
 EXPORT_SYMBOL_GPL(rmi_init_functions);
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index 03df85a..01936a4 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -282,7 +282,8 @@ int rmi_f34_update_firmware(struct f34_data *f34, const struct firmware *fw)
 static int rmi_firmware_update(struct rmi_driver_data *data,
 			       const struct firmware *fw)
 {
-	struct device *dev = &data->rmi_dev->dev;
+	struct rmi_device *rmi_dev = data->rmi_dev;
+	struct device *dev = &rmi_dev->dev;
 	struct f34_data *f34;
 	int ret;
 
@@ -305,8 +306,10 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 	if (ret)
 		return ret;
 
+	rmi_disable_irq(rmi_dev, false);
+
 	/* Tear down functions and re-probe */
-	rmi_free_function_list(data->rmi_dev);
+	rmi_free_function_list(rmi_dev);
 
 	ret = rmi_probe_interrupts(data);
 	if (ret)
@@ -322,6 +325,8 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 		return -EINVAL;
 	}
 
+	rmi_enable_irq(rmi_dev, false);
+
 	f34 = dev_get_drvdata(&data->f34_container->dev);
 
 	/* Perform firmware update */
@@ -329,11 +334,13 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 
 	dev_info(&f34->fn->dev, "Firmware update complete, status:%d\n", ret);
 
+	rmi_disable_irq(rmi_dev, false);
+
 	/* Re-probe */
 	rmi_dbg(RMI_DEBUG_FN, dev, "Re-probing device\n");
-	rmi_free_function_list(data->rmi_dev);
+	rmi_free_function_list(rmi_dev);
 
-	ret = rmi_scan_pdt(data->rmi_dev, NULL, rmi_initial_reset);
+	ret = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
 	if (ret < 0)
 		dev_warn(dev, "RMI reset failed!\n");
 
@@ -345,9 +352,11 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 	if (ret)
 		return ret;
 
+	rmi_enable_irq(rmi_dev, false);
+
 	if (data->f01_container->dev.driver)
 		/* Driver already bound, so enable ATTN now. */
-		return rmi_enable_sensor(data->rmi_dev);
+		return rmi_enable_sensor(rmi_dev);
 
 	rmi_dbg(RMI_DEBUG_FN, dev, "%s complete\n", __func__);
 
-- 
2.7.4

  parent reply	other threads:[~2016-11-29 10:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 02/13] Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 03/13] Input: synaptics-rmi4 - add rmi_enable/disable_irq Benjamin Tissoires
2016-11-29 10:08 ` Benjamin Tissoires [this message]
2016-11-29 10:08 ` [PATCH 05/13] Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal functions Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 06/13] Input: synaptics-rmi4 - have only one struct platform data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 07/13] Input: synaptics-rmi4 - add support for F03 Benjamin Tissoires
2016-12-01  1:36   ` Dmitry Torokhov
2016-12-01 14:54     ` Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 08/13] Input: synaptics-rmi4 - f03: grab data passed by transport device Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 09/13] Input: synaptics-rmi4 - allow to add attention data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 10/13] Input: synaptics-rmi4 - store the attn data in the driver Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 11/13] HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4 Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 12/13] HID: rmi: Handle all Synaptics touchpads using hid-rmi Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 13/13] HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock " Benjamin Tissoires

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480414104-8524-5-git-send-email-benjamin.tissoires@redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=aduggan@synaptics.com \
    --cc=dennis.wassenberg@secunet.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nick@shmanahar.org \
    --cc=thatslyude@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®