[PATCH resend 0/5] remoteproc: updates for new events

[PATCH resend 0/5] remoteproc: updates for new events

[Alex Elder]

(This is just a rebase of https://lkml.org/lkml/2018/5/15/919)

This series changes the prototype for rproc_add_subdev().  The
caller is now responsible for populating the function pointers
recorded in the rproc_subdev structure, rather than having them be
passed as arguments.  These two existing function pointers have been
renamed ("probe" is now "start" and "remove" is now "stop"), and
they are now optional.  Callback functions may now also be assigned
for two new events (prior to start and after stop).

					-Alex

Alex Elder (1):
  remoteproc: rename subdev probe and remove functions

Bjorn Andersson (4):
  remoteproc: Rename subdev functions to start/stop
  remoteproc: Make start and stop in subdev optional
  remoteproc: Make client initialize ops in rproc_subdev
  remoteproc: Introduce prepare and unprepare for subdevices

 drivers/remoteproc/qcom_common.c     |  26 ++++---
 drivers/remoteproc/qcom_sysmon.c     |   5 +-
 drivers/remoteproc/remoteproc_core.c | 110 ++++++++++++++++++++-------
 include/linux/remoteproc.h           |  19 ++---
 4 files changed, 109 insertions(+), 51 deletions(-)

-- 
2.17.1

[PATCH resend 1/5] remoteproc: Rename subdev functions to start/stop

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

"start" and "stop" are more suitable names for how these two operations
are used, and they fit better with the upcoming introduction of two
additional operations in the struct.

[elder@linaro.org: minor comment edits]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 30 ++++++++++++++--------------
 include/linux/remoteproc.h           | 14 ++++++-------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a9609d971f7f..5dd58e6bea88 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -774,13 +774,13 @@ static int rproc_handle_resources(struct rproc *rproc,
 	return ret;
 }
 
-static int rproc_probe_subdevices(struct rproc *rproc)
+static int rproc_start_subdevices(struct rproc *rproc)
 {
 	struct rproc_subdev *subdev;
 	int ret;
 
 	list_for_each_entry(subdev, &rproc->subdevs, node) {
-		ret = subdev->probe(subdev);
+		ret = subdev->start(subdev);
 		if (ret)
 			goto unroll_registration;
 	}
@@ -789,17 +789,17 @@ static int rproc_probe_subdevices(struct rproc *rproc)
 
 unroll_registration:
 	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
-		subdev->remove(subdev, true);
+		subdev->stop(subdev, true);
 
 	return ret;
 }
 
-static void rproc_remove_subdevices(struct rproc *rproc, bool crashed)
+static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 {
 	struct rproc_subdev *subdev;
 
 	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
-		subdev->remove(subdev, crashed);
+		subdev->stop(subdev, crashed);
 }
 
 /**
@@ -901,8 +901,8 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 		return ret;
 	}
 
-	/* probe any subdevices for the remote processor */
-	ret = rproc_probe_subdevices(rproc);
+	/* Start any subdevices for the remote processor */
+	ret = rproc_start_subdevices(rproc);
 	if (ret) {
 		dev_err(dev, "failed to probe subdevices for %s: %d\n",
 			rproc->name, ret);
@@ -1014,8 +1014,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
 	struct device *dev = &rproc->dev;
 	int ret;
 
-	/* remove any subdevices for the remote processor */
-	rproc_remove_subdevices(rproc, crashed);
+	/* Stop any subdevices for the remote processor */
+	rproc_stop_subdevices(rproc, crashed);
 
 	/* the installed resource table is no longer accessible */
 	rproc->table_ptr = rproc->cached_table;
@@ -1657,16 +1657,16 @@ EXPORT_SYMBOL(rproc_del);
  * rproc_add_subdev() - add a subdevice to a remoteproc
  * @rproc: rproc handle to add the subdevice to
  * @subdev: subdev handle to register
- * @probe: function to call when the rproc boots
- * @remove: function to call when the rproc shuts down
+ * @start: function to call after the rproc is started
+ * @stop: function to call before the rproc is stopped
  */
 void rproc_add_subdev(struct rproc *rproc,
 		      struct rproc_subdev *subdev,
-		      int (*probe)(struct rproc_subdev *subdev),
-		      void (*remove)(struct rproc_subdev *subdev, bool crashed))
+		      int (*start)(struct rproc_subdev *subdev),
+		      void (*stop)(struct rproc_subdev *subdev, bool crashed))
 {
-	subdev->probe = probe;
-	subdev->remove = remove;
+	subdev->start = start;
+	subdev->stop = stop;
 
 	list_add_tail(&subdev->node, &rproc->subdevs);
 }
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index dfdaede9139e..bf55bf2a5ee1 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -477,15 +477,15 @@ struct rproc {
 /**
  * struct rproc_subdev - subdevice tied to a remoteproc
  * @node: list node related to the rproc subdevs list
- * @probe: probe function, called as the rproc is started
- * @remove: remove function, called as the rproc is being stopped, the @crashed
- *	    parameter indicates if this originates from the a recovery
+ * @start: start function, called after the rproc has been started
+ * @stop: stop function, called before the rproc is stopped; the @crashed
+ *	    parameter indicates if this originates from a recovery
  */
 struct rproc_subdev {
 	struct list_head node;
 
-	int (*probe)(struct rproc_subdev *subdev);
-	void (*remove)(struct rproc_subdev *subdev, bool crashed);
+	int (*start)(struct rproc_subdev *subdev);
+	void (*stop)(struct rproc_subdev *subdev, bool crashed);
 };
 
 /* we currently support only two vrings per rvdev */
@@ -568,8 +568,8 @@ 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 crashed));
+		      int (*start)(struct rproc_subdev *subdev),
+		      void (*stop)(struct rproc_subdev *subdev, bool crashed));
 
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
-- 
2.17.1

Re: [PATCH resend 1/5] remoteproc: Rename subdev functions to start/stop

[Bjorn Andersson]

On Mon 25 Jun 18:34 PDT 2018, Alex Elder wrote:

> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> "start" and "stop" are more suitable names for how these two operations
> are used, and they fit better with the upcoming introduction of two
> additional operations in the struct.
> 
> [elder@linaro.org: minor comment edits]
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Alex Elder <elder@linaro.org>
> Tested-by: Fabien Dessenne <fabien.dessenne@st.com>

Sorry for not spotting this before, but per section 11 of
Documentation/process/submitting-patches.rst the tag part should read:


Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[elder@linaro.org: minor comment edits]
Signed-off-by: Alex Elder <elder@linaro.org>


I.e. as you're sending the email you must be the last one certifying the
origin of the patch.

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_core.c | 30 ++++++++++++++--------------
>  include/linux/remoteproc.h           | 14 ++++++-------
>  2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index a9609d971f7f..5dd58e6bea88 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -774,13 +774,13 @@ static int rproc_handle_resources(struct rproc *rproc,
>  	return ret;
>  }
>  
> -static int rproc_probe_subdevices(struct rproc *rproc)
> +static int rproc_start_subdevices(struct rproc *rproc)
>  {
>  	struct rproc_subdev *subdev;
>  	int ret;
>  
>  	list_for_each_entry(subdev, &rproc->subdevs, node) {
> -		ret = subdev->probe(subdev);
> +		ret = subdev->start(subdev);
>  		if (ret)
>  			goto unroll_registration;
>  	}
> @@ -789,17 +789,17 @@ static int rproc_probe_subdevices(struct rproc *rproc)
>  
>  unroll_registration:
>  	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
> -		subdev->remove(subdev, true);
> +		subdev->stop(subdev, true);
>  
>  	return ret;
>  }
>  
> -static void rproc_remove_subdevices(struct rproc *rproc, bool crashed)
> +static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>  {
>  	struct rproc_subdev *subdev;
>  
>  	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
> -		subdev->remove(subdev, crashed);
> +		subdev->stop(subdev, crashed);
>  }
>  
>  /**
> @@ -901,8 +901,8 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		return ret;
>  	}
>  
> -	/* probe any subdevices for the remote processor */
> -	ret = rproc_probe_subdevices(rproc);
> +	/* Start any subdevices for the remote processor */
> +	ret = rproc_start_subdevices(rproc);
>  	if (ret) {
>  		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>  			rproc->name, ret);
> @@ -1014,8 +1014,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>  	struct device *dev = &rproc->dev;
>  	int ret;
>  
> -	/* remove any subdevices for the remote processor */
> -	rproc_remove_subdevices(rproc, crashed);
> +	/* Stop any subdevices for the remote processor */
> +	rproc_stop_subdevices(rproc, crashed);
>  
>  	/* the installed resource table is no longer accessible */
>  	rproc->table_ptr = rproc->cached_table;
> @@ -1657,16 +1657,16 @@ EXPORT_SYMBOL(rproc_del);
>   * rproc_add_subdev() - add a subdevice to a remoteproc
>   * @rproc: rproc handle to add the subdevice to
>   * @subdev: subdev handle to register
> - * @probe: function to call when the rproc boots
> - * @remove: function to call when the rproc shuts down
> + * @start: function to call after the rproc is started
> + * @stop: function to call before the rproc is stopped
>   */
>  void rproc_add_subdev(struct rproc *rproc,
>  		      struct rproc_subdev *subdev,
> -		      int (*probe)(struct rproc_subdev *subdev),
> -		      void (*remove)(struct rproc_subdev *subdev, bool crashed))
> +		      int (*start)(struct rproc_subdev *subdev),
> +		      void (*stop)(struct rproc_subdev *subdev, bool crashed))
>  {
> -	subdev->probe = probe;
> -	subdev->remove = remove;
> +	subdev->start = start;
> +	subdev->stop = stop;
>  
>  	list_add_tail(&subdev->node, &rproc->subdevs);
>  }
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index dfdaede9139e..bf55bf2a5ee1 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -477,15 +477,15 @@ struct rproc {
>  /**
>   * struct rproc_subdev - subdevice tied to a remoteproc
>   * @node: list node related to the rproc subdevs list
> - * @probe: probe function, called as the rproc is started
> - * @remove: remove function, called as the rproc is being stopped, the @crashed
> - *	    parameter indicates if this originates from the a recovery
> + * @start: start function, called after the rproc has been started
> + * @stop: stop function, called before the rproc is stopped; the @crashed
> + *	    parameter indicates if this originates from a recovery
>   */
>  struct rproc_subdev {
>  	struct list_head node;
>  
> -	int (*probe)(struct rproc_subdev *subdev);
> -	void (*remove)(struct rproc_subdev *subdev, bool crashed);
> +	int (*start)(struct rproc_subdev *subdev);
> +	void (*stop)(struct rproc_subdev *subdev, bool crashed);
>  };
>  
>  /* we currently support only two vrings per rvdev */
> @@ -568,8 +568,8 @@ 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 crashed));
> +		      int (*start)(struct rproc_subdev *subdev),
> +		      void (*stop)(struct rproc_subdev *subdev, bool crashed));
>  
>  void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
>  
> -- 
> 2.17.1
> 

Re: [PATCH resend 1/5] remoteproc: Rename subdev functions to start/stop

[Alex Elder]

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

Sorry about that. I will take care if it in the morning.
Feel free to edit yourself, I certify it either way.

     -Alex


On Mon, Jun 25, 2018, 9:20 PM Bjorn Andersson <bjorn.andersson@linaro.org>
wrote:

> On Mon 25 Jun 18:34 PDT 2018, Alex Elder wrote:
>
> > From: Bjorn Andersson <bjorn.andersson@linaro.org>
> >
> > "start" and "stop" are more suitable names for how these two operations
> > are used, and they fit better with the upcoming introduction of two
> > additional operations in the struct.
> >
> > [elder@linaro.org: minor comment edits]
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Acked-by: Alex Elder <elder@linaro.org>
> > Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
>
> Sorry for not spotting this before, but per section 11 of
> Documentation/process/submitting-patches.rst the tag part should read:
>
>
> Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> [elder@linaro.org: minor comment edits]
> Signed-off-by: Alex Elder <elder@linaro.org>
>
>
> I.e. as you're sending the email you must be the last one certifying the
> origin of the patch.
>
> Regards,
> Bjorn
>
> > ---
> >  drivers/remoteproc/remoteproc_core.c | 30 ++++++++++++++--------------
> >  include/linux/remoteproc.h           | 14 ++++++-------
> >  2 files changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index a9609d971f7f..5dd58e6bea88 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -774,13 +774,13 @@ static int rproc_handle_resources(struct rproc
> *rproc,
> >       return ret;
> >  }
> >
> > -static int rproc_probe_subdevices(struct rproc *rproc)
> > +static int rproc_start_subdevices(struct rproc *rproc)
> >  {
> >       struct rproc_subdev *subdev;
> >       int ret;
> >
> >       list_for_each_entry(subdev, &rproc->subdevs, node) {
> > -             ret = subdev->probe(subdev);
> > +             ret = subdev->start(subdev);
> >               if (ret)
> >                       goto unroll_registration;
> >       }
> > @@ -789,17 +789,17 @@ static int rproc_probe_subdevices(struct rproc
> *rproc)
> >
> >  unroll_registration:
> >       list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
> > -             subdev->remove(subdev, true);
> > +             subdev->stop(subdev, true);
> >
> >       return ret;
> >  }
> >
> > -static void rproc_remove_subdevices(struct rproc *rproc, bool crashed)
> > +static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
> >  {
> >       struct rproc_subdev *subdev;
> >
> >       list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
> > -             subdev->remove(subdev, crashed);
> > +             subdev->stop(subdev, crashed);
> >  }
> >
> >  /**
> > @@ -901,8 +901,8 @@ static int rproc_start(struct rproc *rproc, const
> struct firmware *fw)
> >               return ret;
> >       }
> >
> > -     /* probe any subdevices for the remote processor */
> > -     ret = rproc_probe_subdevices(rproc);
> > +     /* Start any subdevices for the remote processor */
> > +     ret = rproc_start_subdevices(rproc);
> >       if (ret) {
> >               dev_err(dev, "failed to probe subdevices for %s: %d\n",
> >                       rproc->name, ret);
> > @@ -1014,8 +1014,8 @@ static int rproc_stop(struct rproc *rproc, bool
> crashed)
> >       struct device *dev = &rproc->dev;
> >       int ret;
> >
> > -     /* remove any subdevices for the remote processor */
> > -     rproc_remove_subdevices(rproc, crashed);
> > +     /* Stop any subdevices for the remote processor */
> > +     rproc_stop_subdevices(rproc, crashed);
> >
> >       /* the installed resource table is no longer accessible */
> >       rproc->table_ptr = rproc->cached_table;
> > @@ -1657,16 +1657,16 @@ EXPORT_SYMBOL(rproc_del);
> >   * rproc_add_subdev() - add a subdevice to a remoteproc
> >   * @rproc: rproc handle to add the subdevice to
> >   * @subdev: subdev handle to register
> > - * @probe: function to call when the rproc boots
> > - * @remove: function to call when the rproc shuts down
> > + * @start: function to call after the rproc is started
> > + * @stop: function to call before the rproc is stopped
> >   */
> >  void rproc_add_subdev(struct rproc *rproc,
> >                     struct rproc_subdev *subdev,
> > -                   int (*probe)(struct rproc_subdev *subdev),
> > -                   void (*remove)(struct rproc_subdev *subdev, bool
> crashed))
> > +                   int (*start)(struct rproc_subdev *subdev),
> > +                   void (*stop)(struct rproc_subdev *subdev, bool
> crashed))
> >  {
> > -     subdev->probe = probe;
> > -     subdev->remove = remove;
> > +     subdev->start = start;
> > +     subdev->stop = stop;
> >
> >       list_add_tail(&subdev->node, &rproc->subdevs);
> >  }
> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> > index dfdaede9139e..bf55bf2a5ee1 100644
> > --- a/include/linux/remoteproc.h
> > +++ b/include/linux/remoteproc.h
> > @@ -477,15 +477,15 @@ struct rproc {
> >  /**
> >   * struct rproc_subdev - subdevice tied to a remoteproc
> >   * @node: list node related to the rproc subdevs list
> > - * @probe: probe function, called as the rproc is started
> > - * @remove: remove function, called as the rproc is being stopped, the
> @crashed
> > - *       parameter indicates if this originates from the a recovery
> > + * @start: start function, called after the rproc has been started
> > + * @stop: stop function, called before the rproc is stopped; the
> @crashed
> > + *       parameter indicates if this originates from a recovery
> >   */
> >  struct rproc_subdev {
> >       struct list_head node;
> >
> > -     int (*probe)(struct rproc_subdev *subdev);
> > -     void (*remove)(struct rproc_subdev *subdev, bool crashed);
> > +     int (*start)(struct rproc_subdev *subdev);
> > +     void (*stop)(struct rproc_subdev *subdev, bool crashed);
> >  };
> >
> >  /* we currently support only two vrings per rvdev */
> > @@ -568,8 +568,8 @@ 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
> crashed));
> > +                   int (*start)(struct rproc_subdev *subdev),
> > +                   void (*stop)(struct rproc_subdev *subdev, bool
> crashed));
> >
> >  void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev
> *subdev);
> >
> > --
> > 2.17.1
> >
>

[-- Attachment #2: Type: text/html, Size: 9243 bytes --]

[PATCH resend 2/5] remoteproc: Make start and stop in subdev optional

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Some subdevices, such as glink ssr only care about the stop operation,
so make the operations optional to reduce client code.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5dd58e6bea88..981ae6dff145 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -780,16 +780,20 @@ static int rproc_start_subdevices(struct rproc *rproc)
 	int ret;
 
 	list_for_each_entry(subdev, &rproc->subdevs, node) {
-		ret = subdev->start(subdev);
-		if (ret)
-			goto unroll_registration;
+		if (subdev->start) {
+			ret = subdev->start(subdev);
+			if (ret)
+				goto unroll_registration;
+		}
 	}
 
 	return 0;
 
 unroll_registration:
-	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
-		subdev->stop(subdev, true);
+	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->stop)
+			subdev->stop(subdev, true);
+	}
 
 	return ret;
 }
@@ -798,8 +802,10 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 {
 	struct rproc_subdev *subdev;
 
-	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
-		subdev->stop(subdev, crashed);
+	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->stop)
+			subdev->stop(subdev, crashed);
+	}
 }
 
 /**
-- 
2.17.1

[PATCH resend 3/5] remoteproc: Make client initialize ops in rproc_subdev

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

In preparation of adding the additional prepare and unprepare operations
make the client responsible for filling out the function pointers of the
rproc_subdev. This makes the arguments to rproc_add_subdev() more
manageable, in particular when some of the functions are left out.

[elder@linaro.org: added comment about assigning function pointers]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/qcom_common.c     | 18 ++++++++++--------
 drivers/remoteproc/qcom_sysmon.c     |  5 ++++-
 drivers/remoteproc/remoteproc_core.c | 18 +++++++-----------
 include/linux/remoteproc.h           |  5 +----
 4 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index acfc99f82fb8..4ae87c5b8793 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -64,7 +64,10 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
 		return;
 
 	glink->dev = dev;
-	rproc_add_subdev(rproc, &glink->subdev, glink_subdev_probe, glink_subdev_remove);
+	glink->subdev.start = glink_subdev_probe;
+	glink->subdev.stop = glink_subdev_remove;
+
+	rproc_add_subdev(rproc, &glink->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_glink_subdev);
 
@@ -157,7 +160,10 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
 		return;
 
 	smd->dev = dev;
-	rproc_add_subdev(rproc, &smd->subdev, smd_subdev_probe, smd_subdev_remove);
+	smd->subdev.start = smd_subdev_probe;
+	smd->subdev.stop = smd_subdev_remove;
+
+	rproc_add_subdev(rproc, &smd->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_smd_subdev);
 
@@ -202,11 +208,6 @@ void qcom_unregister_ssr_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);
 
-static int ssr_notify_start(struct rproc_subdev *subdev)
-{
-	return  0;
-}
-
 static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
@@ -227,8 +228,9 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 			 const char *ssr_name)
 {
 	ssr->name = ssr_name;
+	ssr->subdev.stop = ssr_notify_stop;
 
-	rproc_add_subdev(rproc, &ssr->subdev, ssr_notify_start, ssr_notify_stop);
+	rproc_add_subdev(rproc, &ssr->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_ssr_subdev);
 
diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index f085545d7da5..e976a602b015 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -469,7 +469,10 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 
 	qmi_add_lookup(&sysmon->qmi, 43, 0, 0);
 
-	rproc_add_subdev(rproc, &sysmon->subdev, sysmon_start, sysmon_stop);
+	sysmon->subdev.start = sysmon_start;
+	sysmon->subdev.stop = sysmon_stop;
+
+	rproc_add_subdev(rproc, &sysmon->subdev);
 
 	sysmon->nb.notifier_call = sysmon_notify;
 	blocking_notifier_chain_register(&sysmon_notifiers, &sysmon->nb);
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 981ae6dff145..ca39fad175f2 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -399,8 +399,10 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 
 	list_add_tail(&rvdev->node, &rproc->rvdevs);
 
-	rproc_add_subdev(rproc, &rvdev->subdev,
-			 rproc_vdev_do_probe, rproc_vdev_do_remove);
+	rvdev->subdev.start = rproc_vdev_do_probe;
+	rvdev->subdev.stop = rproc_vdev_do_remove;
+
+	rproc_add_subdev(rproc, &rvdev->subdev);
 
 	return 0;
 
@@ -1663,17 +1665,11 @@ EXPORT_SYMBOL(rproc_del);
  * rproc_add_subdev() - add a subdevice to a remoteproc
  * @rproc: rproc handle to add the subdevice to
  * @subdev: subdev handle to register
- * @start: function to call after the rproc is started
- * @stop: function to call before the rproc is stopped
+ *
+ * Caller is responsible for populating optional subdevice function pointers.
  */
-void rproc_add_subdev(struct rproc *rproc,
-		      struct rproc_subdev *subdev,
-		      int (*start)(struct rproc_subdev *subdev),
-		      void (*stop)(struct rproc_subdev *subdev, bool crashed))
+void rproc_add_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
 {
-	subdev->start = start;
-	subdev->stop = stop;
-
 	list_add_tail(&subdev->node, &rproc->subdevs);
 }
 EXPORT_SYMBOL(rproc_add_subdev);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index bf55bf2a5ee1..8f1426330cca 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -566,10 +566,7 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
 	return rvdev->rproc;
 }
 
-void rproc_add_subdev(struct rproc *rproc,
-		      struct rproc_subdev *subdev,
-		      int (*start)(struct rproc_subdev *subdev),
-		      void (*stop)(struct rproc_subdev *subdev, bool crashed));
+void rproc_add_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
-- 
2.17.1

[PATCH resend 4/5] remoteproc: rename subdev probe and remove functions

[Alex Elder]

Rename functions used when subdevices are started and stopped to
reflect the new naming scheme.

Signed-off-by: Alex Elder <elder@linaro.org>
Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
 drivers/remoteproc/remoteproc_core.c |  8 ++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4ae87c5b8793..6f77840140bf 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -33,7 +33,7 @@
 
 static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
 
-static int glink_subdev_probe(struct rproc_subdev *subdev)
+static int glink_subdev_start(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
@@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
 	return PTR_ERR_OR_ZERO(glink->edge);
 }
 
-static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
+static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
@@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
 		return;
 
 	glink->dev = dev;
-	glink->subdev.start = glink_subdev_probe;
-	glink->subdev.stop = glink_subdev_remove;
+	glink->subdev.start = glink_subdev_start;
+	glink->subdev.stop = glink_subdev_stop;
 
 	rproc_add_subdev(rproc, &glink->subdev);
 }
@@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
 }
 EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
 
-static int smd_subdev_probe(struct rproc_subdev *subdev)
+static int smd_subdev_start(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
 
@@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
 	return PTR_ERR_OR_ZERO(smd->edge);
 }
 
-static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
+static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
 
@@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
 		return;
 
 	smd->dev = dev;
-	smd->subdev.start = smd_subdev_probe;
-	smd->subdev.stop = smd_subdev_remove;
+	smd->subdev.start = smd_subdev_start;
+	smd->subdev.stop = smd_subdev_stop;
 
 	rproc_add_subdev(rproc, &smd->subdev);
 }
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index ca39fad175f2..2ede7ae6f5bc 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
 	rsc->vring[idx].notifyid = -1;
 }
 
-static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
+static int rproc_vdev_do_start(struct rproc_subdev *subdev)
 {
 	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
 
 	return rproc_add_virtio_dev(rvdev, rvdev->id);
 }
 
-static void rproc_vdev_do_remove(struct rproc_subdev *subdev, bool crashed)
+static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
 
@@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 
 	list_add_tail(&rvdev->node, &rproc->rvdevs);
 
-	rvdev->subdev.start = rproc_vdev_do_probe;
-	rvdev->subdev.stop = rproc_vdev_do_remove;
+	rvdev->subdev.start = rproc_vdev_do_start;
+	rvdev->subdev.stop = rproc_vdev_do_stop;
 
 	rproc_add_subdev(rproc, &rvdev->subdev);
 
-- 
2.17.1

[PATCH resend 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

On rare occasions a subdevice might need to prepare some hardware
resources before a remote processor is booted, and clean up some
state after it has been shut down.

One such example is the IP Accelerator found in various Qualcomm
platforms, which is accessed directly from both the modem remoteproc
and the application subsystem and requires an intricate lockstep
process when bringing the modem up and down.

[elder@linaro.org: minor description and comment edits]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
 include/linux/remoteproc.h           |  4 ++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 2ede7ae6f5bc..283b258f5e0f 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
 	return ret;
 }
 
+static int rproc_prepare_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+	int ret;
+
+	list_for_each_entry(subdev, &rproc->subdevs, node) {
+		if (subdev->prepare) {
+			ret = subdev->prepare(subdev);
+			if (ret)
+				goto unroll_preparation;
+		}
+	}
+
+	return 0;
+
+unroll_preparation:
+	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->unprepare)
+			subdev->unprepare(subdev);
+	}
+
+	return ret;
+}
+
 static int rproc_start_subdevices(struct rproc *rproc)
 {
 	struct rproc_subdev *subdev;
@@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 	}
 }
 
+static void rproc_unprepare_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+
+	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->unprepare)
+			subdev->unprepare(subdev);
+	}
+}
+
 /**
  * rproc_coredump_cleanup() - clean up dump_segments list
  * @rproc: the remote processor handle
@@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 		rproc->table_ptr = loaded_table;
 	}
 
+	ret = rproc_prepare_subdevices(rproc);
+	if (ret) {
+		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
+			rproc->name, ret);
+		return ret;
+	}
+
 	/* power up the remote processor */
 	ret = rproc->ops->start(rproc);
 	if (ret) {
 		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
-		return ret;
+		goto unprepare_subdevices;
 	}
 
 	/* Start any subdevices for the remote processor */
@@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 	if (ret) {
 		dev_err(dev, "failed to probe subdevices for %s: %d\n",
 			rproc->name, ret);
-		rproc->ops->stop(rproc);
-		return ret;
+		goto stop_rproc;
 	}
 
 	rproc->state = RPROC_RUNNING;
@@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 	dev_info(dev, "remote processor %s is now up\n", rproc->name);
 
 	return 0;
+
+stop_rproc:
+	rproc->ops->stop(rproc);
+
+unprepare_subdevices:
+	rproc_unprepare_subdevices(rproc);
+
+	return ret;
 }
 
 /*
@@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
 		return ret;
 	}
 
+	rproc_unprepare_subdevices(rproc);
+
 	rproc->state = RPROC_OFFLINE;
 
 	dev_info(dev, "stopped remote processor %s\n", rproc->name);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8f1426330cca..e3c5d856b6da 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -477,15 +477,19 @@ struct rproc {
 /**
  * struct rproc_subdev - subdevice tied to a remoteproc
  * @node: list node related to the rproc subdevs list
+ * @prepare: prepare function, called before the rproc is started
  * @start: start function, called after the rproc has been started
  * @stop: stop function, called before the rproc is stopped; the @crashed
  *	    parameter indicates if this originates from a recovery
+ * @unprepare: unprepare function, called after the rproc has been stopped
  */
 struct rproc_subdev {
 	struct list_head node;
 
+	int (*prepare)(struct rproc_subdev *subdev);
 	int (*start)(struct rproc_subdev *subdev);
 	void (*stop)(struct rproc_subdev *subdev, bool crashed);
+	void (*unprepare)(struct rproc_subdev *subdev);
 };
 
 /* we currently support only two vrings per rvdev */
-- 
2.17.1