[RESEND PATCH V6 0/3] remoteproc documentation update

[RESEND PATCH V6 0/3] remoteproc documentation update

[anish kumar]

V6:
divided the patches for each section as suggested by mathieu.
First patch is updating introduction section
second patch is for new overview section as suggested
third patch is for devm version of rprod_add

V5:
based on comment from mathieu poirier, remove all files
and combined that in the original file and as he adviced
nothing with respect to old documentation was changed.

V4:
Fixed compilation errors and moved documentation to
driver-api directory.

V3:
Seperated out the patches further to make the intention
clear for each patch.

V2:
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202410161444.jOKMsoGS-lkp@intel.com/

Hello Folks,

Sending this patchset again as revision was missing in the
last series also added cover letter changes.

Thanks,
anish

anish kumar (3):
  Documentation: remoteproc: update introduction section
  Documentation: remoteproc: add overview section
  Documentation: remoteproc: add a note to rproc_add

 Documentation/staging/remoteproc.rst | 68 ++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

-- 
2.39.3 (Apple Git-146)

[RESEND PATCH V6 1/3] Documentation: remoteproc: update introduction section

[anish kumar]

Update the intrduction section to add key components
provided by remote processor framework.

Signed-off-by: anish kumar <yesanishhere@gmail.com>
---
 Documentation/staging/remoteproc.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index 348ee7e508ac..eeebbeca71de 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -29,6 +29,23 @@ remoteproc will add those devices. This makes it possible to reuse the
 existing virtio drivers with remote processor backends at a minimal development
 cost.
 
+The primary purpose of the remoteproc framework is to download firmware
+for remote processors and manage their lifecycle. The framework consists
+of several key components:
+
+- **Character Driver**: Provides userspace access to control the remote
+  processor.
+- **ELF Utility**: Offers functions for handling ELF files and managing
+  resources requested by the remote processor.
+- **Remoteproc Core**: Manages firmware downloads and recovery actions
+  in case of a remote processor crash.
+- **Coredump**: Provides facilities for coredumping and tracing from
+  the remote processor in the event of a crash.
+- **Userspace Interaction**: Uses sysfs and debugfs to manage the
+  lifecycle and status of the remote processor.
+- **Virtio Support**: Facilitates interaction with the virtio and
+  rpmsg bus.
+
 User API
 ========
 
-- 
2.39.3 (Apple Git-146)

Re: [RESEND PATCH V6 1/3] Documentation: remoteproc: update introduction section

[Mathieu Poirier]

On Tue, Nov 05, 2024 at 09:10:14PM -0800, anish kumar wrote:
> Update the intrduction section to add key components
> provided by remote processor framework.
> 
> Signed-off-by: anish kumar <yesanishhere@gmail.com>
> ---
>  Documentation/staging/remoteproc.rst | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
> index 348ee7e508ac..eeebbeca71de 100644
> --- a/Documentation/staging/remoteproc.rst
> +++ b/Documentation/staging/remoteproc.rst
> @@ -29,6 +29,23 @@ remoteproc will add those devices. This makes it possible to reuse the
>  existing virtio drivers with remote processor backends at a minimal development
>  cost.
>  
> +The primary purpose of the remoteproc framework is to download firmware
> +for remote processors and manage their lifecycle. The framework consists
> +of several key components:
> +
> +- **Character Driver**: Provides userspace access to control the remote
> +  processor.
> +- **ELF Utility**: Offers functions for handling ELF files and managing
> +  resources requested by the remote processor.
> +- **Remoteproc Core**: Manages firmware downloads and recovery actions
> +  in case of a remote processor crash.
> +- **Coredump**: Provides facilities for coredumping and tracing from
> +  the remote processor in the event of a crash.
> +- **Userspace Interaction**: Uses sysfs and debugfs to manage the
> +  lifecycle and status of the remote processor.
> +- **Virtio Support**: Facilitates interaction with the virtio and
> +  rpmsg bus.

Some of the above is either inaccurate or incomplete.  It would be fairly time
consuming for me to point out where the problems are, especially since I don't
see a lot of value in adding this section.

> +
>  User API
>  ========
>  
> -- 
> 2.39.3 (Apple Git-146)
> 

[RESEND PATCH V6 2/3] Documentation: remoteproc: add overview section

[anish kumar]

Added overview section which details
how the remote processor framework works and
how it handles crashes.

Signed-off-by: anish kumar <yesanishhere@gmail.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.
+
+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.
+
+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)

Re: [RESEND PATCH V6 2/3] Documentation: remoteproc: add overview section

[Mathieu Poirier]

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@gmail.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)
> 

[RESEND PATCH V6 3/3] Documentation: remoteproc: add a note to rproc_add

[anish kumar]

Added a note to the rproc_add description regarding
the availability of the resource-managed variant of the API.

Signed-off-by: anish kumar <yesanishhere@gmail.com>
---
 Documentation/staging/remoteproc.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index e0bf68ceade8..658ef7a28dd2 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -223,6 +223,14 @@ If found, those virtio devices will be created and added, so as a result
 of registering this remote processor, additional virtio drivers might get
 probed.
 
+.. note::
+
+  there is a resource managed version of this api called devm_rproc_add.
+  Most of the times, you should use that unless you need to explicitly
+  control the rproc registration with the core. If you are using devm
+  variant of this api, then rproc_del will automatically get called when
+  driver is unloaded.
+
 ::
 
   int rproc_del(struct rproc *rproc)
-- 
2.39.3 (Apple Git-146)

Re: [RESEND PATCH V6 3/3] Documentation: remoteproc: add a note to rproc_add

[Jonathan Corbet]

anish kumar <yesanishhere@gmail.com> writes:

> Added a note to the rproc_add description regarding
> the availability of the resource-managed variant of the API.
>
> Signed-off-by: anish kumar <yesanishhere@gmail.com>
> ---
>  Documentation/staging/remoteproc.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
> index e0bf68ceade8..658ef7a28dd2 100644
> --- a/Documentation/staging/remoteproc.rst
> +++ b/Documentation/staging/remoteproc.rst
> @@ -223,6 +223,14 @@ If found, those virtio devices will be created and added, so as a result
>  of registering this remote processor, additional virtio drivers might get
>  probed.
>  
> +.. note::
> +
> +  there is a resource managed version of this api called devm_rproc_add.

Please always use the function() notation.  Also, starting the sentence
with a capital letter would be a nice touch.

Thanks,

jon