diff mbox

[RESEND,4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer

Message ID 1408536812-7836-5-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State Superseded
Headers show

Commit Message

Javier Martinez Canillas Aug. 20, 2014, 12:13 p.m. UTC
From: Andrew Bresticker <abrestic@chromium.org>

Now that there's a central cros_ec_cmd_xfer(), move the locking
out of the SPI and LPC drivers.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/cros_ec.c     | 10 +++++++++-
 drivers/mfd/cros_ec_spi.c | 11 -----------
 2 files changed, 9 insertions(+), 12 deletions(-)

Comments

Doug Anderson Aug. 20, 2014, 10:36 p.m. UTC | #1
Javier,

On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> Now that there's a central cros_ec_cmd_xfer(), move the locking
> out of the SPI and LPC drivers.

Slight nit that the LPC driver doesn't exist upstream.  This is in
prep for adding the LPC driver, though.


> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>  drivers/mfd/cros_ec_spi.c | 11 -----------
>  2 files changed, 9 insertions(+), 12 deletions(-)

After comment nitfix:

Reviewed-by: Doug Anderson <dianders@chromium.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Aug. 21, 2014, 10:24 a.m. UTC | #2
Hello Doug,

On 08/21/2014 12:36 AM, Doug Anderson wrote:
> Javier,
> 
> On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> From: Andrew Bresticker <abrestic@chromium.org>
>>
>> Now that there's a central cros_ec_cmd_xfer(), move the locking
>> out of the SPI and LPC drivers.
> 
> Slight nit that the LPC driver doesn't exist upstream.  This is in
> prep for adding the LPC driver, though.
> 

Right, the downstream commit was also touching the LPC driver and I
stripped that part but forget to update the commit message, sorry about
that. I'll fix it and do a re-spin.

> 
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>>  drivers/mfd/cros_ec_spi.c | 11 -----------
>>  2 files changed, 9 insertions(+), 12 deletions(-)
> 
> After comment nitfix:
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> 

Thanks and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones Aug. 21, 2014, 2:09 p.m. UTC | #3
On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> Now that there's a central cros_ec_cmd_xfer(), move the locking
> out of the SPI and LPC drivers.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>  drivers/mfd/cros_ec_spi.c | 11 -----------
>  2 files changed, 9 insertions(+), 12 deletions(-)

Acked-by: Lee Jones <lee.jones@linaro.org>
diff mbox

Patch

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index a9faebd..c53804a 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -65,7 +65,13 @@  EXPORT_SYMBOL(cros_ec_check_result);
 int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 		     struct cros_ec_command *msg)
 {
-	return ec_dev->cmd_xfer(ec_dev, msg);
+	int ret;
+
+	mutex_lock(&ec_dev->lock);
+	ret = ec_dev->cmd_xfer(ec_dev, msg);
+	mutex_unlock(&ec_dev->lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
@@ -98,6 +104,8 @@  int cros_ec_register(struct cros_ec_device *ec_dev)
 			return -ENOMEM;
 	}
 
+	mutex_init(&ec_dev->lock);
+
 	err = mfd_add_devices(dev, 0, cros_devs,
 			      ARRAY_SIZE(cros_devs),
 			      NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index b396705..bf6e08e 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -79,13 +79,11 @@ 
  *	if no record
  * @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
  *      is sent when we want to turn off CS at the end of a transaction.
- * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
  */
 struct cros_ec_spi {
 	struct spi_device *spi;
 	s64 last_transfer_ns;
 	unsigned int end_of_msg_delay;
-	struct mutex lock;
 };
 
 static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -232,13 +230,6 @@  static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 	int sum;
 	int ret = 0, final_ret;
 
-	/*
-	 * We have the shared ec_dev buffer plus we do lots of separate spi_sync
-	 * calls, so we need to make sure only one person is using this at a
-	 * time.
-	 */
-	mutex_lock(&ec_spi->lock);
-
 	len = cros_ec_prepare_tx(ec_dev, ec_msg);
 	dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
 
@@ -327,7 +318,6 @@  exit:
 	if (ec_msg->command == EC_CMD_REBOOT_EC)
 		msleep(EC_REBOOT_DELAY_MS);
 
-	mutex_unlock(&ec_spi->lock);
 	return ret;
 }
 
@@ -359,7 +349,6 @@  static int cros_ec_spi_probe(struct spi_device *spi)
 	if (ec_spi == NULL)
 		return -ENOMEM;
 	ec_spi->spi = spi;
-	mutex_init(&ec_spi->lock);
 	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
 	if (!ec_dev)
 		return -ENOMEM;