[PATCH] staging: nvec: Make nvec_write_sync return result by parameter

[PATCH] staging: nvec: Make nvec_write_sync return result by parameter

[Evan Hauck]

As outlined in the TODO file, nvec_write_sync was modified to return an error
code instead of NULL on failure, and return the nvec_msg as a parameter.

Signed-off-by: Evan Hauck <echauck@mtu.edu>
---
 drivers/staging/nvec/TODO   |  2 --
 drivers/staging/nvec/nvec.c | 37 +++++++++++++++++++++----------------
 drivers/staging/nvec/nvec.h |  5 +++--
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/nvec/TODO b/drivers/staging/nvec/TODO
index e5ae42a..e4d85d9 100644
--- a/drivers/staging/nvec/TODO
+++ b/drivers/staging/nvec/TODO
@@ -3,6 +3,4 @@ ToDo list (incomplete, unordered)
 	- move half of the nvec init stuff to i2c-tegra.c
 	- move event handling to nvec_events
 	- finish suspend/resume support
-	- modifiy the sync_write method to return the received
-	  message in a variable (and return the error code).
 	- add support for more device implementations
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 120b70d..47ef013 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -288,46 +288,53 @@ EXPORT_SYMBOL(nvec_write_async);
  * @nvec: An &struct nvec_chip
  * @data: The data to write
  * @size: The size of @data
+ * @result: The resulting message
  *
  * This is similar to nvec_write_async(), but waits for the
  * request to be answered before returning. This function
  * uses a mutex and can thus not be called from e.g.
  * interrupt handlers.
  *
- * Returns: A pointer to the response message on success,
+ * Returns in *result: A pointer to the response message on success,
  * %NULL on failure. Free with nvec_msg_free() once no longer
  * used.
+ *
+ * Returns: 0 on success, a negative error code on failure. If a failure
+ * occurred, the nvec driver may print an error.
  */
-struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
-		const unsigned char *data, short size)
+int nvec_write_sync(struct nvec_chip *nvec,
+		const unsigned char *data, short size,
+		struct nvec_msg **msg)
 {
-	struct nvec_msg *msg;
+	int err;
 
 	mutex_lock(&nvec->sync_write_mutex);
 
 	nvec->sync_write_pending = (data[1] << 8) + data[0];
 
-	if (nvec_write_async(nvec, data, size) < 0) {
+	err = nvec_write_async(nvec, data, size);
+	if (err < 0) {
 		mutex_unlock(&nvec->sync_write_mutex);
-		return NULL;
+		return err;
 	}
 
 	dev_dbg(nvec->dev, "nvec_sync_write: 0x%04x\n",
 					nvec->sync_write_pending);
-	if (!(wait_for_completion_timeout(&nvec->sync_write,
-				msecs_to_jiffies(2000)))) {
+	err = wait_for_completion_timeout(&nvec->sync_write,
+			msecs_to_jiffies(2000));
+	if (!err) {
 		dev_warn(nvec->dev, "timeout waiting for sync write to complete\n");
 		mutex_unlock(&nvec->sync_write_mutex);
-		return NULL;
+		return err;
 	}
 
 	dev_dbg(nvec->dev, "nvec_sync_write: pong!\n");
 
-	msg = nvec->last_sync_msg;
+	*msg = nvec->last_sync_msg;
 
 	mutex_unlock(&nvec->sync_write_mutex);
 
-	return msg;
+	return 0;
 }
 EXPORT_SYMBOL(nvec_write_sync);
 
@@ -879,9 +886,7 @@ static int tegra_nvec_probe(struct platform_device *pdev)
 	pm_power_off = nvec_power_off;
 
 	/* Get Firmware Version */
-	msg = nvec_write_sync(nvec, get_firmware_version, 2);
-
-	if (msg) {
+	if (!nvec_write_sync(nvec, get_firmware_version, 2, &msg)) {
 		dev_warn(nvec->dev, "ec firmware version %02x.%02x.%02x / %02x\n",
 			msg->data[4], msg->data[5], msg->data[6], msg->data[7]);
 
@@ -935,8 +940,8 @@ static int nvec_suspend(struct device *dev)
 	/* keep these sync or you'll break suspend */
 	nvec_toggle_global_events(nvec, false);
 
-	msg = nvec_write_sync(nvec, ap_suspend, sizeof(ap_suspend));
-	nvec_msg_free(nvec, msg);
+	if (!nvec_write_sync(nvec, ap_suspend, sizeof(ap_suspend), &msg))
+		nvec_msg_free(nvec, msg);
 
 	nvec_disable_i2c_slave(nvec);
 
diff --git a/drivers/staging/nvec/nvec.h b/drivers/staging/nvec/nvec.h
index e271375..00d099a 100644
--- a/drivers/staging/nvec/nvec.h
+++ b/drivers/staging/nvec/nvec.h
@@ -168,8 +168,9 @@ struct nvec_chip {
 extern int nvec_write_async(struct nvec_chip *nvec, const unsigned char *data,
 			     short size);
 
-extern struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
-					const unsigned char *data, short size);
+extern int nvec_write_sync(struct nvec_chip *nvec,
+			   const unsigned char *data, short size,
+			   struct nvec_msg **result);
 
 extern int nvec_register_notifier(struct nvec_chip *nvec,
 				  struct notifier_block *nb,
-- 
2.3.0

Re: [PATCH] staging: nvec: Make nvec_write_sync return result by parameter

[Marc Dietrich]

[-- Attachment #1: Type: text/plain, Size: 4916 bytes --]

Hi Even,

Am Freitag, 13. Februar 2015, 21:28:48 schrieb Evan Hauck:
> As outlined in the TODO file, nvec_write_sync was modified to return an
> error code instead of NULL on failure, and return the nvec_msg as a
> parameter.
> 
> Signed-off-by: Evan Hauck <echauck@mtu.edu>

thanks for looking into this issue! I tested this and saw no issue. You 
confused msg and result variables below. Can you re-send with this problem 
fixed?

Marc

> ---
>  drivers/staging/nvec/TODO   |  2 --
>  drivers/staging/nvec/nvec.c | 37 +++++++++++++++++++++----------------
>  drivers/staging/nvec/nvec.h |  5 +++--
>  3 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/nvec/TODO b/drivers/staging/nvec/TODO
> index e5ae42a..e4d85d9 100644
> --- a/drivers/staging/nvec/TODO
> +++ b/drivers/staging/nvec/TODO
> @@ -3,6 +3,4 @@ ToDo list (incomplete, unordered)
>  	- move half of the nvec init stuff to i2c-tegra.c
>  	- move event handling to nvec_events
>  	- finish suspend/resume support
> -	- modifiy the sync_write method to return the received
> -	  message in a variable (and return the error code).
>  	- add support for more device implementations
> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> index 120b70d..47ef013 100644
> --- a/drivers/staging/nvec/nvec.c
> +++ b/drivers/staging/nvec/nvec.c
> @@ -288,46 +288,53 @@ EXPORT_SYMBOL(nvec_write_async);
>   * @nvec: An &struct nvec_chip
>   * @data: The data to write
>   * @size: The size of @data
> + * @result: The resulting message

s/result/msg/ (or vise versa)

>   *
>   * This is similar to nvec_write_async(), but waits for the
>   * request to be answered before returning. This function
>   * uses a mutex and can thus not be called from e.g.
>   * interrupt handlers.
>   *
> - * Returns: A pointer to the response message on success,
> + * Returns in *result: A pointer to the response message on success,
>   * %NULL on failure. Free with nvec_msg_free() once no longer
>   * used.
> + *
> + * Returns: 0 on success, a negative error code on failure. If a failure
> + * occurred, the nvec driver may print an error.
>   */
> -struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
> -		const unsigned char *data, short size)
> +int nvec_write_sync(struct nvec_chip *nvec,
> +		const unsigned char *data, short size,
> +		struct nvec_msg **msg)
>  {
> -	struct nvec_msg *msg;
> +	int err;
> 
>  	mutex_lock(&nvec->sync_write_mutex);
> 
>  	nvec->sync_write_pending = (data[1] << 8) + data[0];
> 
> -	if (nvec_write_async(nvec, data, size) < 0) {
> +	err = nvec_write_async(nvec, data, size);
> +	if (err < 0) {
>  		mutex_unlock(&nvec->sync_write_mutex);
> -		return NULL;
> +		return err;
>  	}
> 
>  	dev_dbg(nvec->dev, "nvec_sync_write: 0x%04x\n",
>  					nvec->sync_write_pending);
> -	if (!(wait_for_completion_timeout(&nvec->sync_write,
> -				msecs_to_jiffies(2000)))) {
> +	err = wait_for_completion_timeout(&nvec->sync_write,
> +			msecs_to_jiffies(2000));
> +	if (!err) {
>  		dev_warn(nvec->dev, "timeout waiting for sync write to complete\n");
>  		mutex_unlock(&nvec->sync_write_mutex);
> -		return NULL;
> +		return err;
>  	}
> 
>  	dev_dbg(nvec->dev, "nvec_sync_write: pong!\n");
> 
> -	msg = nvec->last_sync_msg;
> +	*msg = nvec->last_sync_msg;
> 
>  	mutex_unlock(&nvec->sync_write_mutex);
> 
> -	return msg;
> +	return 0;
>  }
>  EXPORT_SYMBOL(nvec_write_sync);
> 
> @@ -879,9 +886,7 @@ static int tegra_nvec_probe(struct platform_device
> *pdev) pm_power_off = nvec_power_off;
> 
>  	/* Get Firmware Version */
> -	msg = nvec_write_sync(nvec, get_firmware_version, 2);
> -
> -	if (msg) {
> +	if (!nvec_write_sync(nvec, get_firmware_version, 2, &msg)) {
>  		dev_warn(nvec->dev, "ec firmware version %02x.%02x.%02x / %02x\n",
>  			msg->data[4], msg->data[5], msg->data[6], msg->data[7]);
> 
> @@ -935,8 +940,8 @@ static int nvec_suspend(struct device *dev)
>  	/* keep these sync or you'll break suspend */
>  	nvec_toggle_global_events(nvec, false);
> 
> -	msg = nvec_write_sync(nvec, ap_suspend, sizeof(ap_suspend));
> -	nvec_msg_free(nvec, msg);
> +	if (!nvec_write_sync(nvec, ap_suspend, sizeof(ap_suspend), &msg))
> +		nvec_msg_free(nvec, msg);
> 
>  	nvec_disable_i2c_slave(nvec);
> 
> diff --git a/drivers/staging/nvec/nvec.h b/drivers/staging/nvec/nvec.h
> index e271375..00d099a 100644
> --- a/drivers/staging/nvec/nvec.h
> +++ b/drivers/staging/nvec/nvec.h
> @@ -168,8 +168,9 @@ struct nvec_chip {
>  extern int nvec_write_async(struct nvec_chip *nvec, const unsigned char
> *data, short size);
> 
> -extern struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
> -					const unsigned char *data, short size);
> +extern int nvec_write_sync(struct nvec_chip *nvec,
> +			   const unsigned char *data, short size,
> +			   struct nvec_msg **result);

result or msg?

>  extern int nvec_register_notifier(struct nvec_chip *nvec,
>  				  struct notifier_block *nb,

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]