Linux-remoteproc Archive on lore.kernel.org
 help / color / Atom feed
From: Arnaud POULIQUEN <arnaud.pouliquen@st.com>
To: Mathieu Poirier <mathieu.poirier@linaro.org>,
	"bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
	"ohad@wizery.com" <ohad@wizery.com>
Cc: "linux-remoteproc@vger.kernel.org"
	<linux-remoteproc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 00/13] remoteproc: Add support for detaching from rproc
Date: Tue, 1 Sep 2020 18:55:14 +0200
Message-ID: <cc10db90-92ca-41e1-110d-885ef8266191@st.com> (raw)
In-Reply-To: <20200826164529.224476-1-mathieu.poirier@linaro.org>

Hi Mathieu,

On 8/26/20 6:45 PM, Mathieu Poirier wrote:
> Following the work done here [1] this set provides support for the
> remoteproc core to release resources associated with a remote processor
> without having to switch it off. That way a platform driver can be removed
> or the applcation processor power cycled while the remote processor is
> still operating.
> 
> I have not tested the solution because of the work involved in getting
> a new firmware image to support the feature.  I will do so once it is
> determined that this is the right approach to follow.

I just started watching your series. I also think that we have first to
determine the approach that match meets the requirements of all companies.

Here is my feeling, waiting more feedback from community:

If I understand your approach correctly you propose that the application
determines the firmware live-cycle. Depending on request, the remoteproc core
performs a "shutdown" ( stop + unprepare) or a "detach".
The platform driver can(or have to?) implement the stop and/or the detach.
By default a preloaded firmware is detached and a "standard" firmware is stopped
on kernel shutdown (rproc_del).

As we have seen with the rproc cdev, it might not be simple to manage this
in case of crash.
For instance you can have a Firmware started by the boot-stages but
which must be gracefully stopped in case of crash.  

Another approach would be to let the platform driver decides what should
be done on the stop and prepare ops depending on the HW context.
So the platform driver would be in charge of detaching the firmware.
In this case the issue is to determine the state after stop. the information
would be in platform driver.

I would be more in flavor of the second one, because application would not
have to be aware of the co-processor firmware life-cycle, and the firmware
could expose its own constraints for shutdown.  

A third approach (or complementary approach): 
I don't know why i didn't think of it before... The attach/detach
feature is quite similar to the regulator management.

For regulator 2 DT properties exist[1]:
- regulator-always-on: boolean, regulator should never be disabled
- regulator-boot-on: bootloader/firmware enabled regulator

It is a static configuration but could be implemented for both the attach and
the detach in the core part.
Else if a more dynamic management could be managed by the platform driver
(depending on the loaded firmware).
  
[1]https://elixir.bootlin.com/linux/v4.0/source/Documentation/devicetree/bindings/regulator/regulator.txt

Thanks,
Arnaud

> 
> Applies cleanly on rproc-next (62b8f9e99329)
> 
> Thanks,
> Mathieu 
> 
> [1]. https://lkml.org/lkml/2020/7/14/1600
> 
> Mathieu Poirier (13):
>   remoteproc: Re-check state in rproc_shutdown()
>   remoteproc: Remove useless check in rproc_del()
>   remoteproc: Add new RPROC_ATTACHED state
>   remoteproc: Properly represent the attached state
>   remoteproc: Add new detach() remoteproc operation
>   remoteproc: Introduce function __rproc_detach()
>   remoteproc: Introduce function rproc_detach()
>   remoteproc: Rename function rproc_actuate()
>   remoteproc: Add return value to function rproc_shutdown()
>   remoteproc: Properly deal with a stop request when attached
>   remoteproc: Properly deal with detach request
>   remoteproc: Refactor rproc delete and cdev release path
>   remoteproc: Properly deal with a kernel panic when attached
> 
>  drivers/remoteproc/remoteproc_cdev.c  |  18 ++-
>  drivers/remoteproc/remoteproc_core.c  | 151 +++++++++++++++++++++-----
>  drivers/remoteproc/remoteproc_sysfs.c |  17 ++-
>  include/linux/remoteproc.h            |  14 ++-
>  4 files changed, 157 insertions(+), 43 deletions(-)
> 

  parent reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-26 16:45 Mathieu Poirier
2020-08-26 16:45 ` [PATCH 01/13] remoteproc: Re-check state in rproc_shutdown() Mathieu Poirier
2020-10-15  1:28   ` Peng Fan
2020-08-26 16:45 ` [PATCH 02/13] remoteproc: Remove useless check in rproc_del() Mathieu Poirier
2020-10-15  1:29   ` Peng Fan
2020-08-26 16:45 ` [PATCH 03/13] remoteproc: Add new RPROC_ATTACHED state Mathieu Poirier
2020-10-15  1:31   ` Peng Fan
2020-08-26 16:45 ` [PATCH 04/13] remoteproc: Properly represent the attached state Mathieu Poirier
2020-10-15  1:33   ` Peng Fan
2020-08-26 16:45 ` [PATCH 05/13] remoteproc: Add new detach() remoteproc operation Mathieu Poirier
2020-10-15  1:37   ` Peng Fan
2020-10-22 21:51     ` Mathieu Poirier
2020-08-26 16:45 ` [PATCH 06/13] remoteproc: Introduce function __rproc_detach() Mathieu Poirier
2020-10-15  1:39   ` Peng Fan
2020-08-26 16:45 ` [PATCH 07/13] remoteproc: Introduce function rproc_detach() Mathieu Poirier
2020-10-15  1:52   ` Peng Fan
2020-10-22 21:55     ` Mathieu Poirier
2020-08-26 16:45 ` [PATCH 08/13] remoteproc: Rename function rproc_actuate() Mathieu Poirier
2020-10-15  2:15   ` Peng Fan
2020-08-26 16:45 ` [PATCH 09/13] remoteproc: Add return value to function rproc_shutdown() Mathieu Poirier
2020-10-15  2:21   ` Peng Fan
2020-08-26 16:45 ` [PATCH 10/13] remoteproc: Properly deal with a stop request when attached Mathieu Poirier
2020-10-15  2:29   ` Peng Fan
2020-08-26 16:45 ` [PATCH 11/13] remoteproc: Properly deal with detach request Mathieu Poirier
2020-10-15  2:30   ` Peng Fan
2020-08-26 16:45 ` [PATCH 12/13] remoteproc: Refactor rproc delete and cdev release path Mathieu Poirier
2020-10-15  2:31   ` Peng Fan
2020-08-26 16:45 ` [PATCH 13/13] remoteproc: Properly deal with a kernel panic when attached Mathieu Poirier
2020-10-15  2:28   ` Peng Fan
2020-09-01 16:55 ` Arnaud POULIQUEN [this message]
2020-09-01 18:02   ` [PATCH 00/13] remoteproc: Add support for detaching from rproc Mathieu Poirier

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=cc10db90-92ca-41e1-110d-885ef8266191@st.com \
    --to=arnaud.pouliquen@st.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=mathieu.poirier@linaro.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

Linux-remoteproc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-remoteproc/0 linux-remoteproc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-remoteproc linux-remoteproc/ https://lore.kernel.org/linux-remoteproc \
		linux-remoteproc@vger.kernel.org
	public-inbox-index linux-remoteproc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-remoteproc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git