Message-ID: <ZzTeKTieC8YHC1Pv@p14s>
Date: Wed, 13 Nov 2024 10:13:13 -0700
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: anish kumar <yesanishhere@...il.com>
Cc: andersson@...nel.org, corbet@....net, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-remoteproc@...r.kernel.org
Subject: Re: [RESEND PATCH V6 2/3] Documentation: remoteproc: add overview
 section

On Tue, Nov 05, 2024 at 09:10:15PM -0800, anish kumar wrote:
> Added overview section which details
> how the remote processor framework works and
> how it handles crashes.
> 
> Signed-off-by: anish kumar <yesanishhere@...il.com>
> ---
>  Documentation/staging/remoteproc.rst | 43 ++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
> index eeebbeca71de..e0bf68ceade8 100644
> --- a/Documentation/staging/remoteproc.rst
> +++ b/Documentation/staging/remoteproc.rst
> @@ -46,6 +46,49 @@ of several key components:
>  - **Virtio Support**: Facilitates interaction with the virtio and
>    rpmsg bus.
>  
> +Overview
> +========
> +
> +The framework begins by gathering information about the firmware file
> +to be downloaded through the request_firmware function. It supports
> +the ELF format and parses the firmware image to identify the physical
> +addresses that need to be populated from the corresponding ELF sections.
> +Once this information is obtained from the driver, the framework transfers
> +the data to the specified addresses and starts the remote processor,
> +along with subdevices.

Here again, some information is inaccurate and some is incomplete.

> +
> +Dependent devices, referred to as `subdevices` within the framework,
> +are also managed post-registration by their respective drivers.
> +Subdevices can register themselves using `rproc_(add/remove)_subdev`.
> +Non-remoteproc drivers can use subdevices as a way to logically connect
> +to remote and get lifecycle notifications of the remote.

All of the above is pretty much inaccurate.

I do not see a way forward for this revision and as such will stop here.

As I suggested before, I advise you to take time to read the documentation for
other subsystems.  You will find that narrating what the code does, as you
are conveying in this work, is not a common practice.  The code speaks for
itself, there is no need for duplication.

You are obviously able to read and understand code, have you thought about
fixing kernel problems?  Buy a development board, boot a kernel on it, follow
the patches people are sending for that specific SoC and start testing them.
You will find more problems to fix that you can handle, and it's a lot of fun.

Regards,
Mathieu

> +
> +The framework oversees the lifecycle of the remote and
> +provides the `rproc_report_crash` function, which the driver invokes
> +upon receiving a crash notification from the remote. The
> +notification method can differ based on the design of the remote
> +processor and its communication with the application processor. For
> +instance, if the remote is a DSP equipped with a watchdog,
> +unresponsive behavior triggers the watchdog, generating an interrupt
> +that routes to the application processor, allowing it to call
> +`rproc_report_crash` in the driver's interrupt context.
> +
> +During crash handling, the framework performs the following actions:
> +
> +a. Sends a request to stop the remote and any connected or
> +   dependent subdevices.
> +b. Generates a coredump, dumping all `resources` requested by the
> +   remote alongside relevant debugging information. Resources are
> +   explained below.
> +c. Reloads the firmware and restarts the remote processor.
> +
> +If the `RPROC_FEAT_ATTACH_ON_RECOVERY` flag is set, the detach and
> +attach callbacks of the driver are invoked without reloading the
> +firmware. This is useful when the remote requires no
> +assistance for recovery, or when the application processor can restart
> +independently. After recovery, the application processor can reattach
> +to the remote.
> +
>  User API
>  ========
>  
> -- 
> 2.39.3 (Apple Git-146)
> 

Powered by blists - more mailing lists