Message-ID: <Zx8KBkBPX5qJYWWg@archie.me>
Date: Mon, 28 Oct 2024 10:50:30 +0700
From: Bagas Sanjaya <bagasdotme@...il.com>
To: anish kumar <yesanishhere@...il.com>, andersson@...nel.org,
	mathieu.poirier@...aro.orgi, corbet@....net
Cc: linux-doc@...r.kernel.org, linux-remoteproc@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH V5] remoteproc: Documentation: update with details

On Sat, Oct 26, 2024 at 02:22:59PM -0700, anish kumar wrote:
> +The rproc device structure looks like this::
> +
> +  struct rproc {
> +	struct list_head node;
> +	struct iommu_domain *domain;
> +	const char *name;
> +	const char *firmware;
> +	void *priv;
> +	struct rproc_ops *ops;
> +	struct device dev;
> +	atomic_t power;
> +	unsigned int state;
> +	enum rproc_dump_mechanism dump_conf;
> +	struct mutex lock;
> +	struct dentry *dbg_dir;
> +	struct list_head traces;
> +	int num_traces;
> +	struct list_head carveouts;
> +	struct list_head mappings;
> +	u64 bootaddr;
> +	struct list_head rvdevs;
> +	struct list_head subdevs;
> +	struct idr notifyids;
> +	int index;
> +	struct work_struct crash_handler;
> +	unsigned int crash_cnt;
> +	bool recovery_disabled;
> +	int max_notifyid;
> +	struct resource_table *table_ptr;
> +	struct resource_table *clean_table;
> +	struct resource_table *cached_table;
> +	size_t table_sz;
> +	bool has_iommu;
> +	bool auto_boot;
> +	bool sysfs_read_only;
> +	struct list_head dump_segments;
> +	int nb_vdev;
> +	u8 elf_class;
> +	u16 elf_machine;
> +	struct cdev cdev;
> +	bool cdev_put_on_release;
> +	DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
> +  };
> +
> +It contains following fields:
> +
> +* node: list node of this rproc object
> +* domain: iommu domain
> +* name: human readable name of the rproc
> +* firmware: name of firmware file to be loaded
> +* priv: private data which belongs to the platform-specific rproc module
> +* ops: platform-specific start/stop rproc handlers
> +* dev: virtual device for refcounting and common remoteproc behavior
> +* power: refcount of users who need this rproc powered up
> +* state: state of the device
> +* dump_conf: Currently selected coredump configuration
> +* lock: lock which protects concurrent manipulations of the rproc
> +* dbg_dir: debugfs directory of this rproc device
> +* traces: list of trace buffers
> +* num_traces: number of trace buffers
> +* carveouts: list of physically contiguous memory allocations
> +* mappings: list of iommu mappings we initiated, needed on shutdown
> +* bootaddr: address of first instruction to boot rproc with (optional)
> +* rvdevs: list of remote virtio devices
> +* subdevs: list of subdevices, to following the running state
> +* notifyids: idr for dynamically assigning rproc-wide unique notify ids
> +* index: index of this rproc device
> +* crash_handler: workqueue for handling a crash
> +* crash_cnt: crash counter
> +* recovery_disabled: flag that state if recovery was disabled
> +* max_notifyid: largest allocated notify id.
> +* table_ptr: pointer to the resource table in effect
> +* clean_table: copy of the resource table without modifications.  Used
> +*      	 when a remote processor is attached or detached from the core
> +* cached_table: copy of the resource table
> +* table_sz: size of @cached_table
> +* has_iommu: flag to indicate if remote processor is behind an MMU
> +* auto_boot: flag to indicate if remote processor should be auto-started
> +* sysfs_read_only: flag to make remoteproc sysfs files read only
> +* dump_segments: list of segments in the firmware
> +* nb_vdev: number of vdev currently handled by rproc
> +* elf_class: firmware ELF class
> +* elf_machine: firmware ELF machine
> +* cdev: character device of the rproc
> +* cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
> +* features: indicate remoteproc features

clean_table list item, has two separated bullets instead of being spanned
across two lines, so I fix its formatting:

---- >8 ----
diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index 1c15f4d1b9eb43..b456f94a1d73f1 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -260,7 +260,7 @@ It contains following fields:
 * max_notifyid: largest allocated notify id.
 * table_ptr: pointer to the resource table in effect
 * clean_table: copy of the resource table without modifications.  Used
-*      	 when a remote processor is attached or detached from the core
+  when a remote processor is attached or detached from the core
 * cached_table: copy of the resource table
 * table_sz: size of @cached_table
 * has_iommu: flag to indicate if remote processor is behind an MMU

> +
> +The list of rproc operations is defined as::
> +
> +  struct rproc_ops {
> +	int (*prepare)(struct rproc *rproc);
> +	int (*unprepare)(struct rproc *rproc);
> +	int (*start)(struct rproc *rproc);
> +	int (*stop)(struct rproc *rproc);
> +	int (*attach)(struct rproc *rproc);
> +	int (*detach)(struct rproc *rproc);
> +	void (*kick)(struct rproc *rproc, int vqid);
> +	void * (*da_to_va)(struct rproc *rproc, u64 da, size_t len, bool *is_iomem);
> +	int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
> +	int (*handle_rsc)(struct rproc *rproc, u32 rsc_type, void *rsc,
> +			  int offset, int avail);
> +	struct resource_table *(*find_loaded_rsc_table)(
> +				struct rproc *rproc, const struct firmware *fw);
> +	struct resource_table *(*get_loaded_rsc_table)(
> +				struct rproc *rproc, size_t *size);
> +	int (*load)(struct rproc *rproc, const struct firmware *fw);
> +	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> +	u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
> +	unsigned long (*panic)(struct rproc *rproc);
> +	void (*coredump)(struct rproc *rproc);
> +  };
> +
> <snipped>...
> +Drivers must fill the `rproc_ops` structure and call `rproc_alloc`
> +to register themselves with the framework.
> +
> +.. code-block:: c
> +
> +   struct rproc_ops {
> +       int (*prepare)(struct rproc *rproc);
> +       int (*unprepare)(struct rproc *rproc);
> +       int (*start)(struct rproc *rproc);
> +       int (*stop)(struct rproc *rproc);
> +       int (*attach)(struct rproc *rproc);
> +       int (*detach)(struct rproc *rproc);
> +       void * (*da_to_va)(struct rproc *rproc, u64 da, size_t len,
> +                          bool *is_iomem);
> +       int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
> +       int (*handle_rsc)(struct rproc *rproc, u32 rsc_type,
> +                         void *rsc, int offset, int avail);
> +       int (*load)(struct rproc *rproc, const struct firmware *fw);
> +       //snip
> +   };
> +
>  ::
>  
>    struct rproc *rproc_alloc(struct device *dev, const char *name,
> @@ -190,6 +485,35 @@ platform specific rproc implementation. This should not be called from a
>  non-remoteproc driver. This function can be called from atomic/interrupt
>  context.
>  
> +To add a subdev corresponding driver can call
> +
> +::
> +
> +  void rproc_add_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
> +
> +To remove a subdev, driver can call.
> +
> +::
> +
> +  void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
> +
> +To work with ELF coredump below function can be called
> +
> +::
> +
> +  void rproc_coredump_cleanup(struct rproc *rproc)
> +  void rproc_coredump(struct rproc *rproc)
> +  void rproc_coredump_using_sections(struct rproc *rproc)
> +  int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size)
> +  int rproc_coredump_add_custom_segment(struct rproc *rproc,
> +                                        dma_addr_t da, size_t size,
> +                                        void (*dumpfn)(struct rproc *rproc,
> +                                        struct rproc_dump_segment *segment,
> +                                        void *dest, size_t offset,
> +                                        size_t size))
> +

For consistency, all other C snippets should be syntax-highlighted as such
by ``code-block:: c`` directive.

> +Below set of commands can be used to start and stop the rproc
> +where 'X' refers to instance of associated remoteproc. There can be systems
> +where there are more than one rprocs such as multiple DSP's
> +connected to application processors running Linux.
> +
> +.. code-block:: c
> +
> +   echo start > /sys/class/remoteproc/remoteprocX/state
> +   echo stop > /sys/class/remoteproc/remoteprocX/state

Shouldn't this and following shell snippets use bash syntax highlighting
(``code-block:: bash``) or simply double colon (``::``) instead?

> +
> +To know the state of rproc:
"To retrieve ..."
> +
> +.. code-block:: c
> +
> +   cat /sys/class/remoteproc/remoteprocX/state
> +
> +
> +To dynamically replace firmware, execute the following commands:
> +
> +.. code-block:: c
> +
> +   echo stop > /sys/class/remoteproc/remoteprocX/state
> +   echo -n <firmware_name> >
> +   /sys/class/remoteproc/remoteprocX/firmware
> +   echo start > /sys/class/remoteproc/remoteprocX/state
> +
> +To simulate a remote crash, execute:
> +
> +.. code-block:: c
> +
> +   echo 1 > /sys/kernel/debug/remoteproc/remoteprocX/crash
> +
> +To get the trace logs, execute
> +
> +.. code-block:: c
> +
> +   cat /sys/kernel/debug/remoteproc/remoteprocX/crashX
> +
> +where X will be 0 or 1 if there are 2 resources. Also, this
> +file will only exist if resources are defined in ELF firmware
> +file.
> +
> +The coredump feature can be disabled with the following command:
> +
> +.. code-block:: c
> +
> +   echo disabled > /sys/kernel/debug/remoteproc/remoteprocX/coredump
> +
> +Userspace can also control start/stop of rproc by using a
> +remoteproc Character Device, it can open the open a file descriptor
> +and write `start` to initiate it, and `stop` to terminate it.
> +Below set of api's can be used to start and stop the rproc
> +where 'X' refers to instance of associated remoteproc. There can be systems
> +where there are more than one rprocs such as multiple DSP's
> +connected to application processors running Linux.
> +
> +.. code-block:: c
> +
> +   echo start > /sys/class/remoteproc/remoteprocX/state
> +   echo stop > /sys/class/remoteproc/remoteprocX/state
> +
> +To know the state of rproc:
> +
> +.. code-block:: c
> +
> +   cat /sys/class/remoteproc/remoteprocX/state
> +
> +
> +To dynamically replace firmware, execute the following commands:
> +
> +.. code-block:: c
> +
> +   echo stop > /sys/class/remoteproc/remoteprocX/state
> +   echo -n <firmware_name> >
> +   /sys/class/remoteproc/remoteprocX/firmware
> +   echo start > /sys/class/remoteproc/remoteprocX/state
> +
> +To simulate a remote crash, execute:
> +
> +.. code-block:: c
> +
> +   echo 1 > /sys/kernel/debug/remoteproc/remoteprocX/crash
> +
> +To get the trace logs, execute
> +
> +.. code-block:: c
> +
> +   cat /sys/kernel/debug/remoteproc/remoteprocX/crashX
> +
> +where X will be 0 or 1 if there are 2 resources. Also, this
> +file will only exist if resources are defined in ELF firmware
> +file.
> +
> +The coredump feature can be disabled with the following command:
> +
> +.. code-block:: c
> +
> +   echo disabled > /sys/kernel/debug/remoteproc/remoteprocX/coredump
> +
> +Userspace can also control start/stop of rproc by using a
> +remoteproc Character Device, it can open the open a file descriptor
> +and write `start` to initiate it, and `stop` to terminate it.
> +

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists