mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alex Elder <elder@linaro.org>
To: ohad@wizery.com, bjorn.andersson@linaro.org
Cc: linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] remoteproc: rename "crashed" parameter
Date: Wed, 25 Apr 2018 08:36:26 -0500	[thread overview]
Message-ID: <20180425133626.11781-1-elder@linaro.org> (raw)

The last commit to "remoteproc_core.c":
  880f5b388252 remoteproc: Pass type of shutdown to subdev remove
added a Boolean flag to the subdevice remove method, to distinguish
between graceful shutdown and a crash.  Unfortunately, the names of
the parameters were inconsistent, and in fact have opposite meanings.

In most cases, the parameter is named "crashed", but rproc_add_subdev()
names the parameter "graceful" in the prototype for the remove method.

The remove method is ultimately called (and supplied with the Boolean
flag value) by rproc_remove_subdevices().  That is only called by
rproc_stop(), and in the two spots where that is used, "graceful" is
the right name for the flag:
  rproc_shutdown() passes true, indicating a graceful shutdown
  rproc_trigger_recovery() passes false, indicating a crash

The fix is to make the parameter name consistent, and making the
name and sense of the parameter to always be "crashed" produces the
smallest change.  So that's what this patch does.

To verify this change, rproc_add_subdev() is called in five spots:
  - qcom_add_glink_subdev() passes glink_subdev_remove()
  - qcom_add_smd_subdev() passes smd_subdev_remove()
  - qcom_add_ssr_subdev() passes ssr_notify_stop()
  - qcom_add_sysmon_subdev() passes sysmon_stop()
  - rproc_handle_vdev() passes rproc_vdev_do_remove()

Of these, only sysmon_stop() uses its "crashed" parameter.  And it
clearly assumes that "crashed" is the intended meaning:

        /* Don't request graceful shutdown if we've crashed */
        if (crashed)
                return;

So this function (added after the "crashed" parameter was added)
exhibited buggy behavior, which is now fixed by this patch.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 4 ++--
 include/linux/remoteproc.h           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 6d9c5832ce47..a9609d971f7f 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1163,7 +1163,7 @@ int rproc_trigger_recovery(struct rproc *rproc)
 	if (ret)
 		return ret;
 
-	ret = rproc_stop(rproc, false);
+	ret = rproc_stop(rproc, true);
 	if (ret)
 		goto unlock_mutex;
 
@@ -1316,7 +1316,7 @@ void rproc_shutdown(struct rproc *rproc)
 	if (!atomic_dec_and_test(&rproc->power))
 		goto out;
 
-	ret = rproc_stop(rproc, true);
+	ret = rproc_stop(rproc, false);
 	if (ret) {
 		atomic_inc(&rproc->power);
 		goto out;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index d09a9c7af109..dfdaede9139e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -569,7 +569,7 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
 void rproc_add_subdev(struct rproc *rproc,
 		      struct rproc_subdev *subdev,
 		      int (*probe)(struct rproc_subdev *subdev),
-		      void (*remove)(struct rproc_subdev *subdev, bool graceful));
+		      void (*remove)(struct rproc_subdev *subdev, bool crashed));
 
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
-- 
2.14.1

             reply	other threads:[~2018-04-25 13:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 13:36 Alex Elder [this message]
2018-04-25 13:49 ` Arnaud Pouliquen
2018-04-25 14:47   ` Alex Elder

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=20180425133626.11781-1-elder@linaro.org \
    --to=elder@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=ohad@wizery.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®