* [PATCH 0/2] Minor remoteproc cleanups
@ 2021-05-19 18:03 ` Suman Anna
0 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-arm-kernel, linux-kernel, Suman Anna
Hi Bjorn, Mathieu,
The following series is minor document related cleanups. Some
of the warnings look to be very old, and these are not functional
issues, so I am not expecting these to be -stable content, and so
I did not split or add any Fixes: tags.
Patch 1 fixes a warning when building remoteproc core files with W=1,
and Patch 2 fixes all the existing kernel-doc warnings.
Patches are on top of v5.13-rc1.
regards
Suman
Suman Anna (2):
remoteproc: Add kernel-doc comment for is_iomem
remoteproc: Fix various kernel-doc warnings
drivers/remoteproc/remoteproc_core.c | 45 +++++++++++++------
drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
drivers/remoteproc/remoteproc_virtio.c | 6 ++-
include/linux/remoteproc.h | 50 ++++++++++++----------
4 files changed, 70 insertions(+), 43 deletions(-)
--
2.30.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Minor remoteproc cleanups
@ 2021-05-19 18:03 ` Suman Anna
0 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-kernel, linux-arm-kernel
Hi Bjorn, Mathieu,
The following series is minor document related cleanups. Some
of the warnings look to be very old, and these are not functional
issues, so I am not expecting these to be -stable content, and so
I did not split or add any Fixes: tags.
Patch 1 fixes a warning when building remoteproc core files with W=1,
and Patch 2 fixes all the existing kernel-doc warnings.
Patches are on top of v5.13-rc1.
regards
Suman
Suman Anna (2):
remoteproc: Add kernel-doc comment for is_iomem
remoteproc: Fix various kernel-doc warnings
drivers/remoteproc/remoteproc_core.c | 45 +++++++++++++------
drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
drivers/remoteproc/remoteproc_virtio.c | 6 ++-
include/linux/remoteproc.h | 50 ++++++++++++----------
4 files changed, 70 insertions(+), 43 deletions(-)
--
2.30.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] remoteproc: Add kernel-doc comment for is_iomem
2021-05-19 18:03 ` Suman Anna
@ 2021-05-19 18:03 ` Suman Anna
-1 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-arm-kernel, linux-kernel, Suman Anna
Add a kernel-doc comment for the is_iomem function argument in
rproc_da_to_va(). This fixes a warning generated when building
the remoteproc_core with W=1,
warning: Function parameter or member 'is_iomem' not described in 'rproc_da_to_va'
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 626a6b90fba2..8c279039b6a3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -165,6 +165,7 @@ EXPORT_SYMBOL(rproc_va_to_pa);
* @rproc: handle of a remote processor
* @da: remoteproc device address to translate
* @len: length of the memory region @da is pointing to
+ * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
*
* Some remote processors will ask us to allocate them physically contiguous
* memory regions (which we call "carveouts"), and map them to specific
--
2.30.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] remoteproc: Add kernel-doc comment for is_iomem
@ 2021-05-19 18:03 ` Suman Anna
0 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-kernel, linux-arm-kernel
Add a kernel-doc comment for the is_iomem function argument in
rproc_da_to_va(). This fixes a warning generated when building
the remoteproc_core with W=1,
warning: Function parameter or member 'is_iomem' not described in 'rproc_da_to_va'
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 626a6b90fba2..8c279039b6a3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -165,6 +165,7 @@ EXPORT_SYMBOL(rproc_va_to_pa);
* @rproc: handle of a remote processor
* @da: remoteproc device address to translate
* @len: length of the memory region @da is pointing to
+ * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
*
* Some remote processors will ask us to allocate them physically contiguous
* memory regions (which we call "carveouts"), and map them to specific
--
2.30.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
2021-05-19 18:03 ` Suman Anna
@ 2021-05-19 18:03 ` Suman Anna
-1 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-arm-kernel, linux-kernel, Suman Anna
Fix all the kernel-doc warnings in various remoteproc core files.
Some of them just needed a formatting cleanup change, while others
needed the Return statement to be added, or documenting the missed
structure elements.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
drivers/remoteproc/remoteproc_virtio.c | 6 ++-
include/linux/remoteproc.h | 50 ++++++++++++----------
4 files changed, 69 insertions(+), 43 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 8c279039b6a3..6348aaa42bbb 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
* translations on the internal remoteproc memory regions through a platform
* implementation specific da_to_va ops, if present.
*
- * The function returns a valid kernel address on success or NULL on failure.
- *
* Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
* but only on kernel direct mapped RAM memory. Instead, we're just using
* here the output of the DMA API for the carveouts, which should be more
* correct.
+ *
+ * Return: a valid kernel address on success or NULL on failure
*/
void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
{
@@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
* use RSC_DEVMEM resource entries to map their required @da to the physical
* address of their base CMA region (ouch, hacky!).
*
- * Returns 0 on success, or an appropriate error code otherwise
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
* support dynamically allocating this address using the generic
* DMA API (but currently there isn't a use case for that).
*
- * Returns 0 on success, or an appropriate error code otherwise
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_trace(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
* tell us ranges of physical addresses the firmware is allowed to request,
* and not allow firmwares to request access to physical addresses that
* are outside those ranges.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
*
* This function allocate specified memory entry @mem using
* dma_alloc_coherent() as default allocator
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_alloc_carveout(struct rproc *rproc,
struct rproc_mem_entry *mem)
@@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
*
* This function releases specified memory entry @mem allocated via
* rproc_alloc_carveout() function by @rproc.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_release_carveout(struct rproc *rproc,
struct rproc_mem_entry *mem)
@@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
* (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
* needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
* pressure is important; it may have a substantial impact on performance.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_carveout(struct rproc *rproc,
void *ptr, int offset, int avail)
@@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
*
* This function allocates a rproc_mem_entry struct and fill it with parameters
* provided by client.
+ *
+ * Return: a valid pointer on success, or NULL on failure
*/
__printf(8, 9)
struct rproc_mem_entry *
@@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
*
* This function allocates a rproc_mem_entry struct and fill it with parameters
* provided by client.
+ *
+ * Return: a valid pointer on success, or NULL on failure
*/
__printf(5, 6)
struct rproc_mem_entry *
@@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
* remoteproc functional again.
*
* This function can sleep, so it cannot be called from atomic context.
+ *
+ * Return: 0 on success or a negative value upon failure
*/
int rproc_trigger_recovery(struct rproc *rproc)
{
@@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
* If the remote processor is already powered on, this function immediately
* returns (successfully).
*
- * Returns 0 on success, and an appropriate error value otherwise.
+ * Return: 0 on success, and an appropriate error value otherwise
*/
int rproc_boot(struct rproc *rproc)
{
@@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
* no longer available. From there it should be possible to remove the
* platform driver and even power cycle the application processor (if the HW
* supports it) without needing to switch off the remote processor.
+ *
+ * Return: 0 on success, and an appropriate error value otherwise
*/
int rproc_detach(struct rproc *rproc)
{
@@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
* This function increments the remote processor's refcount, so always
* use rproc_put() to decrement it back once rproc isn't needed anymore.
*
- * Returns the rproc handle on success, and NULL on failure.
+ * Return: rproc handle on success, and NULL on failure
*/
#ifdef CONFIG_OF
struct rproc *rproc_get_by_phandle(phandle phandle)
@@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
* This is called by the platform-specific rproc implementation, whenever
* a new remote processor device is probed.
*
- * Returns 0 on success and an appropriate error code otherwise.
- *
* Note: this function initiates an asynchronous firmware loading
* context, which will look for virtio devices supported by the rproc's
* firmware.
@@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
* If found, those virtio devices will be created and added, so as a result
* of registering this remote processor, additional virtio drivers might be
* probed.
+ *
+ * Return: 0 on success and an appropriate error code otherwise
*/
int rproc_add(struct rproc *rproc)
{
@@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
* This function performs like rproc_add() but the registered rproc device will
* automatically be removed on driver detach.
*
- * Returns: 0 on success, negative errno on failure
+ * Return: 0 on success, negative errno on failure
*/
int devm_rproc_add(struct device *dev, struct rproc *rproc)
{
@@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
* implementations should then call rproc_add() to complete
* the registration of the remote processor.
*
- * On success the new rproc is returned, and on failure, NULL.
- *
* Note: _never_ directly deallocate @rproc, even if it was not registered
* yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
+ *
+ * Return: new rproc pointer on success, and NULL on failure
*/
struct rproc *rproc_alloc(struct device *dev, const char *name,
const struct rproc_ops *ops,
@@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
* of the outstanding reference created by rproc_alloc. To decrement that
* one last refcount, one still needs to call rproc_free().
*
- * Returns 0 on success and -EINVAL if @rproc isn't valid.
+ * Return: 0 on success and -EINVAL if @rproc isn't valid
*/
int rproc_del(struct rproc *rproc)
{
@@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
* This function performs like rproc_alloc() but the acquired rproc device will
* automatically be released on driver detach.
*
- * Returns: new rproc instance, or NULL on failure
+ * Return: new rproc instance, or NULL on failure
*/
struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
const struct rproc_ops *ops,
@@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
* rproc_get_by_child() - acquire rproc handle of @dev's ancestor
* @dev: child device to find ancestor of
*
- * Returns the ancestor rproc instance, or NULL if not found.
+ * Return: the ancestor rproc instance, or NULL if not found
*/
struct rproc *rproc_get_by_child(struct device *dev)
{
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 11423588965a..469c52e62faf 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -31,6 +31,8 @@
* @fw: the ELF firmware image
*
* Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
+ *
+ * Return: 0 on success and -EINVAL upon any failure
*/
int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
{
@@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
* @rproc: the remote processor handle
* @fw: the ELF firmware image
*
- * This function returns the entry point address of the ELF
- * image.
- *
* Note that the boot address is not a configurable property of all remote
* processors. Some will always boot at a specific hard-coded address.
+ *
+ * Return: entry point address of the ELF image
+ *
*/
u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
{
@@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
* might be different: they might not have iommus, and would prefer to
* directly allocate memory for every segment/resource. This is not yet
* supported, though.
+ *
+ * Return: 0 on success and an appropriate error code otherwise
*/
int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
{
@@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
* This function finds the location of the loaded resource table. Don't
* call this function if the table wasn't loaded yet - it's a bug if you do.
*
- * Returns the pointer to the resource table if it is found or NULL otherwise.
+ * Return: pointer to the resource table if it is found or NULL otherwise.
* If the table wasn't loaded yet the result is unspecified.
*/
struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0cc617f76068..cf4d54e98e6a 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
* when the remote processor signals that a specific virtqueue has pending
* messages available.
*
- * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
+ * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
* and otherwise returns IRQ_HANDLED.
*/
irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
@@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
* This function registers a virtio device. This vdev's partent is
* the rproc device.
*
- * Returns 0 on success or an appropriate error value otherwise.
+ * Return: 0 on success or an appropriate error value otherwise
*/
int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
{
@@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
* @data: must be null
*
* This function unregisters an existing virtio device.
+ *
+ * Return: 0
*/
int rproc_remove_virtio_dev(struct device *dev, void *data)
{
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8b795b544f75..42a1f30e33a7 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -243,7 +243,7 @@ struct fw_rsc_trace {
* @da: device address
* @align: the alignment between the consumer and producer parts of the vring
* @num: num of buffers supported by this vring (must be power of two)
- * @notifyid is a unique rproc-wide notify index for this vring. This notify
+ * @notifyid: a unique rproc-wide notify index for this vring. This notify
* index is used when kicking a remote processor, to let it know that this
* vring is triggered.
* @pa: physical address
@@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
/**
* struct fw_rsc_vdev - virtio device header
* @id: virtio device id (as in virtio_ids.h)
- * @notifyid is a unique rproc-wide notify index for this vdev. This notify
+ * @notifyid: a unique rproc-wide notify index for this vdev. This notify
* index is used when kicking a remote processor, to let it know that the
* status/features of this vdev have changes.
- * @dfeatures specifies the virtio device features supported by the firmware
- * @gfeatures is a place holder used by the host to write back the
+ * @dfeatures: specifies the virtio device features supported by the firmware
+ * @gfeatures: a place holder used by the host to write back the
* negotiated features that are supported by both sides.
- * @config_len is the size of the virtio config space of this vdev. The config
+ * @config_len: the size of the virtio config space of this vdev. The config
* space lies in the resource table immediate after this vdev header.
- * @status is a place holder where the host will indicate its virtio progress.
- * @num_of_vrings indicates how many vrings are described in this vdev header
+ * @status: a place holder where the host will indicate its virtio progress.
+ * @num_of_vrings: indicates how many vrings are described in this vdev header
* @reserved: reserved (must be zero)
- * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
+ * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
*
* This resource is a virtio device header: it provides information about
* the vdev, and is then used by the host and its peer remote processors
@@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
* to statically allocate a vdev upon registration of the rproc (dynamic vdev
* allocation is not yet supported).
*
- * Note: unlike virtualization systems, the term 'host' here means
- * the Linux side which is running remoteproc to control the remote
- * processors. We use the name 'gfeatures' to comply with virtio's terms,
- * though there isn't really any virtualized guest OS here: it's the host
- * which is responsible for negotiating the final features.
- * Yeah, it's a bit confusing.
- *
- * Note: immediately following this structure is the virtio config space for
- * this vdev (which is specific to the vdev; for more info, read the virtio
- * spec). the size of the config space is specified by @config_len.
+ * Note:
+ * 1. unlike virtualization systems, the term 'host' here means
+ * the Linux side which is running remoteproc to control the remote
+ * processors. We use the name 'gfeatures' to comply with virtio's terms,
+ * though there isn't really any virtualized guest OS here: it's the host
+ * which is responsible for negotiating the final features.
+ * Yeah, it's a bit confusing.
+ *
+ * 2. immediately following this structure is the virtio config space for
+ * this vdev (which is specific to the vdev; for more info, read the virtio
+ * spec). the size of the config space is specified by @config_len.
*/
struct fw_rsc_vdev {
u32 id;
@@ -440,7 +441,7 @@ enum rproc_state {
* enum rproc_crash_type - remote processor crash types
* @RPROC_MMUFAULT: iommu fault
* @RPROC_WATCHDOG: watchdog bite
- * @RPROC_FATAL_ERROR fatal error
+ * @RPROC_FATAL_ERROR: fatal error
*
* Each element of the enum is used as an array index. So that, the value of
* the elements should be always something sane.
@@ -457,9 +458,9 @@ enum rproc_crash_type {
* enum rproc_dump_mechanism - Coredump options for core
* @RPROC_COREDUMP_DISABLED: Don't perform any dump
* @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
- recovery
+ * recovery
* @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
- recovery until all segments are read
+ * recovery until all segments are read
*/
enum rproc_dump_mechanism {
RPROC_COREDUMP_DISABLED,
@@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
* @priv: private data associated with the dump_segment
* @dump: custom dump function to fill device memory segment associated
* with coredump
+ * @offset: offset of the segment
*/
struct rproc_dump_segment {
struct list_head node;
@@ -524,7 +526,9 @@ struct rproc_dump_segment {
* @auto_boot: flag to indicate if remote processor should be auto-started
* @dump_segments: list of segments in the firmware
* @nb_vdev: number of vdev currently handled by rproc
- * @char_dev: character device of the 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
*/
struct rproc {
@@ -613,10 +617,10 @@ struct rproc_vring {
* struct rproc_vdev - remoteproc state for a supported virtio device
* @refcount: reference counter for the vdev and vring allocations
* @subdev: handle for registering the vdev as a rproc subdevice
+ * @dev: device struct used for reference count semantics
* @id: virtio device id (as in virtio_ids.h)
* @node: list node
* @rproc: the rproc handle
- * @vdev: the virio device
* @vring: the vrings for this vdev
* @rsc_offset: offset of the vdev's resource entry
* @index: vdev position versus other vdev declared in resource table
--
2.30.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
@ 2021-05-19 18:03 ` Suman Anna
0 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-kernel, linux-arm-kernel
Fix all the kernel-doc warnings in various remoteproc core files.
Some of them just needed a formatting cleanup change, while others
needed the Return statement to be added, or documenting the missed
structure elements.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
drivers/remoteproc/remoteproc_virtio.c | 6 ++-
include/linux/remoteproc.h | 50 ++++++++++++----------
4 files changed, 69 insertions(+), 43 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 8c279039b6a3..6348aaa42bbb 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
* translations on the internal remoteproc memory regions through a platform
* implementation specific da_to_va ops, if present.
*
- * The function returns a valid kernel address on success or NULL on failure.
- *
* Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
* but only on kernel direct mapped RAM memory. Instead, we're just using
* here the output of the DMA API for the carveouts, which should be more
* correct.
+ *
+ * Return: a valid kernel address on success or NULL on failure
*/
void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
{
@@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
* use RSC_DEVMEM resource entries to map their required @da to the physical
* address of their base CMA region (ouch, hacky!).
*
- * Returns 0 on success, or an appropriate error code otherwise
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
* support dynamically allocating this address using the generic
* DMA API (but currently there isn't a use case for that).
*
- * Returns 0 on success, or an appropriate error code otherwise
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_trace(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
* tell us ranges of physical addresses the firmware is allowed to request,
* and not allow firmwares to request access to physical addresses that
* are outside those ranges.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
int offset, int avail)
@@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
*
* This function allocate specified memory entry @mem using
* dma_alloc_coherent() as default allocator
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_alloc_carveout(struct rproc *rproc,
struct rproc_mem_entry *mem)
@@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
*
* This function releases specified memory entry @mem allocated via
* rproc_alloc_carveout() function by @rproc.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_release_carveout(struct rproc *rproc,
struct rproc_mem_entry *mem)
@@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
* (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
* needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
* pressure is important; it may have a substantial impact on performance.
+ *
+ * Return: 0 on success, or an appropriate error code otherwise
*/
static int rproc_handle_carveout(struct rproc *rproc,
void *ptr, int offset, int avail)
@@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
*
* This function allocates a rproc_mem_entry struct and fill it with parameters
* provided by client.
+ *
+ * Return: a valid pointer on success, or NULL on failure
*/
__printf(8, 9)
struct rproc_mem_entry *
@@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
*
* This function allocates a rproc_mem_entry struct and fill it with parameters
* provided by client.
+ *
+ * Return: a valid pointer on success, or NULL on failure
*/
__printf(5, 6)
struct rproc_mem_entry *
@@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
* remoteproc functional again.
*
* This function can sleep, so it cannot be called from atomic context.
+ *
+ * Return: 0 on success or a negative value upon failure
*/
int rproc_trigger_recovery(struct rproc *rproc)
{
@@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
* If the remote processor is already powered on, this function immediately
* returns (successfully).
*
- * Returns 0 on success, and an appropriate error value otherwise.
+ * Return: 0 on success, and an appropriate error value otherwise
*/
int rproc_boot(struct rproc *rproc)
{
@@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
* no longer available. From there it should be possible to remove the
* platform driver and even power cycle the application processor (if the HW
* supports it) without needing to switch off the remote processor.
+ *
+ * Return: 0 on success, and an appropriate error value otherwise
*/
int rproc_detach(struct rproc *rproc)
{
@@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
* This function increments the remote processor's refcount, so always
* use rproc_put() to decrement it back once rproc isn't needed anymore.
*
- * Returns the rproc handle on success, and NULL on failure.
+ * Return: rproc handle on success, and NULL on failure
*/
#ifdef CONFIG_OF
struct rproc *rproc_get_by_phandle(phandle phandle)
@@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
* This is called by the platform-specific rproc implementation, whenever
* a new remote processor device is probed.
*
- * Returns 0 on success and an appropriate error code otherwise.
- *
* Note: this function initiates an asynchronous firmware loading
* context, which will look for virtio devices supported by the rproc's
* firmware.
@@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
* If found, those virtio devices will be created and added, so as a result
* of registering this remote processor, additional virtio drivers might be
* probed.
+ *
+ * Return: 0 on success and an appropriate error code otherwise
*/
int rproc_add(struct rproc *rproc)
{
@@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
* This function performs like rproc_add() but the registered rproc device will
* automatically be removed on driver detach.
*
- * Returns: 0 on success, negative errno on failure
+ * Return: 0 on success, negative errno on failure
*/
int devm_rproc_add(struct device *dev, struct rproc *rproc)
{
@@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
* implementations should then call rproc_add() to complete
* the registration of the remote processor.
*
- * On success the new rproc is returned, and on failure, NULL.
- *
* Note: _never_ directly deallocate @rproc, even if it was not registered
* yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
+ *
+ * Return: new rproc pointer on success, and NULL on failure
*/
struct rproc *rproc_alloc(struct device *dev, const char *name,
const struct rproc_ops *ops,
@@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
* of the outstanding reference created by rproc_alloc. To decrement that
* one last refcount, one still needs to call rproc_free().
*
- * Returns 0 on success and -EINVAL if @rproc isn't valid.
+ * Return: 0 on success and -EINVAL if @rproc isn't valid
*/
int rproc_del(struct rproc *rproc)
{
@@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
* This function performs like rproc_alloc() but the acquired rproc device will
* automatically be released on driver detach.
*
- * Returns: new rproc instance, or NULL on failure
+ * Return: new rproc instance, or NULL on failure
*/
struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
const struct rproc_ops *ops,
@@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
* rproc_get_by_child() - acquire rproc handle of @dev's ancestor
* @dev: child device to find ancestor of
*
- * Returns the ancestor rproc instance, or NULL if not found.
+ * Return: the ancestor rproc instance, or NULL if not found
*/
struct rproc *rproc_get_by_child(struct device *dev)
{
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 11423588965a..469c52e62faf 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -31,6 +31,8 @@
* @fw: the ELF firmware image
*
* Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
+ *
+ * Return: 0 on success and -EINVAL upon any failure
*/
int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
{
@@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
* @rproc: the remote processor handle
* @fw: the ELF firmware image
*
- * This function returns the entry point address of the ELF
- * image.
- *
* Note that the boot address is not a configurable property of all remote
* processors. Some will always boot at a specific hard-coded address.
+ *
+ * Return: entry point address of the ELF image
+ *
*/
u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
{
@@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
* might be different: they might not have iommus, and would prefer to
* directly allocate memory for every segment/resource. This is not yet
* supported, though.
+ *
+ * Return: 0 on success and an appropriate error code otherwise
*/
int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
{
@@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
* This function finds the location of the loaded resource table. Don't
* call this function if the table wasn't loaded yet - it's a bug if you do.
*
- * Returns the pointer to the resource table if it is found or NULL otherwise.
+ * Return: pointer to the resource table if it is found or NULL otherwise.
* If the table wasn't loaded yet the result is unspecified.
*/
struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0cc617f76068..cf4d54e98e6a 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
* when the remote processor signals that a specific virtqueue has pending
* messages available.
*
- * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
+ * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
* and otherwise returns IRQ_HANDLED.
*/
irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
@@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
* This function registers a virtio device. This vdev's partent is
* the rproc device.
*
- * Returns 0 on success or an appropriate error value otherwise.
+ * Return: 0 on success or an appropriate error value otherwise
*/
int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
{
@@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
* @data: must be null
*
* This function unregisters an existing virtio device.
+ *
+ * Return: 0
*/
int rproc_remove_virtio_dev(struct device *dev, void *data)
{
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8b795b544f75..42a1f30e33a7 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -243,7 +243,7 @@ struct fw_rsc_trace {
* @da: device address
* @align: the alignment between the consumer and producer parts of the vring
* @num: num of buffers supported by this vring (must be power of two)
- * @notifyid is a unique rproc-wide notify index for this vring. This notify
+ * @notifyid: a unique rproc-wide notify index for this vring. This notify
* index is used when kicking a remote processor, to let it know that this
* vring is triggered.
* @pa: physical address
@@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
/**
* struct fw_rsc_vdev - virtio device header
* @id: virtio device id (as in virtio_ids.h)
- * @notifyid is a unique rproc-wide notify index for this vdev. This notify
+ * @notifyid: a unique rproc-wide notify index for this vdev. This notify
* index is used when kicking a remote processor, to let it know that the
* status/features of this vdev have changes.
- * @dfeatures specifies the virtio device features supported by the firmware
- * @gfeatures is a place holder used by the host to write back the
+ * @dfeatures: specifies the virtio device features supported by the firmware
+ * @gfeatures: a place holder used by the host to write back the
* negotiated features that are supported by both sides.
- * @config_len is the size of the virtio config space of this vdev. The config
+ * @config_len: the size of the virtio config space of this vdev. The config
* space lies in the resource table immediate after this vdev header.
- * @status is a place holder where the host will indicate its virtio progress.
- * @num_of_vrings indicates how many vrings are described in this vdev header
+ * @status: a place holder where the host will indicate its virtio progress.
+ * @num_of_vrings: indicates how many vrings are described in this vdev header
* @reserved: reserved (must be zero)
- * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
+ * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
*
* This resource is a virtio device header: it provides information about
* the vdev, and is then used by the host and its peer remote processors
@@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
* to statically allocate a vdev upon registration of the rproc (dynamic vdev
* allocation is not yet supported).
*
- * Note: unlike virtualization systems, the term 'host' here means
- * the Linux side which is running remoteproc to control the remote
- * processors. We use the name 'gfeatures' to comply with virtio's terms,
- * though there isn't really any virtualized guest OS here: it's the host
- * which is responsible for negotiating the final features.
- * Yeah, it's a bit confusing.
- *
- * Note: immediately following this structure is the virtio config space for
- * this vdev (which is specific to the vdev; for more info, read the virtio
- * spec). the size of the config space is specified by @config_len.
+ * Note:
+ * 1. unlike virtualization systems, the term 'host' here means
+ * the Linux side which is running remoteproc to control the remote
+ * processors. We use the name 'gfeatures' to comply with virtio's terms,
+ * though there isn't really any virtualized guest OS here: it's the host
+ * which is responsible for negotiating the final features.
+ * Yeah, it's a bit confusing.
+ *
+ * 2. immediately following this structure is the virtio config space for
+ * this vdev (which is specific to the vdev; for more info, read the virtio
+ * spec). the size of the config space is specified by @config_len.
*/
struct fw_rsc_vdev {
u32 id;
@@ -440,7 +441,7 @@ enum rproc_state {
* enum rproc_crash_type - remote processor crash types
* @RPROC_MMUFAULT: iommu fault
* @RPROC_WATCHDOG: watchdog bite
- * @RPROC_FATAL_ERROR fatal error
+ * @RPROC_FATAL_ERROR: fatal error
*
* Each element of the enum is used as an array index. So that, the value of
* the elements should be always something sane.
@@ -457,9 +458,9 @@ enum rproc_crash_type {
* enum rproc_dump_mechanism - Coredump options for core
* @RPROC_COREDUMP_DISABLED: Don't perform any dump
* @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
- recovery
+ * recovery
* @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
- recovery until all segments are read
+ * recovery until all segments are read
*/
enum rproc_dump_mechanism {
RPROC_COREDUMP_DISABLED,
@@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
* @priv: private data associated with the dump_segment
* @dump: custom dump function to fill device memory segment associated
* with coredump
+ * @offset: offset of the segment
*/
struct rproc_dump_segment {
struct list_head node;
@@ -524,7 +526,9 @@ struct rproc_dump_segment {
* @auto_boot: flag to indicate if remote processor should be auto-started
* @dump_segments: list of segments in the firmware
* @nb_vdev: number of vdev currently handled by rproc
- * @char_dev: character device of the 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
*/
struct rproc {
@@ -613,10 +617,10 @@ struct rproc_vring {
* struct rproc_vdev - remoteproc state for a supported virtio device
* @refcount: reference counter for the vdev and vring allocations
* @subdev: handle for registering the vdev as a rproc subdevice
+ * @dev: device struct used for reference count semantics
* @id: virtio device id (as in virtio_ids.h)
* @node: list node
* @rproc: the rproc handle
- * @vdev: the virio device
* @vring: the vrings for this vdev
* @rsc_offset: offset of the vdev's resource entry
* @index: vdev position versus other vdev declared in resource table
--
2.30.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] remoteproc: Add kernel-doc comment for is_iomem
2021-05-19 18:03 ` Suman Anna
@ 2021-05-25 17:37 ` Mathieu Poirier
-1 siblings, 0 replies; 14+ messages in thread
From: Mathieu Poirier @ 2021-05-25 17:37 UTC (permalink / raw)
To: Suman Anna
Cc: Bjorn Andersson, linux-remoteproc, linux-arm-kernel, linux-kernel
On Wed, May 19, 2021 at 01:03:03PM -0500, Suman Anna wrote:
> Add a kernel-doc comment for the is_iomem function argument in
> rproc_da_to_va(). This fixes a warning generated when building
> the remoteproc_core with W=1,
> warning: Function parameter or member 'is_iomem' not described in 'rproc_da_to_va'
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> drivers/remoteproc/remoteproc_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 626a6b90fba2..8c279039b6a3 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -165,6 +165,7 @@ EXPORT_SYMBOL(rproc_va_to_pa);
> * @rproc: handle of a remote processor
> * @da: remoteproc device address to translate
> * @len: length of the memory region @da is pointing to
> + * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
> *
> * Some remote processors will ask us to allocate them physically contiguous
> * memory regions (which we call "carveouts"), and map them to specific
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] remoteproc: Add kernel-doc comment for is_iomem
@ 2021-05-25 17:37 ` Mathieu Poirier
0 siblings, 0 replies; 14+ messages in thread
From: Mathieu Poirier @ 2021-05-25 17:37 UTC (permalink / raw)
To: Suman Anna
Cc: Bjorn Andersson, linux-remoteproc, linux-arm-kernel, linux-kernel
On Wed, May 19, 2021 at 01:03:03PM -0500, Suman Anna wrote:
> Add a kernel-doc comment for the is_iomem function argument in
> rproc_da_to_va(). This fixes a warning generated when building
> the remoteproc_core with W=1,
> warning: Function parameter or member 'is_iomem' not described in 'rproc_da_to_va'
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> drivers/remoteproc/remoteproc_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 626a6b90fba2..8c279039b6a3 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -165,6 +165,7 @@ EXPORT_SYMBOL(rproc_va_to_pa);
> * @rproc: handle of a remote processor
> * @da: remoteproc device address to translate
> * @len: length of the memory region @da is pointing to
> + * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
> *
> * Some remote processors will ask us to allocate them physically contiguous
> * memory regions (which we call "carveouts"), and map them to specific
> --
> 2.30.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
2021-05-19 18:03 ` Suman Anna
@ 2021-05-25 18:00 ` Mathieu Poirier
-1 siblings, 0 replies; 14+ messages in thread
From: Mathieu Poirier @ 2021-05-25 18:00 UTC (permalink / raw)
To: Suman Anna
Cc: Bjorn Andersson, linux-remoteproc, linux-arm-kernel, linux-kernel
On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
> Fix all the kernel-doc warnings in various remoteproc core files.
> Some of them just needed a formatting cleanup change, while others
> needed the Return statement to be added, or documenting the missed
> structure elements.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
> drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
> drivers/remoteproc/remoteproc_virtio.c | 6 ++-
> include/linux/remoteproc.h | 50 ++++++++++++----------
> 4 files changed, 69 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 8c279039b6a3..6348aaa42bbb 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
> * translations on the internal remoteproc memory regions through a platform
> * implementation specific da_to_va ops, if present.
> *
> - * The function returns a valid kernel address on success or NULL on failure.
> - *
> * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
> * but only on kernel direct mapped RAM memory. Instead, we're just using
> * here the output of the DMA API for the carveouts, which should be more
> * correct.
> + *
> + * Return: a valid kernel address on success or NULL on failure
> */
> void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
> {
> @@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
> * use RSC_DEVMEM resource entries to map their required @da to the physical
> * address of their base CMA region (ouch, hacky!).
> *
> - * Returns 0 on success, or an appropriate error code otherwise
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
> * support dynamically allocating this address using the generic
> * DMA API (but currently there isn't a use case for that).
> *
> - * Returns 0 on success, or an appropriate error code otherwise
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_trace(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
> * tell us ranges of physical addresses the firmware is allowed to request,
> * and not allow firmwares to request access to physical addresses that
> * are outside those ranges.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
> *
> * This function allocate specified memory entry @mem using
> * dma_alloc_coherent() as default allocator
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_alloc_carveout(struct rproc *rproc,
> struct rproc_mem_entry *mem)
> @@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
> *
> * This function releases specified memory entry @mem allocated via
> * rproc_alloc_carveout() function by @rproc.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_release_carveout(struct rproc *rproc,
> struct rproc_mem_entry *mem)
> @@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
> * (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
> * needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
> * pressure is important; it may have a substantial impact on performance.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_carveout(struct rproc *rproc,
> void *ptr, int offset, int avail)
> @@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
> *
> * This function allocates a rproc_mem_entry struct and fill it with parameters
> * provided by client.
> + *
> + * Return: a valid pointer on success, or NULL on failure
> */
> __printf(8, 9)
> struct rproc_mem_entry *
> @@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
> *
> * This function allocates a rproc_mem_entry struct and fill it with parameters
> * provided by client.
> + *
> + * Return: a valid pointer on success, or NULL on failure
> */
> __printf(5, 6)
> struct rproc_mem_entry *
> @@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
> * remoteproc functional again.
> *
> * This function can sleep, so it cannot be called from atomic context.
> + *
> + * Return: 0 on success or a negative value upon failure
> */
> int rproc_trigger_recovery(struct rproc *rproc)
> {
> @@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
> * If the remote processor is already powered on, this function immediately
> * returns (successfully).
> *
> - * Returns 0 on success, and an appropriate error value otherwise.
> + * Return: 0 on success, and an appropriate error value otherwise
> */
> int rproc_boot(struct rproc *rproc)
> {
> @@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
> * no longer available. From there it should be possible to remove the
> * platform driver and even power cycle the application processor (if the HW
> * supports it) without needing to switch off the remote processor.
> + *
> + * Return: 0 on success, and an appropriate error value otherwise
> */
> int rproc_detach(struct rproc *rproc)
> {
> @@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
> * This function increments the remote processor's refcount, so always
> * use rproc_put() to decrement it back once rproc isn't needed anymore.
> *
> - * Returns the rproc handle on success, and NULL on failure.
> + * Return: rproc handle on success, and NULL on failure
> */
> #ifdef CONFIG_OF
> struct rproc *rproc_get_by_phandle(phandle phandle)
> @@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
> * This is called by the platform-specific rproc implementation, whenever
> * a new remote processor device is probed.
> *
> - * Returns 0 on success and an appropriate error code otherwise.
> - *
> * Note: this function initiates an asynchronous firmware loading
> * context, which will look for virtio devices supported by the rproc's
> * firmware.
> @@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
> * If found, those virtio devices will be created and added, so as a result
> * of registering this remote processor, additional virtio drivers might be
> * probed.
> + *
> + * Return: 0 on success and an appropriate error code otherwise
> */
> int rproc_add(struct rproc *rproc)
> {
> @@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
> * This function performs like rproc_add() but the registered rproc device will
> * automatically be removed on driver detach.
> *
> - * Returns: 0 on success, negative errno on failure
> + * Return: 0 on success, negative errno on failure
> */
> int devm_rproc_add(struct device *dev, struct rproc *rproc)
> {
> @@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
> * implementations should then call rproc_add() to complete
> * the registration of the remote processor.
> *
> - * On success the new rproc is returned, and on failure, NULL.
> - *
> * Note: _never_ directly deallocate @rproc, even if it was not registered
> * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
> + *
> + * Return: new rproc pointer on success, and NULL on failure
> */
> struct rproc *rproc_alloc(struct device *dev, const char *name,
> const struct rproc_ops *ops,
> @@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
> * of the outstanding reference created by rproc_alloc. To decrement that
> * one last refcount, one still needs to call rproc_free().
> *
> - * Returns 0 on success and -EINVAL if @rproc isn't valid.
> + * Return: 0 on success and -EINVAL if @rproc isn't valid
> */
> int rproc_del(struct rproc *rproc)
> {
> @@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
> * This function performs like rproc_alloc() but the acquired rproc device will
> * automatically be released on driver detach.
> *
> - * Returns: new rproc instance, or NULL on failure
> + * Return: new rproc instance, or NULL on failure
> */
> struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
> const struct rproc_ops *ops,
> @@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
> * rproc_get_by_child() - acquire rproc handle of @dev's ancestor
> * @dev: child device to find ancestor of
> *
> - * Returns the ancestor rproc instance, or NULL if not found.
> + * Return: the ancestor rproc instance, or NULL if not found
> */
> struct rproc *rproc_get_by_child(struct device *dev)
> {
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index 11423588965a..469c52e62faf 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -31,6 +31,8 @@
> * @fw: the ELF firmware image
> *
> * Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
> + *
> + * Return: 0 on success and -EINVAL upon any failure
> */
> int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
> * @rproc: the remote processor handle
> * @fw: the ELF firmware image
> *
> - * This function returns the entry point address of the ELF
> - * image.
> - *
> * Note that the boot address is not a configurable property of all remote
> * processors. Some will always boot at a specific hard-coded address.
> + *
> + * Return: entry point address of the ELF image
> + *
> */
> u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> * might be different: they might not have iommus, and would prefer to
> * directly allocate memory for every segment/resource. This is not yet
> * supported, though.
> + *
> + * Return: 0 on success and an appropriate error code otherwise
> */
> int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
> * This function finds the location of the loaded resource table. Don't
> * call this function if the table wasn't loaded yet - it's a bug if you do.
> *
> - * Returns the pointer to the resource table if it is found or NULL otherwise.
> + * Return: pointer to the resource table if it is found or NULL otherwise.
Here the '.' has been kept while it was remove for all of the above. I don't
know that the right guidelines are for this.
> * If the table wasn't loaded yet the result is unspecified.
> */
> struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 0cc617f76068..cf4d54e98e6a 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
> * when the remote processor signals that a specific virtqueue has pending
> * messages available.
> *
> - * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
> + * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
> * and otherwise returns IRQ_HANDLED.
Same
> */
> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
> @@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
> * This function registers a virtio device. This vdev's partent is
> * the rproc device.
> *
> - * Returns 0 on success or an appropriate error value otherwise.
> + * Return: 0 on success or an appropriate error value otherwise
> */
> int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> {
> @@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> * @data: must be null
> *
> * This function unregisters an existing virtio device.
> + *
> + * Return: 0
> */
> int rproc_remove_virtio_dev(struct device *dev, void *data)
> {
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8b795b544f75..42a1f30e33a7 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -243,7 +243,7 @@ struct fw_rsc_trace {
> * @da: device address
> * @align: the alignment between the consumer and producer parts of the vring
> * @num: num of buffers supported by this vring (must be power of two)
> - * @notifyid is a unique rproc-wide notify index for this vring. This notify
> + * @notifyid: a unique rproc-wide notify index for this vring. This notify
> * index is used when kicking a remote processor, to let it know that this
> * vring is triggered.
> * @pa: physical address
> @@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
> /**
> * struct fw_rsc_vdev - virtio device header
> * @id: virtio device id (as in virtio_ids.h)
> - * @notifyid is a unique rproc-wide notify index for this vdev. This notify
> + * @notifyid: a unique rproc-wide notify index for this vdev. This notify
> * index is used when kicking a remote processor, to let it know that the
> * status/features of this vdev have changes.
> - * @dfeatures specifies the virtio device features supported by the firmware
> - * @gfeatures is a place holder used by the host to write back the
> + * @dfeatures: specifies the virtio device features supported by the firmware
> + * @gfeatures: a place holder used by the host to write back the
> * negotiated features that are supported by both sides.
> - * @config_len is the size of the virtio config space of this vdev. The config
> + * @config_len: the size of the virtio config space of this vdev. The config
> * space lies in the resource table immediate after this vdev header.
> - * @status is a place holder where the host will indicate its virtio progress.
> - * @num_of_vrings indicates how many vrings are described in this vdev header
> + * @status: a place holder where the host will indicate its virtio progress.
> + * @num_of_vrings: indicates how many vrings are described in this vdev header
> * @reserved: reserved (must be zero)
> - * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
> + * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
> *
> * This resource is a virtio device header: it provides information about
> * the vdev, and is then used by the host and its peer remote processors
> @@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
> * to statically allocate a vdev upon registration of the rproc (dynamic vdev
> * allocation is not yet supported).
> *
> - * Note: unlike virtualization systems, the term 'host' here means
> - * the Linux side which is running remoteproc to control the remote
> - * processors. We use the name 'gfeatures' to comply with virtio's terms,
> - * though there isn't really any virtualized guest OS here: it's the host
> - * which is responsible for negotiating the final features.
> - * Yeah, it's a bit confusing.
> - *
> - * Note: immediately following this structure is the virtio config space for
> - * this vdev (which is specific to the vdev; for more info, read the virtio
> - * spec). the size of the config space is specified by @config_len.
> + * Note:
> + * 1. unlike virtualization systems, the term 'host' here means
> + * the Linux side which is running remoteproc to control the remote
> + * processors. We use the name 'gfeatures' to comply with virtio's terms,
> + * though there isn't really any virtualized guest OS here: it's the host
> + * which is responsible for negotiating the final features.
> + * Yeah, it's a bit confusing.
> + *
> + * 2. immediately following this structure is the virtio config space for
> + * this vdev (which is specific to the vdev; for more info, read the virtio
> + * spec). the size of the config space is specified by @config_len.
s/the/The
> */
> struct fw_rsc_vdev {
> u32 id;
> @@ -440,7 +441,7 @@ enum rproc_state {
> * enum rproc_crash_type - remote processor crash types
> * @RPROC_MMUFAULT: iommu fault
> * @RPROC_WATCHDOG: watchdog bite
> - * @RPROC_FATAL_ERROR fatal error
> + * @RPROC_FATAL_ERROR: fatal error
> *
> * Each element of the enum is used as an array index. So that, the value of
> * the elements should be always something sane.
> @@ -457,9 +458,9 @@ enum rproc_crash_type {
> * enum rproc_dump_mechanism - Coredump options for core
> * @RPROC_COREDUMP_DISABLED: Don't perform any dump
> * @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
> - recovery
> + * recovery
> * @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
> - recovery until all segments are read
> + * recovery until all segments are read
> */
> enum rproc_dump_mechanism {
> RPROC_COREDUMP_DISABLED,
> @@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
> * @priv: private data associated with the dump_segment
> * @dump: custom dump function to fill device memory segment associated
> * with coredump
> + * @offset: offset of the segment
> */
> struct rproc_dump_segment {
> struct list_head node;
> @@ -524,7 +526,9 @@ struct rproc_dump_segment {
> * @auto_boot: flag to indicate if remote processor should be auto-started
> * @dump_segments: list of segments in the firmware
> * @nb_vdev: number of vdev currently handled by rproc
> - * @char_dev: character device of the 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
> */
> struct rproc {
> @@ -613,10 +617,10 @@ struct rproc_vring {
> * struct rproc_vdev - remoteproc state for a supported virtio device
> * @refcount: reference counter for the vdev and vring allocations
> * @subdev: handle for registering the vdev as a rproc subdevice
> + * @dev: device struct used for reference count semantics
> * @id: virtio device id (as in virtio_ids.h)
> * @node: list node
> * @rproc: the rproc handle
> - * @vdev: the virio device
> * @vring: the vrings for this vdev
> * @rsc_offset: offset of the vdev's resource entry
> * @index: vdev position versus other vdev declared in resource table
With or without the above:
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
@ 2021-05-25 18:00 ` Mathieu Poirier
0 siblings, 0 replies; 14+ messages in thread
From: Mathieu Poirier @ 2021-05-25 18:00 UTC (permalink / raw)
To: Suman Anna
Cc: Bjorn Andersson, linux-remoteproc, linux-arm-kernel, linux-kernel
On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
> Fix all the kernel-doc warnings in various remoteproc core files.
> Some of them just needed a formatting cleanup change, while others
> needed the Return statement to be added, or documenting the missed
> structure elements.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
> drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
> drivers/remoteproc/remoteproc_virtio.c | 6 ++-
> include/linux/remoteproc.h | 50 ++++++++++++----------
> 4 files changed, 69 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 8c279039b6a3..6348aaa42bbb 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
> * translations on the internal remoteproc memory regions through a platform
> * implementation specific da_to_va ops, if present.
> *
> - * The function returns a valid kernel address on success or NULL on failure.
> - *
> * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
> * but only on kernel direct mapped RAM memory. Instead, we're just using
> * here the output of the DMA API for the carveouts, which should be more
> * correct.
> + *
> + * Return: a valid kernel address on success or NULL on failure
> */
> void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
> {
> @@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
> * use RSC_DEVMEM resource entries to map their required @da to the physical
> * address of their base CMA region (ouch, hacky!).
> *
> - * Returns 0 on success, or an appropriate error code otherwise
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
> * support dynamically allocating this address using the generic
> * DMA API (but currently there isn't a use case for that).
> *
> - * Returns 0 on success, or an appropriate error code otherwise
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_trace(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
> * tell us ranges of physical addresses the firmware is allowed to request,
> * and not allow firmwares to request access to physical addresses that
> * are outside those ranges.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
> int offset, int avail)
> @@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
> *
> * This function allocate specified memory entry @mem using
> * dma_alloc_coherent() as default allocator
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_alloc_carveout(struct rproc *rproc,
> struct rproc_mem_entry *mem)
> @@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
> *
> * This function releases specified memory entry @mem allocated via
> * rproc_alloc_carveout() function by @rproc.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_release_carveout(struct rproc *rproc,
> struct rproc_mem_entry *mem)
> @@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
> * (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
> * needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
> * pressure is important; it may have a substantial impact on performance.
> + *
> + * Return: 0 on success, or an appropriate error code otherwise
> */
> static int rproc_handle_carveout(struct rproc *rproc,
> void *ptr, int offset, int avail)
> @@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
> *
> * This function allocates a rproc_mem_entry struct and fill it with parameters
> * provided by client.
> + *
> + * Return: a valid pointer on success, or NULL on failure
> */
> __printf(8, 9)
> struct rproc_mem_entry *
> @@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
> *
> * This function allocates a rproc_mem_entry struct and fill it with parameters
> * provided by client.
> + *
> + * Return: a valid pointer on success, or NULL on failure
> */
> __printf(5, 6)
> struct rproc_mem_entry *
> @@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
> * remoteproc functional again.
> *
> * This function can sleep, so it cannot be called from atomic context.
> + *
> + * Return: 0 on success or a negative value upon failure
> */
> int rproc_trigger_recovery(struct rproc *rproc)
> {
> @@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
> * If the remote processor is already powered on, this function immediately
> * returns (successfully).
> *
> - * Returns 0 on success, and an appropriate error value otherwise.
> + * Return: 0 on success, and an appropriate error value otherwise
> */
> int rproc_boot(struct rproc *rproc)
> {
> @@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
> * no longer available. From there it should be possible to remove the
> * platform driver and even power cycle the application processor (if the HW
> * supports it) without needing to switch off the remote processor.
> + *
> + * Return: 0 on success, and an appropriate error value otherwise
> */
> int rproc_detach(struct rproc *rproc)
> {
> @@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
> * This function increments the remote processor's refcount, so always
> * use rproc_put() to decrement it back once rproc isn't needed anymore.
> *
> - * Returns the rproc handle on success, and NULL on failure.
> + * Return: rproc handle on success, and NULL on failure
> */
> #ifdef CONFIG_OF
> struct rproc *rproc_get_by_phandle(phandle phandle)
> @@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
> * This is called by the platform-specific rproc implementation, whenever
> * a new remote processor device is probed.
> *
> - * Returns 0 on success and an appropriate error code otherwise.
> - *
> * Note: this function initiates an asynchronous firmware loading
> * context, which will look for virtio devices supported by the rproc's
> * firmware.
> @@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
> * If found, those virtio devices will be created and added, so as a result
> * of registering this remote processor, additional virtio drivers might be
> * probed.
> + *
> + * Return: 0 on success and an appropriate error code otherwise
> */
> int rproc_add(struct rproc *rproc)
> {
> @@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
> * This function performs like rproc_add() but the registered rproc device will
> * automatically be removed on driver detach.
> *
> - * Returns: 0 on success, negative errno on failure
> + * Return: 0 on success, negative errno on failure
> */
> int devm_rproc_add(struct device *dev, struct rproc *rproc)
> {
> @@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
> * implementations should then call rproc_add() to complete
> * the registration of the remote processor.
> *
> - * On success the new rproc is returned, and on failure, NULL.
> - *
> * Note: _never_ directly deallocate @rproc, even if it was not registered
> * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
> + *
> + * Return: new rproc pointer on success, and NULL on failure
> */
> struct rproc *rproc_alloc(struct device *dev, const char *name,
> const struct rproc_ops *ops,
> @@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
> * of the outstanding reference created by rproc_alloc. To decrement that
> * one last refcount, one still needs to call rproc_free().
> *
> - * Returns 0 on success and -EINVAL if @rproc isn't valid.
> + * Return: 0 on success and -EINVAL if @rproc isn't valid
> */
> int rproc_del(struct rproc *rproc)
> {
> @@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
> * This function performs like rproc_alloc() but the acquired rproc device will
> * automatically be released on driver detach.
> *
> - * Returns: new rproc instance, or NULL on failure
> + * Return: new rproc instance, or NULL on failure
> */
> struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
> const struct rproc_ops *ops,
> @@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
> * rproc_get_by_child() - acquire rproc handle of @dev's ancestor
> * @dev: child device to find ancestor of
> *
> - * Returns the ancestor rproc instance, or NULL if not found.
> + * Return: the ancestor rproc instance, or NULL if not found
> */
> struct rproc *rproc_get_by_child(struct device *dev)
> {
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index 11423588965a..469c52e62faf 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -31,6 +31,8 @@
> * @fw: the ELF firmware image
> *
> * Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
> + *
> + * Return: 0 on success and -EINVAL upon any failure
> */
> int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
> * @rproc: the remote processor handle
> * @fw: the ELF firmware image
> *
> - * This function returns the entry point address of the ELF
> - * image.
> - *
> * Note that the boot address is not a configurable property of all remote
> * processors. Some will always boot at a specific hard-coded address.
> + *
> + * Return: entry point address of the ELF image
> + *
> */
> u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> * might be different: they might not have iommus, and would prefer to
> * directly allocate memory for every segment/resource. This is not yet
> * supported, though.
> + *
> + * Return: 0 on success and an appropriate error code otherwise
> */
> int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> {
> @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
> * This function finds the location of the loaded resource table. Don't
> * call this function if the table wasn't loaded yet - it's a bug if you do.
> *
> - * Returns the pointer to the resource table if it is found or NULL otherwise.
> + * Return: pointer to the resource table if it is found or NULL otherwise.
Here the '.' has been kept while it was remove for all of the above. I don't
know that the right guidelines are for this.
> * If the table wasn't loaded yet the result is unspecified.
> */
> struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 0cc617f76068..cf4d54e98e6a 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
> * when the remote processor signals that a specific virtqueue has pending
> * messages available.
> *
> - * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
> + * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
> * and otherwise returns IRQ_HANDLED.
Same
> */
> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
> @@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
> * This function registers a virtio device. This vdev's partent is
> * the rproc device.
> *
> - * Returns 0 on success or an appropriate error value otherwise.
> + * Return: 0 on success or an appropriate error value otherwise
> */
> int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> {
> @@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> * @data: must be null
> *
> * This function unregisters an existing virtio device.
> + *
> + * Return: 0
> */
> int rproc_remove_virtio_dev(struct device *dev, void *data)
> {
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8b795b544f75..42a1f30e33a7 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -243,7 +243,7 @@ struct fw_rsc_trace {
> * @da: device address
> * @align: the alignment between the consumer and producer parts of the vring
> * @num: num of buffers supported by this vring (must be power of two)
> - * @notifyid is a unique rproc-wide notify index for this vring. This notify
> + * @notifyid: a unique rproc-wide notify index for this vring. This notify
> * index is used when kicking a remote processor, to let it know that this
> * vring is triggered.
> * @pa: physical address
> @@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
> /**
> * struct fw_rsc_vdev - virtio device header
> * @id: virtio device id (as in virtio_ids.h)
> - * @notifyid is a unique rproc-wide notify index for this vdev. This notify
> + * @notifyid: a unique rproc-wide notify index for this vdev. This notify
> * index is used when kicking a remote processor, to let it know that the
> * status/features of this vdev have changes.
> - * @dfeatures specifies the virtio device features supported by the firmware
> - * @gfeatures is a place holder used by the host to write back the
> + * @dfeatures: specifies the virtio device features supported by the firmware
> + * @gfeatures: a place holder used by the host to write back the
> * negotiated features that are supported by both sides.
> - * @config_len is the size of the virtio config space of this vdev. The config
> + * @config_len: the size of the virtio config space of this vdev. The config
> * space lies in the resource table immediate after this vdev header.
> - * @status is a place holder where the host will indicate its virtio progress.
> - * @num_of_vrings indicates how many vrings are described in this vdev header
> + * @status: a place holder where the host will indicate its virtio progress.
> + * @num_of_vrings: indicates how many vrings are described in this vdev header
> * @reserved: reserved (must be zero)
> - * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
> + * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
> *
> * This resource is a virtio device header: it provides information about
> * the vdev, and is then used by the host and its peer remote processors
> @@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
> * to statically allocate a vdev upon registration of the rproc (dynamic vdev
> * allocation is not yet supported).
> *
> - * Note: unlike virtualization systems, the term 'host' here means
> - * the Linux side which is running remoteproc to control the remote
> - * processors. We use the name 'gfeatures' to comply with virtio's terms,
> - * though there isn't really any virtualized guest OS here: it's the host
> - * which is responsible for negotiating the final features.
> - * Yeah, it's a bit confusing.
> - *
> - * Note: immediately following this structure is the virtio config space for
> - * this vdev (which is specific to the vdev; for more info, read the virtio
> - * spec). the size of the config space is specified by @config_len.
> + * Note:
> + * 1. unlike virtualization systems, the term 'host' here means
> + * the Linux side which is running remoteproc to control the remote
> + * processors. We use the name 'gfeatures' to comply with virtio's terms,
> + * though there isn't really any virtualized guest OS here: it's the host
> + * which is responsible for negotiating the final features.
> + * Yeah, it's a bit confusing.
> + *
> + * 2. immediately following this structure is the virtio config space for
> + * this vdev (which is specific to the vdev; for more info, read the virtio
> + * spec). the size of the config space is specified by @config_len.
s/the/The
> */
> struct fw_rsc_vdev {
> u32 id;
> @@ -440,7 +441,7 @@ enum rproc_state {
> * enum rproc_crash_type - remote processor crash types
> * @RPROC_MMUFAULT: iommu fault
> * @RPROC_WATCHDOG: watchdog bite
> - * @RPROC_FATAL_ERROR fatal error
> + * @RPROC_FATAL_ERROR: fatal error
> *
> * Each element of the enum is used as an array index. So that, the value of
> * the elements should be always something sane.
> @@ -457,9 +458,9 @@ enum rproc_crash_type {
> * enum rproc_dump_mechanism - Coredump options for core
> * @RPROC_COREDUMP_DISABLED: Don't perform any dump
> * @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
> - recovery
> + * recovery
> * @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
> - recovery until all segments are read
> + * recovery until all segments are read
> */
> enum rproc_dump_mechanism {
> RPROC_COREDUMP_DISABLED,
> @@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
> * @priv: private data associated with the dump_segment
> * @dump: custom dump function to fill device memory segment associated
> * with coredump
> + * @offset: offset of the segment
> */
> struct rproc_dump_segment {
> struct list_head node;
> @@ -524,7 +526,9 @@ struct rproc_dump_segment {
> * @auto_boot: flag to indicate if remote processor should be auto-started
> * @dump_segments: list of segments in the firmware
> * @nb_vdev: number of vdev currently handled by rproc
> - * @char_dev: character device of the 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
> */
> struct rproc {
> @@ -613,10 +617,10 @@ struct rproc_vring {
> * struct rproc_vdev - remoteproc state for a supported virtio device
> * @refcount: reference counter for the vdev and vring allocations
> * @subdev: handle for registering the vdev as a rproc subdevice
> + * @dev: device struct used for reference count semantics
> * @id: virtio device id (as in virtio_ids.h)
> * @node: list node
> * @rproc: the rproc handle
> - * @vdev: the virio device
> * @vring: the vrings for this vdev
> * @rsc_offset: offset of the vdev's resource entry
> * @index: vdev position versus other vdev declared in resource table
With or without the above:
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> --
> 2.30.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
2021-05-25 18:00 ` Mathieu Poirier
@ 2021-05-25 18:07 ` Suman Anna
-1 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-25 18:07 UTC (permalink / raw)
To: Mathieu Poirier, Bjorn Andersson
Cc: linux-remoteproc, linux-arm-kernel, linux-kernel
On 5/25/21 1:00 PM, Mathieu Poirier wrote:
> On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
>> Fix all the kernel-doc warnings in various remoteproc core files.
>> Some of them just needed a formatting cleanup change, while others
>> needed the Return statement to be added, or documenting the missed
>> structure elements.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
>> drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
>> drivers/remoteproc/remoteproc_virtio.c | 6 ++-
>> include/linux/remoteproc.h | 50 ++++++++++++----------
>> 4 files changed, 69 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index 8c279039b6a3..6348aaa42bbb 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
>> * translations on the internal remoteproc memory regions through a platform
>> * implementation specific da_to_va ops, if present.
>> *
>> - * The function returns a valid kernel address on success or NULL on failure.
>> - *
>> * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
>> * but only on kernel direct mapped RAM memory. Instead, we're just using
>> * here the output of the DMA API for the carveouts, which should be more
>> * correct.
>> + *
>> + * Return: a valid kernel address on success or NULL on failure
>> */
>> void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
>> {
>> @@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
>> * use RSC_DEVMEM resource entries to map their required @da to the physical
>> * address of their base CMA region (ouch, hacky!).
>> *
>> - * Returns 0 on success, or an appropriate error code otherwise
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
>> * support dynamically allocating this address using the generic
>> * DMA API (but currently there isn't a use case for that).
>> *
>> - * Returns 0 on success, or an appropriate error code otherwise
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_trace(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
>> * tell us ranges of physical addresses the firmware is allowed to request,
>> * and not allow firmwares to request access to physical addresses that
>> * are outside those ranges.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
>> *
>> * This function allocate specified memory entry @mem using
>> * dma_alloc_coherent() as default allocator
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_alloc_carveout(struct rproc *rproc,
>> struct rproc_mem_entry *mem)
>> @@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
>> *
>> * This function releases specified memory entry @mem allocated via
>> * rproc_alloc_carveout() function by @rproc.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_release_carveout(struct rproc *rproc,
>> struct rproc_mem_entry *mem)
>> @@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
>> * (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
>> * needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
>> * pressure is important; it may have a substantial impact on performance.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_carveout(struct rproc *rproc,
>> void *ptr, int offset, int avail)
>> @@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
>> *
>> * This function allocates a rproc_mem_entry struct and fill it with parameters
>> * provided by client.
>> + *
>> + * Return: a valid pointer on success, or NULL on failure
>> */
>> __printf(8, 9)
>> struct rproc_mem_entry *
>> @@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
>> *
>> * This function allocates a rproc_mem_entry struct and fill it with parameters
>> * provided by client.
>> + *
>> + * Return: a valid pointer on success, or NULL on failure
>> */
>> __printf(5, 6)
>> struct rproc_mem_entry *
>> @@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
>> * remoteproc functional again.
>> *
>> * This function can sleep, so it cannot be called from atomic context.
>> + *
>> + * Return: 0 on success or a negative value upon failure
>> */
>> int rproc_trigger_recovery(struct rproc *rproc)
>> {
>> @@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
>> * If the remote processor is already powered on, this function immediately
>> * returns (successfully).
>> *
>> - * Returns 0 on success, and an appropriate error value otherwise.
>> + * Return: 0 on success, and an appropriate error value otherwise
>> */
>> int rproc_boot(struct rproc *rproc)
>> {
>> @@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
>> * no longer available. From there it should be possible to remove the
>> * platform driver and even power cycle the application processor (if the HW
>> * supports it) without needing to switch off the remote processor.
>> + *
>> + * Return: 0 on success, and an appropriate error value otherwise
>> */
>> int rproc_detach(struct rproc *rproc)
>> {
>> @@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
>> * This function increments the remote processor's refcount, so always
>> * use rproc_put() to decrement it back once rproc isn't needed anymore.
>> *
>> - * Returns the rproc handle on success, and NULL on failure.
>> + * Return: rproc handle on success, and NULL on failure
>> */
>> #ifdef CONFIG_OF
>> struct rproc *rproc_get_by_phandle(phandle phandle)
>> @@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
>> * This is called by the platform-specific rproc implementation, whenever
>> * a new remote processor device is probed.
>> *
>> - * Returns 0 on success and an appropriate error code otherwise.
>> - *
>> * Note: this function initiates an asynchronous firmware loading
>> * context, which will look for virtio devices supported by the rproc's
>> * firmware.
>> @@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
>> * If found, those virtio devices will be created and added, so as a result
>> * of registering this remote processor, additional virtio drivers might be
>> * probed.
>> + *
>> + * Return: 0 on success and an appropriate error code otherwise
>> */
>> int rproc_add(struct rproc *rproc)
>> {
>> @@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
>> * This function performs like rproc_add() but the registered rproc device will
>> * automatically be removed on driver detach.
>> *
>> - * Returns: 0 on success, negative errno on failure
>> + * Return: 0 on success, negative errno on failure
>> */
>> int devm_rproc_add(struct device *dev, struct rproc *rproc)
>> {
>> @@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
>> * implementations should then call rproc_add() to complete
>> * the registration of the remote processor.
>> *
>> - * On success the new rproc is returned, and on failure, NULL.
>> - *
>> * Note: _never_ directly deallocate @rproc, even if it was not registered
>> * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
>> + *
>> + * Return: new rproc pointer on success, and NULL on failure
>> */
>> struct rproc *rproc_alloc(struct device *dev, const char *name,
>> const struct rproc_ops *ops,
>> @@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
>> * of the outstanding reference created by rproc_alloc. To decrement that
>> * one last refcount, one still needs to call rproc_free().
>> *
>> - * Returns 0 on success and -EINVAL if @rproc isn't valid.
>> + * Return: 0 on success and -EINVAL if @rproc isn't valid
>> */
>> int rproc_del(struct rproc *rproc)
>> {
>> @@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
>> * This function performs like rproc_alloc() but the acquired rproc device will
>> * automatically be released on driver detach.
>> *
>> - * Returns: new rproc instance, or NULL on failure
>> + * Return: new rproc instance, or NULL on failure
>> */
>> struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
>> const struct rproc_ops *ops,
>> @@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
>> * rproc_get_by_child() - acquire rproc handle of @dev's ancestor
>> * @dev: child device to find ancestor of
>> *
>> - * Returns the ancestor rproc instance, or NULL if not found.
>> + * Return: the ancestor rproc instance, or NULL if not found
>> */
>> struct rproc *rproc_get_by_child(struct device *dev)
>> {
>> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
>> index 11423588965a..469c52e62faf 100644
>> --- a/drivers/remoteproc/remoteproc_elf_loader.c
>> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
>> @@ -31,6 +31,8 @@
>> * @fw: the ELF firmware image
>> *
>> * Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
>> + *
>> + * Return: 0 on success and -EINVAL upon any failure
>> */
>> int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
>> * @rproc: the remote processor handle
>> * @fw: the ELF firmware image
>> *
>> - * This function returns the entry point address of the ELF
>> - * image.
>> - *
>> * Note that the boot address is not a configurable property of all remote
>> * processors. Some will always boot at a specific hard-coded address.
>> + *
>> + * Return: entry point address of the ELF image
>> + *
>> */
>> u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
>> * might be different: they might not have iommus, and would prefer to
>> * directly allocate memory for every segment/resource. This is not yet
>> * supported, though.
>> + *
>> + * Return: 0 on success and an appropriate error code otherwise
>> */
>> int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
>> * This function finds the location of the loaded resource table. Don't
>> * call this function if the table wasn't loaded yet - it's a bug if you do.
>> *
>> - * Returns the pointer to the resource table if it is found or NULL otherwise.
>> + * Return: pointer to the resource table if it is found or NULL otherwise.
>
> Here the '.' has been kept while it was remove for all of the above. I don't
> know that the right guidelines are for this.
>
>> * If the table wasn't loaded yet the result is unspecified.
>> */
>> struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 0cc617f76068..cf4d54e98e6a 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
>> * when the remote processor signals that a specific virtqueue has pending
>> * messages available.
>> *
>> - * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
>> + * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
>> * and otherwise returns IRQ_HANDLED.
>
> Same
>
>> */
>> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
>> @@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
>> * This function registers a virtio device. This vdev's partent is
>> * the rproc device.
>> *
>> - * Returns 0 on success or an appropriate error value otherwise.
>> + * Return: 0 on success or an appropriate error value otherwise
>> */
>> int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> {
>> @@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> * @data: must be null
>> *
>> * This function unregisters an existing virtio device.
>> + *
>> + * Return: 0
>> */
>> int rproc_remove_virtio_dev(struct device *dev, void *data)
>> {
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 8b795b544f75..42a1f30e33a7 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -243,7 +243,7 @@ struct fw_rsc_trace {
>> * @da: device address
>> * @align: the alignment between the consumer and producer parts of the vring
>> * @num: num of buffers supported by this vring (must be power of two)
>> - * @notifyid is a unique rproc-wide notify index for this vring. This notify
>> + * @notifyid: a unique rproc-wide notify index for this vring. This notify
>> * index is used when kicking a remote processor, to let it know that this
>> * vring is triggered.
>> * @pa: physical address
>> @@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
>> /**
>> * struct fw_rsc_vdev - virtio device header
>> * @id: virtio device id (as in virtio_ids.h)
>> - * @notifyid is a unique rproc-wide notify index for this vdev. This notify
>> + * @notifyid: a unique rproc-wide notify index for this vdev. This notify
>> * index is used when kicking a remote processor, to let it know that the
>> * status/features of this vdev have changes.
>> - * @dfeatures specifies the virtio device features supported by the firmware
>> - * @gfeatures is a place holder used by the host to write back the
>> + * @dfeatures: specifies the virtio device features supported by the firmware
>> + * @gfeatures: a place holder used by the host to write back the
>> * negotiated features that are supported by both sides.
>> - * @config_len is the size of the virtio config space of this vdev. The config
>> + * @config_len: the size of the virtio config space of this vdev. The config
>> * space lies in the resource table immediate after this vdev header.
>> - * @status is a place holder where the host will indicate its virtio progress.
>> - * @num_of_vrings indicates how many vrings are described in this vdev header
>> + * @status: a place holder where the host will indicate its virtio progress.
>> + * @num_of_vrings: indicates how many vrings are described in this vdev header
>> * @reserved: reserved (must be zero)
>> - * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
>> + * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
>> *
>> * This resource is a virtio device header: it provides information about
>> * the vdev, and is then used by the host and its peer remote processors
>> @@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
>> * to statically allocate a vdev upon registration of the rproc (dynamic vdev
>> * allocation is not yet supported).
>> *
>> - * Note: unlike virtualization systems, the term 'host' here means
>> - * the Linux side which is running remoteproc to control the remote
>> - * processors. We use the name 'gfeatures' to comply with virtio's terms,
>> - * though there isn't really any virtualized guest OS here: it's the host
>> - * which is responsible for negotiating the final features.
>> - * Yeah, it's a bit confusing.
>> - *
>> - * Note: immediately following this structure is the virtio config space for
>> - * this vdev (which is specific to the vdev; for more info, read the virtio
>> - * spec). the size of the config space is specified by @config_len.
>> + * Note:
>> + * 1. unlike virtualization systems, the term 'host' here means
>> + * the Linux side which is running remoteproc to control the remote
>> + * processors. We use the name 'gfeatures' to comply with virtio's terms,
>> + * though there isn't really any virtualized guest OS here: it's the host
>> + * which is responsible for negotiating the final features.
>> + * Yeah, it's a bit confusing.
>> + *
>> + * 2. immediately following this structure is the virtio config space for
>> + * this vdev (which is specific to the vdev; for more info, read the virtio
>> + * spec). the size of the config space is specified by @config_len.
>
> s/the/The
>
>> */
>> struct fw_rsc_vdev {
>> u32 id;
>> @@ -440,7 +441,7 @@ enum rproc_state {
>> * enum rproc_crash_type - remote processor crash types
>> * @RPROC_MMUFAULT: iommu fault
>> * @RPROC_WATCHDOG: watchdog bite
>> - * @RPROC_FATAL_ERROR fatal error
>> + * @RPROC_FATAL_ERROR: fatal error
>> *
>> * Each element of the enum is used as an array index. So that, the value of
>> * the elements should be always something sane.
>> @@ -457,9 +458,9 @@ enum rproc_crash_type {
>> * enum rproc_dump_mechanism - Coredump options for core
>> * @RPROC_COREDUMP_DISABLED: Don't perform any dump
>> * @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
>> - recovery
>> + * recovery
>> * @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
>> - recovery until all segments are read
>> + * recovery until all segments are read
>> */
>> enum rproc_dump_mechanism {
>> RPROC_COREDUMP_DISABLED,
>> @@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
>> * @priv: private data associated with the dump_segment
>> * @dump: custom dump function to fill device memory segment associated
>> * with coredump
>> + * @offset: offset of the segment
>> */
>> struct rproc_dump_segment {
>> struct list_head node;
>> @@ -524,7 +526,9 @@ struct rproc_dump_segment {
>> * @auto_boot: flag to indicate if remote processor should be auto-started
>> * @dump_segments: list of segments in the firmware
>> * @nb_vdev: number of vdev currently handled by rproc
>> - * @char_dev: character device of the 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
>> */
>> struct rproc {
>> @@ -613,10 +617,10 @@ struct rproc_vring {
>> * struct rproc_vdev - remoteproc state for a supported virtio device
>> * @refcount: reference counter for the vdev and vring allocations
>> * @subdev: handle for registering the vdev as a rproc subdevice
>> + * @dev: device struct used for reference count semantics
>> * @id: virtio device id (as in virtio_ids.h)
>> * @node: list node
>> * @rproc: the rproc handle
>> - * @vdev: the virio device
>> * @vring: the vrings for this vdev
>> * @rsc_offset: offset of the vdev's resource entry
>> * @index: vdev position versus other vdev declared in resource table
>
> With or without the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu.
Bjorn,
Any preferences, let me know if you want me to quickly respin this patch, or if
you want to fixup locally while applying?
regards
Suman
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
@ 2021-05-25 18:07 ` Suman Anna
0 siblings, 0 replies; 14+ messages in thread
From: Suman Anna @ 2021-05-25 18:07 UTC (permalink / raw)
To: Mathieu Poirier, Bjorn Andersson
Cc: linux-remoteproc, linux-arm-kernel, linux-kernel
On 5/25/21 1:00 PM, Mathieu Poirier wrote:
> On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
>> Fix all the kernel-doc warnings in various remoteproc core files.
>> Some of them just needed a formatting cleanup change, while others
>> needed the Return statement to be added, or documenting the missed
>> structure elements.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> drivers/remoteproc/remoteproc_core.c | 44 +++++++++++++------
>> drivers/remoteproc/remoteproc_elf_loader.c | 12 ++++--
>> drivers/remoteproc/remoteproc_virtio.c | 6 ++-
>> include/linux/remoteproc.h | 50 ++++++++++++----------
>> 4 files changed, 69 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index 8c279039b6a3..6348aaa42bbb 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -183,12 +183,12 @@ EXPORT_SYMBOL(rproc_va_to_pa);
>> * translations on the internal remoteproc memory regions through a platform
>> * implementation specific da_to_va ops, if present.
>> *
>> - * The function returns a valid kernel address on success or NULL on failure.
>> - *
>> * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
>> * but only on kernel direct mapped RAM memory. Instead, we're just using
>> * here the output of the DMA API for the carveouts, which should be more
>> * correct.
>> + *
>> + * Return: a valid kernel address on success or NULL on failure
>> */
>> void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
>> {
>> @@ -509,7 +509,7 @@ static int copy_dma_range_map(struct device *to, struct device *from)
>> * use RSC_DEVMEM resource entries to map their required @da to the physical
>> * address of their base CMA region (ouch, hacky!).
>> *
>> - * Returns 0 on success, or an appropriate error code otherwise
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -644,7 +644,7 @@ void rproc_vdev_release(struct kref *ref)
>> * support dynamically allocating this address using the generic
>> * DMA API (but currently there isn't a use case for that).
>> *
>> - * Returns 0 on success, or an appropriate error code otherwise
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_trace(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -721,6 +721,8 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
>> * tell us ranges of physical addresses the firmware is allowed to request,
>> * and not allow firmwares to request access to physical addresses that
>> * are outside those ranges.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
>> int offset, int avail)
>> @@ -783,6 +785,8 @@ static int rproc_handle_devmem(struct rproc *rproc, void *ptr,
>> *
>> * This function allocate specified memory entry @mem using
>> * dma_alloc_coherent() as default allocator
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_alloc_carveout(struct rproc *rproc,
>> struct rproc_mem_entry *mem)
>> @@ -889,6 +893,8 @@ static int rproc_alloc_carveout(struct rproc *rproc,
>> *
>> * This function releases specified memory entry @mem allocated via
>> * rproc_alloc_carveout() function by @rproc.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_release_carveout(struct rproc *rproc,
>> struct rproc_mem_entry *mem)
>> @@ -918,6 +924,8 @@ static int rproc_release_carveout(struct rproc *rproc,
>> * (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
>> * needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
>> * pressure is important; it may have a substantial impact on performance.
>> + *
>> + * Return: 0 on success, or an appropriate error code otherwise
>> */
>> static int rproc_handle_carveout(struct rproc *rproc,
>> void *ptr, int offset, int avail)
>> @@ -1006,6 +1014,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
>> *
>> * This function allocates a rproc_mem_entry struct and fill it with parameters
>> * provided by client.
>> + *
>> + * Return: a valid pointer on success, or NULL on failure
>> */
>> __printf(8, 9)
>> struct rproc_mem_entry *
>> @@ -1050,6 +1060,8 @@ EXPORT_SYMBOL(rproc_mem_entry_init);
>> *
>> * This function allocates a rproc_mem_entry struct and fill it with parameters
>> * provided by client.
>> + *
>> + * Return: a valid pointer on success, or NULL on failure
>> */
>> __printf(5, 6)
>> struct rproc_mem_entry *
>> @@ -1881,6 +1893,8 @@ static int __rproc_detach(struct rproc *rproc)
>> * remoteproc functional again.
>> *
>> * This function can sleep, so it cannot be called from atomic context.
>> + *
>> + * Return: 0 on success or a negative value upon failure
>> */
>> int rproc_trigger_recovery(struct rproc *rproc)
>> {
>> @@ -1965,7 +1979,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
>> * If the remote processor is already powered on, this function immediately
>> * returns (successfully).
>> *
>> - * Returns 0 on success, and an appropriate error value otherwise.
>> + * Return: 0 on success, and an appropriate error value otherwise
>> */
>> int rproc_boot(struct rproc *rproc)
>> {
>> @@ -2100,6 +2114,8 @@ EXPORT_SYMBOL(rproc_shutdown);
>> * no longer available. From there it should be possible to remove the
>> * platform driver and even power cycle the application processor (if the HW
>> * supports it) without needing to switch off the remote processor.
>> + *
>> + * Return: 0 on success, and an appropriate error value otherwise
>> */
>> int rproc_detach(struct rproc *rproc)
>> {
>> @@ -2152,7 +2168,7 @@ EXPORT_SYMBOL(rproc_detach);
>> * This function increments the remote processor's refcount, so always
>> * use rproc_put() to decrement it back once rproc isn't needed anymore.
>> *
>> - * Returns the rproc handle on success, and NULL on failure.
>> + * Return: rproc handle on success, and NULL on failure
>> */
>> #ifdef CONFIG_OF
>> struct rproc *rproc_get_by_phandle(phandle phandle)
>> @@ -2302,8 +2318,6 @@ static int rproc_validate(struct rproc *rproc)
>> * This is called by the platform-specific rproc implementation, whenever
>> * a new remote processor device is probed.
>> *
>> - * Returns 0 on success and an appropriate error code otherwise.
>> - *
>> * Note: this function initiates an asynchronous firmware loading
>> * context, which will look for virtio devices supported by the rproc's
>> * firmware.
>> @@ -2311,6 +2325,8 @@ static int rproc_validate(struct rproc *rproc)
>> * If found, those virtio devices will be created and added, so as a result
>> * of registering this remote processor, additional virtio drivers might be
>> * probed.
>> + *
>> + * Return: 0 on success and an appropriate error code otherwise
>> */
>> int rproc_add(struct rproc *rproc)
>> {
>> @@ -2364,7 +2380,7 @@ static void devm_rproc_remove(void *rproc)
>> * This function performs like rproc_add() but the registered rproc device will
>> * automatically be removed on driver detach.
>> *
>> - * Returns: 0 on success, negative errno on failure
>> + * Return: 0 on success, negative errno on failure
>> */
>> int devm_rproc_add(struct device *dev, struct rproc *rproc)
>> {
>> @@ -2472,10 +2488,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
>> * implementations should then call rproc_add() to complete
>> * the registration of the remote processor.
>> *
>> - * On success the new rproc is returned, and on failure, NULL.
>> - *
>> * Note: _never_ directly deallocate @rproc, even if it was not registered
>> * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free().
>> + *
>> + * Return: new rproc pointer on success, and NULL on failure
>> */
>> struct rproc *rproc_alloc(struct device *dev, const char *name,
>> const struct rproc_ops *ops,
>> @@ -2588,7 +2604,7 @@ EXPORT_SYMBOL(rproc_put);
>> * of the outstanding reference created by rproc_alloc. To decrement that
>> * one last refcount, one still needs to call rproc_free().
>> *
>> - * Returns 0 on success and -EINVAL if @rproc isn't valid.
>> + * Return: 0 on success and -EINVAL if @rproc isn't valid
>> */
>> int rproc_del(struct rproc *rproc)
>> {
>> @@ -2635,7 +2651,7 @@ static void devm_rproc_free(struct device *dev, void *res)
>> * This function performs like rproc_alloc() but the acquired rproc device will
>> * automatically be released on driver detach.
>> *
>> - * Returns: new rproc instance, or NULL on failure
>> + * Return: new rproc instance, or NULL on failure
>> */
>> struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
>> const struct rproc_ops *ops,
>> @@ -2687,7 +2703,7 @@ EXPORT_SYMBOL(rproc_remove_subdev);
>> * rproc_get_by_child() - acquire rproc handle of @dev's ancestor
>> * @dev: child device to find ancestor of
>> *
>> - * Returns the ancestor rproc instance, or NULL if not found.
>> + * Return: the ancestor rproc instance, or NULL if not found
>> */
>> struct rproc *rproc_get_by_child(struct device *dev)
>> {
>> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
>> index 11423588965a..469c52e62faf 100644
>> --- a/drivers/remoteproc/remoteproc_elf_loader.c
>> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
>> @@ -31,6 +31,8 @@
>> * @fw: the ELF firmware image
>> *
>> * Make sure this fw image is sane (ie a correct ELF32/ELF64 file).
>> + *
>> + * Return: 0 on success and -EINVAL upon any failure
>> */
>> int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -117,11 +119,11 @@ EXPORT_SYMBOL(rproc_elf_sanity_check);
>> * @rproc: the remote processor handle
>> * @fw: the ELF firmware image
>> *
>> - * This function returns the entry point address of the ELF
>> - * image.
>> - *
>> * Note that the boot address is not a configurable property of all remote
>> * processors. Some will always boot at a specific hard-coded address.
>> + *
>> + * Return: entry point address of the ELF image
>> + *
>> */
>> u64 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -152,6 +154,8 @@ EXPORT_SYMBOL(rproc_elf_get_boot_addr);
>> * might be different: they might not have iommus, and would prefer to
>> * directly allocate memory for every segment/resource. This is not yet
>> * supported, though.
>> + *
>> + * Return: 0 on success and an appropriate error code otherwise
>> */
>> int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>> {
>> @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
>> * This function finds the location of the loaded resource table. Don't
>> * call this function if the table wasn't loaded yet - it's a bug if you do.
>> *
>> - * Returns the pointer to the resource table if it is found or NULL otherwise.
>> + * Return: pointer to the resource table if it is found or NULL otherwise.
>
> Here the '.' has been kept while it was remove for all of the above. I don't
> know that the right guidelines are for this.
>
>> * If the table wasn't loaded yet the result is unspecified.
>> */
>> struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 0cc617f76068..cf4d54e98e6a 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -45,7 +45,7 @@ static bool rproc_virtio_notify(struct virtqueue *vq)
>> * when the remote processor signals that a specific virtqueue has pending
>> * messages available.
>> *
>> - * Returns IRQ_NONE if no message was found in the @notifyid virtqueue,
>> + * Return: IRQ_NONE if no message was found in the @notifyid virtqueue,
>> * and otherwise returns IRQ_HANDLED.
>
> Same
>
>> */
>> irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
>> @@ -325,7 +325,7 @@ static void rproc_virtio_dev_release(struct device *dev)
>> * This function registers a virtio device. This vdev's partent is
>> * the rproc device.
>> *
>> - * Returns 0 on success or an appropriate error value otherwise.
>> + * Return: 0 on success or an appropriate error value otherwise
>> */
>> int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> {
>> @@ -432,6 +432,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> * @data: must be null
>> *
>> * This function unregisters an existing virtio device.
>> + *
>> + * Return: 0
>> */
>> int rproc_remove_virtio_dev(struct device *dev, void *data)
>> {
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 8b795b544f75..42a1f30e33a7 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -243,7 +243,7 @@ struct fw_rsc_trace {
>> * @da: device address
>> * @align: the alignment between the consumer and producer parts of the vring
>> * @num: num of buffers supported by this vring (must be power of two)
>> - * @notifyid is a unique rproc-wide notify index for this vring. This notify
>> + * @notifyid: a unique rproc-wide notify index for this vring. This notify
>> * index is used when kicking a remote processor, to let it know that this
>> * vring is triggered.
>> * @pa: physical address
>> @@ -266,18 +266,18 @@ struct fw_rsc_vdev_vring {
>> /**
>> * struct fw_rsc_vdev - virtio device header
>> * @id: virtio device id (as in virtio_ids.h)
>> - * @notifyid is a unique rproc-wide notify index for this vdev. This notify
>> + * @notifyid: a unique rproc-wide notify index for this vdev. This notify
>> * index is used when kicking a remote processor, to let it know that the
>> * status/features of this vdev have changes.
>> - * @dfeatures specifies the virtio device features supported by the firmware
>> - * @gfeatures is a place holder used by the host to write back the
>> + * @dfeatures: specifies the virtio device features supported by the firmware
>> + * @gfeatures: a place holder used by the host to write back the
>> * negotiated features that are supported by both sides.
>> - * @config_len is the size of the virtio config space of this vdev. The config
>> + * @config_len: the size of the virtio config space of this vdev. The config
>> * space lies in the resource table immediate after this vdev header.
>> - * @status is a place holder where the host will indicate its virtio progress.
>> - * @num_of_vrings indicates how many vrings are described in this vdev header
>> + * @status: a place holder where the host will indicate its virtio progress.
>> + * @num_of_vrings: indicates how many vrings are described in this vdev header
>> * @reserved: reserved (must be zero)
>> - * @vring is an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
>> + * @vring: an array of @num_of_vrings entries of 'struct fw_rsc_vdev_vring'.
>> *
>> * This resource is a virtio device header: it provides information about
>> * the vdev, and is then used by the host and its peer remote processors
>> @@ -287,16 +287,17 @@ struct fw_rsc_vdev_vring {
>> * to statically allocate a vdev upon registration of the rproc (dynamic vdev
>> * allocation is not yet supported).
>> *
>> - * Note: unlike virtualization systems, the term 'host' here means
>> - * the Linux side which is running remoteproc to control the remote
>> - * processors. We use the name 'gfeatures' to comply with virtio's terms,
>> - * though there isn't really any virtualized guest OS here: it's the host
>> - * which is responsible for negotiating the final features.
>> - * Yeah, it's a bit confusing.
>> - *
>> - * Note: immediately following this structure is the virtio config space for
>> - * this vdev (which is specific to the vdev; for more info, read the virtio
>> - * spec). the size of the config space is specified by @config_len.
>> + * Note:
>> + * 1. unlike virtualization systems, the term 'host' here means
>> + * the Linux side which is running remoteproc to control the remote
>> + * processors. We use the name 'gfeatures' to comply with virtio's terms,
>> + * though there isn't really any virtualized guest OS here: it's the host
>> + * which is responsible for negotiating the final features.
>> + * Yeah, it's a bit confusing.
>> + *
>> + * 2. immediately following this structure is the virtio config space for
>> + * this vdev (which is specific to the vdev; for more info, read the virtio
>> + * spec). the size of the config space is specified by @config_len.
>
> s/the/The
>
>> */
>> struct fw_rsc_vdev {
>> u32 id;
>> @@ -440,7 +441,7 @@ enum rproc_state {
>> * enum rproc_crash_type - remote processor crash types
>> * @RPROC_MMUFAULT: iommu fault
>> * @RPROC_WATCHDOG: watchdog bite
>> - * @RPROC_FATAL_ERROR fatal error
>> + * @RPROC_FATAL_ERROR: fatal error
>> *
>> * Each element of the enum is used as an array index. So that, the value of
>> * the elements should be always something sane.
>> @@ -457,9 +458,9 @@ enum rproc_crash_type {
>> * enum rproc_dump_mechanism - Coredump options for core
>> * @RPROC_COREDUMP_DISABLED: Don't perform any dump
>> * @RPROC_COREDUMP_ENABLED: Copy dump to separate buffer and carry on with
>> - recovery
>> + * recovery
>> * @RPROC_COREDUMP_INLINE: Read segments directly from device memory. Stall
>> - recovery until all segments are read
>> + * recovery until all segments are read
>> */
>> enum rproc_dump_mechanism {
>> RPROC_COREDUMP_DISABLED,
>> @@ -475,6 +476,7 @@ enum rproc_dump_mechanism {
>> * @priv: private data associated with the dump_segment
>> * @dump: custom dump function to fill device memory segment associated
>> * with coredump
>> + * @offset: offset of the segment
>> */
>> struct rproc_dump_segment {
>> struct list_head node;
>> @@ -524,7 +526,9 @@ struct rproc_dump_segment {
>> * @auto_boot: flag to indicate if remote processor should be auto-started
>> * @dump_segments: list of segments in the firmware
>> * @nb_vdev: number of vdev currently handled by rproc
>> - * @char_dev: character device of the 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
>> */
>> struct rproc {
>> @@ -613,10 +617,10 @@ struct rproc_vring {
>> * struct rproc_vdev - remoteproc state for a supported virtio device
>> * @refcount: reference counter for the vdev and vring allocations
>> * @subdev: handle for registering the vdev as a rproc subdevice
>> + * @dev: device struct used for reference count semantics
>> * @id: virtio device id (as in virtio_ids.h)
>> * @node: list node
>> * @rproc: the rproc handle
>> - * @vdev: the virio device
>> * @vring: the vrings for this vdev
>> * @rsc_offset: offset of the vdev's resource entry
>> * @index: vdev position versus other vdev declared in resource table
>
> With or without the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu.
Bjorn,
Any preferences, let me know if you want me to quickly respin this patch, or if
you want to fixup locally while applying?
regards
Suman
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
2021-05-25 18:00 ` Mathieu Poirier
@ 2021-05-28 3:07 ` Bjorn Andersson
-1 siblings, 0 replies; 14+ messages in thread
From: Bjorn Andersson @ 2021-05-28 3:07 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Suman Anna, linux-remoteproc, linux-arm-kernel, linux-kernel
On Tue 25 May 13:00 CDT 2021, Mathieu Poirier wrote:
> On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
> > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
[..]
> > @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
> > * This function finds the location of the loaded resource table. Don't
> > * call this function if the table wasn't loaded yet - it's a bug if you do.
> > *
> > - * Returns the pointer to the resource table if it is found or NULL otherwise.
> > + * Return: pointer to the resource table if it is found or NULL otherwise.
>
> Here the '.' has been kept while it was remove for all of the above. I don't
> know that the right guidelines are for this.
>
Reviewing https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
I don't see that this is defined. So I'm fine with whatever looks good.
That said, the section about "Return values" shows that the "Return:
..." line should be short and concise and if needed followed by a
newline and then a longer paragraph.
I'll fix the capitalization of "the" below and apply this as is and we
can go back an reformat these multiline Return entries later...
> > * If the table wasn't loaded yet the result is unspecified.
> > */
[..]
> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
[..]
> > + * 2. immediately following this structure is the virtio config space for
> > + * this vdev (which is specific to the vdev; for more info, read the virtio
> > + * spec). the size of the config space is specified by @config_len.
>
> s/the/The
>
[..]
> > struct rproc {
> > @@ -613,10 +617,10 @@ struct rproc_vring {
> > * struct rproc_vdev - remoteproc state for a supported virtio device
> > * @refcount: reference counter for the vdev and vring allocations
> > * @subdev: handle for registering the vdev as a rproc subdevice
> > + * @dev: device struct used for reference count semantics
> > * @id: virtio device id (as in virtio_ids.h)
> > * @node: list node
> > * @rproc: the rproc handle
> > - * @vdev: the virio device
> > * @vring: the vrings for this vdev
> > * @rsc_offset: offset of the vdev's resource entry
> > * @index: vdev position versus other vdev declared in resource table
>
> With or without the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
Thanks Mathieu, and thanks Suman.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] remoteproc: Fix various kernel-doc warnings
@ 2021-05-28 3:07 ` Bjorn Andersson
0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Andersson @ 2021-05-28 3:07 UTC (permalink / raw)
To: Mathieu Poirier; +Cc: linux-remoteproc, linux-kernel, linux-arm-kernel
On Tue 25 May 13:00 CDT 2021, Mathieu Poirier wrote:
> On Wed, May 19, 2021 at 01:03:04PM -0500, Suman Anna wrote:
> > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
[..]
> > @@ -362,7 +366,7 @@ EXPORT_SYMBOL(rproc_elf_load_rsc_table);
> > * This function finds the location of the loaded resource table. Don't
> > * call this function if the table wasn't loaded yet - it's a bug if you do.
> > *
> > - * Returns the pointer to the resource table if it is found or NULL otherwise.
> > + * Return: pointer to the resource table if it is found or NULL otherwise.
>
> Here the '.' has been kept while it was remove for all of the above. I don't
> know that the right guidelines are for this.
>
Reviewing https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
I don't see that this is defined. So I'm fine with whatever looks good.
That said, the section about "Return values" shows that the "Return:
..." line should be short and concise and if needed followed by a
newline and then a longer paragraph.
I'll fix the capitalization of "the" below and apply this as is and we
can go back an reformat these multiline Return entries later...
> > * If the table wasn't loaded yet the result is unspecified.
> > */
[..]
> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
[..]
> > + * 2. immediately following this structure is the virtio config space for
> > + * this vdev (which is specific to the vdev; for more info, read the virtio
> > + * spec). the size of the config space is specified by @config_len.
>
> s/the/The
>
[..]
> > struct rproc {
> > @@ -613,10 +617,10 @@ struct rproc_vring {
> > * struct rproc_vdev - remoteproc state for a supported virtio device
> > * @refcount: reference counter for the vdev and vring allocations
> > * @subdev: handle for registering the vdev as a rproc subdevice
> > + * @dev: device struct used for reference count semantics
> > * @id: virtio device id (as in virtio_ids.h)
> > * @node: list node
> > * @rproc: the rproc handle
> > - * @vdev: the virio device
> > * @vring: the vrings for this vdev
> > * @rsc_offset: offset of the vdev's resource entry
> > * @index: vdev position versus other vdev declared in resource table
>
> With or without the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
Thanks Mathieu, and thanks Suman.
Regards,
Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-05-28 3:09 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 18:03 [PATCH 0/2] Minor remoteproc cleanups Suman Anna
2021-05-19 18:03 ` Suman Anna
2021-05-19 18:03 ` [PATCH 1/2] remoteproc: Add kernel-doc comment for is_iomem Suman Anna
2021-05-19 18:03 ` Suman Anna
2021-05-25 17:37 ` Mathieu Poirier
2021-05-25 17:37 ` Mathieu Poirier
2021-05-19 18:03 ` [PATCH 2/2] remoteproc: Fix various kernel-doc warnings Suman Anna
2021-05-19 18:03 ` Suman Anna
2021-05-25 18:00 ` Mathieu Poirier
2021-05-25 18:00 ` Mathieu Poirier
2021-05-25 18:07 ` Suman Anna
2021-05-25 18:07 ` Suman Anna
2021-05-28 3:07 ` Bjorn Andersson
2021-05-28 3:07 ` Bjorn Andersson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.