* [PATCH 0/6] remoteproc: core: misc update
@ 2025-10-05 14:14 Peng Fan
2025-10-05 14:14 ` [PATCH 1/6] remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown() Peng Fan
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
This patchset is a misc update of remoteproc_core.c.
Patch 1: Drop a pointless initialization to variable ret
Patch 2-3: Cleanup the included headers
Patch 4: Use cleanup.h to simplify code
Patch 5: Remove export of rproc_va_to_pa
Patch 6: Use 1-bit bitfields for bool
I am also reviewing the rproc->lock usage and thinking whether we
need to add a lockdep_assert_held for some functions that should have
lock held. But not sure.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (6):
remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown()
remoteproc: core: Sort header includes
remoteproc: core: Removed unused headers
remoteproc: core: Use cleanup.h macros to simplify lock handling
remoteproc: core: Remove unused export of rproc_va_to_pa
remoteproc: core: Consolidate bool flags into 1-bit bitfields
drivers/remoteproc/remoteproc_core.c | 144 ++++++++++++++---------------------
include/linux/remoteproc.h | 18 ++---
2 files changed, 67 insertions(+), 95 deletions(-)
---
base-commit: 3b9b1f8df454caa453c7fb07689064edb2eda90a
change-id: 20251003-remoteproc-cleanup-345cd50fe138
Best regards,
--
Peng Fan <peng.fan@nxp.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/6] remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown()
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-05 14:14 ` [PATCH 2/6] remoteproc: core: Sort header includes Peng Fan
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
The variable ret is immediately assigned the return value of
mutex_lock_interruptible(), making its prior initialization to zero
unnecessary. Remove the redundant assignment
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/remoteproc_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 82567210052893a501e7591204af1feb07befb22..29bbaa349e340eedd122fb553004f7e6a5c46e55 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1989,7 +1989,7 @@ EXPORT_SYMBOL(rproc_boot);
int rproc_shutdown(struct rproc *rproc)
{
struct device *dev = &rproc->dev;
- int ret = 0;
+ int ret;
ret = mutex_lock_interruptible(&rproc->lock);
if (ret) {
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/6] remoteproc: core: Sort header includes
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
2025-10-05 14:14 ` [PATCH 1/6] remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown() Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-05 14:14 ` [PATCH 3/6] remoteproc: core: Removed unused headers Peng Fan
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
Reordered the header includes in drivers/remoteproc/remoteproc_core.c
to follow alphabetical order to simplify future maintenance.
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/remoteproc_core.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 29bbaa349e340eedd122fb553004f7e6a5c46e55..f7d21e99d171667d925de769db003c4e13fe8fe8 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -16,29 +16,29 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <asm/byteorder.h>
+#include <linux/crc32.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
#include <linux/device.h>
-#include <linux/panic_notifier.h>
-#include <linux/slab.h>
-#include <linux/mutex.h>
#include <linux/dma-mapping.h>
+#include <linux/elf.h>
#include <linux/firmware.h>
-#include <linux/string.h>
-#include <linux/debugfs.h>
-#include <linux/rculist.h>
-#include <linux/remoteproc.h>
-#include <linux/iommu.h>
#include <linux/idr.h>
-#include <linux/elf.h>
-#include <linux/crc32.h>
+#include <linux/iommu.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of_platform.h>
#include <linux/of_reserved_mem.h>
+#include <linux/panic_notifier.h>
+#include <linux/platform_device.h>
+#include <linux/rculist.h>
+#include <linux/remoteproc.h>
+#include <linux/slab.h>
+#include <linux/string.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
-#include <asm/byteorder.h>
-#include <linux/platform_device.h>
#include "remoteproc_internal.h"
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/6] remoteproc: core: Removed unused headers
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
2025-10-05 14:14 ` [PATCH 1/6] remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown() Peng Fan
2025-10-05 14:14 ` [PATCH 2/6] remoteproc: core: Sort header includes Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-05 14:14 ` [PATCH 4/6] remoteproc: core: Use cleanup.h macros to simplify lock handling Peng Fan
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
There is no user of crc32.h, debugfs.h, of_reserved_mem.h, virtio_ids.h,
so remove from the included headers.
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/remoteproc_core.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index f7d21e99d171667d925de769db003c4e13fe8fe8..8004a480348378abef78ad5641a8c8b5766c20a6 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -17,8 +17,6 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
#include <asm/byteorder.h>
-#include <linux/crc32.h>
-#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
@@ -30,14 +28,12 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of_platform.h>
-#include <linux/of_reserved_mem.h>
#include <linux/panic_notifier.h>
#include <linux/platform_device.h>
#include <linux/rculist.h>
#include <linux/remoteproc.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
#include "remoteproc_internal.h"
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/6] remoteproc: core: Use cleanup.h macros to simplify lock handling
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
` (2 preceding siblings ...)
2025-10-05 14:14 ` [PATCH 3/6] remoteproc: core: Removed unused headers Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-05 14:14 ` [PATCH 5/6] remoteproc: core: Remove unused export of rproc_va_to_pa Peng Fan
2025-10-05 14:14 ` [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields Peng Fan
5 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
Replace manual mutex_lock/unlock and error-handling patterns with cleanup.h
macros (ACQUIRE, ACQUIRE_ERR, and scoped_guard) to streamline lock
management. As a result, several goto labels and redundant error paths are
eliminated.
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/remoteproc_core.c | 113 ++++++++++++++---------------------
1 file changed, 45 insertions(+), 68 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 8004a480348378abef78ad5641a8c8b5766c20a6..dd859378f6ff6dec2728980cc82d31687aa7a3dc 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -17,6 +17,7 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
#include <asm/byteorder.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
@@ -1830,13 +1831,14 @@ int rproc_trigger_recovery(struct rproc *rproc)
struct device *dev = &rproc->dev;
int ret;
- ret = mutex_lock_interruptible(&rproc->lock);
+ ACQUIRE(mutex_intr, lock)(&rproc->lock);
+ ret = ACQUIRE_ERR(mutex_intr, &lock);
if (ret)
return ret;
/* State could have changed before we got the mutex */
if (rproc->state != RPROC_CRASHED)
- goto unlock_mutex;
+ return ret;
dev_err(dev, "recovering %s\n", rproc->name);
@@ -1845,8 +1847,6 @@ int rproc_trigger_recovery(struct rproc *rproc)
else
ret = rproc_boot_recovery(rproc);
-unlock_mutex:
- mutex_unlock(&rproc->lock);
return ret;
}
@@ -1864,25 +1864,19 @@ static void rproc_crash_handler_work(struct work_struct *work)
dev_dbg(dev, "enter %s\n", __func__);
- mutex_lock(&rproc->lock);
-
- if (rproc->state == RPROC_CRASHED) {
+ scoped_guard(mutex, &rproc->lock) {
/* handle only the first crash detected */
- mutex_unlock(&rproc->lock);
- return;
- }
+ if (rproc->state == RPROC_CRASHED)
+ return;
- if (rproc->state == RPROC_OFFLINE) {
/* Don't recover if the remote processor was stopped */
- mutex_unlock(&rproc->lock);
- goto out;
- }
-
- rproc->state = RPROC_CRASHED;
- dev_err(dev, "handling crash #%u in %s\n", ++rproc->crash_cnt,
- rproc->name);
+ if (rproc->state == RPROC_OFFLINE)
+ goto out;
- mutex_unlock(&rproc->lock);
+ rproc->state = RPROC_CRASHED;
+ dev_err(dev, "handling crash #%u in %s\n", ++rproc->crash_cnt,
+ rproc->name);
+ }
if (!rproc->recovery_disabled)
rproc_trigger_recovery(rproc);
@@ -1915,23 +1909,21 @@ int rproc_boot(struct rproc *rproc)
dev = &rproc->dev;
- ret = mutex_lock_interruptible(&rproc->lock);
+ ACQUIRE(mutex_intr, lock)(&rproc->lock);
+ ret = ACQUIRE_ERR(mutex_intr, &lock);
if (ret) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
return ret;
}
if (rproc->state == RPROC_DELETED) {
- ret = -ENODEV;
dev_err(dev, "can't boot deleted rproc %s\n", rproc->name);
- goto unlock_mutex;
+ return -ENODEV;
}
/* skip the boot or attach process if rproc is already powered up */
- if (atomic_inc_return(&rproc->power) > 1) {
- ret = 0;
- goto unlock_mutex;
- }
+ if (atomic_inc_return(&rproc->power) > 1)
+ return 0;
if (rproc->state == RPROC_DETACHED) {
dev_info(dev, "attaching to %s\n", rproc->name);
@@ -1955,8 +1947,7 @@ int rproc_boot(struct rproc *rproc)
downref_rproc:
if (ret)
atomic_dec(&rproc->power);
-unlock_mutex:
- mutex_unlock(&rproc->lock);
+
return ret;
}
EXPORT_SYMBOL(rproc_boot);
@@ -1987,26 +1978,24 @@ int rproc_shutdown(struct rproc *rproc)
struct device *dev = &rproc->dev;
int ret;
- ret = mutex_lock_interruptible(&rproc->lock);
+ ACQUIRE(mutex_intr, lock)(&rproc->lock);
+ ret = ACQUIRE_ERR(mutex_intr, &lock);
if (ret) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
return ret;
}
- if (rproc->state != RPROC_RUNNING &&
- rproc->state != RPROC_ATTACHED) {
- ret = -EINVAL;
- goto out;
- }
+ if (rproc->state != RPROC_RUNNING && rproc->state != RPROC_ATTACHED)
+ return -EINVAL;
/* if the remote proc is still needed, bail out */
if (!atomic_dec_and_test(&rproc->power))
- goto out;
+ return ret;
ret = rproc_stop(rproc, false);
if (ret) {
atomic_inc(&rproc->power);
- goto out;
+ return ret;
}
/* clean up all acquired resources */
@@ -2021,8 +2010,7 @@ int rproc_shutdown(struct rproc *rproc)
kfree(rproc->cached_table);
rproc->cached_table = NULL;
rproc->table_ptr = NULL;
-out:
- mutex_unlock(&rproc->lock);
+
return ret;
}
EXPORT_SYMBOL(rproc_shutdown);
@@ -2052,27 +2040,25 @@ int rproc_detach(struct rproc *rproc)
struct device *dev = &rproc->dev;
int ret;
- ret = mutex_lock_interruptible(&rproc->lock);
+ ACQUIRE(mutex_intr, lock)(&rproc->lock);
+ ret = ACQUIRE_ERR(mutex_intr, &lock);
if (ret) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
return ret;
}
if (rproc->state != RPROC_ATTACHED) {
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
/* if the remote proc is still needed, bail out */
- if (!atomic_dec_and_test(&rproc->power)) {
- ret = 0;
- goto out;
- }
+ if (!atomic_dec_and_test(&rproc->power))
+ return 0;
ret = __rproc_detach(rproc);
if (ret) {
atomic_inc(&rproc->power);
- goto out;
+ return ret;
}
/* clean up all acquired resources */
@@ -2087,8 +2073,7 @@ int rproc_detach(struct rproc *rproc)
kfree(rproc->cached_table);
rproc->cached_table = NULL;
rproc->table_ptr = NULL;
-out:
- mutex_unlock(&rproc->lock);
+
return ret;
}
EXPORT_SYMBOL(rproc_detach);
@@ -2192,7 +2177,8 @@ int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
dev = rproc->dev.parent;
- ret = mutex_lock_interruptible(&rproc->lock);
+ ACQUIRE(mutex_intr, lock)(&rproc->lock);
+ ret = ACQUIRE_ERR(mutex_intr, &lock);
if (ret) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
return -EINVAL;
@@ -2200,28 +2186,22 @@ int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
if (rproc->state != RPROC_OFFLINE) {
dev_err(dev, "can't change firmware while running\n");
- ret = -EBUSY;
- goto out;
+ return -EBUSY;
}
len = strcspn(fw_name, "\n");
if (!len) {
dev_err(dev, "can't provide empty string for firmware name\n");
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
p = kstrndup(fw_name, len, GFP_KERNEL);
- if (!p) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!p)
+ return -ENOMEM;
kfree_const(rproc->firmware);
rproc->firmware = p;
-out:
- mutex_unlock(&rproc->lock);
return ret;
}
EXPORT_SYMBOL(rproc_set_firmware);
@@ -2316,9 +2296,8 @@ int rproc_add(struct rproc *rproc)
}
/* expose to rproc_get_by_phandle users */
- mutex_lock(&rproc_list_mutex);
- list_add_rcu(&rproc->node, &rproc_list);
- mutex_unlock(&rproc_list_mutex);
+ scoped_guard(mutex, &rproc_list_mutex)
+ list_add_rcu(&rproc->node, &rproc_list);
return 0;
@@ -2582,16 +2561,14 @@ int rproc_del(struct rproc *rproc)
/* TODO: make sure this works with rproc->power > 1 */
rproc_shutdown(rproc);
- mutex_lock(&rproc->lock);
- rproc->state = RPROC_DELETED;
- mutex_unlock(&rproc->lock);
+ scoped_guard(mutex, &rproc->lock)
+ rproc->state = RPROC_DELETED;
rproc_delete_debug_dir(rproc);
/* the rproc is downref'ed as soon as it's removed from the klist */
- mutex_lock(&rproc_list_mutex);
- list_del_rcu(&rproc->node);
- mutex_unlock(&rproc_list_mutex);
+ scoped_guard(mutex, &rproc_list_mutex)
+ list_del_rcu(&rproc->node);
/* Ensure that no readers of rproc_list are still active */
synchronize_rcu();
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/6] remoteproc: core: Remove unused export of rproc_va_to_pa
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
` (3 preceding siblings ...)
2025-10-05 14:14 ` [PATCH 4/6] remoteproc: core: Use cleanup.h macros to simplify lock handling Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-05 14:14 ` [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields Peng Fan
5 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
commit 086d08725d34 ("remoteproc: create vdev subdevice with specific dma
memory pool") added an export for rproc_va_to_pa. However, since its
introduction, this symbol has not been used by any loadable modules. It
remains only referenced within remoteproc_virtio.c, which is always built
together with remoteproc_core.c.
As such, exporting rproc_va_to_pa is unnecessary, so remove the export.
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/remoteproc_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index dd859378f6ff6dec2728980cc82d31687aa7a3dc..383479d624c89da1c481adc956a169c03b793bcc 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -156,7 +156,6 @@ phys_addr_t rproc_va_to_pa(void *cpu_addr)
WARN_ON(!virt_addr_valid(cpu_addr));
return virt_to_phys(cpu_addr);
}
-EXPORT_SYMBOL(rproc_va_to_pa);
/**
* rproc_da_to_va() - lookup the kernel virtual address for a remoteproc address
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
` (4 preceding siblings ...)
2025-10-05 14:14 ` [PATCH 5/6] remoteproc: core: Remove unused export of rproc_va_to_pa Peng Fan
@ 2025-10-05 14:14 ` Peng Fan
2025-10-10 8:44 ` kernel test robot
2025-10-10 13:01 ` Andrew Davis
5 siblings, 2 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-05 14:14 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Andrew Davis, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel, Peng Fan
Per Documentation/process/coding-style.rst rule 17 regarding the use of
bool types:
If a structure has many true/false values, consider consolidating them into
a bitfield with 1-bit members, or using an appropriate fixed-width type
such as u8.
This commit replaces multiple bool members in struct rproc with 1-bit
bitfields and groups them together. This change reduces the overall size of
struct rproc from 0x4d8 to 0x4c8 on ARM64.
No functional changes.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
include/linux/remoteproc.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index b4795698d8c2a4e80ccafbe632436c4dfb636a1e..d8468a96edfbd82f4011881c10f59bf7c12e9c1a 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -528,21 +528,21 @@ enum rproc_features {
* @index: index of this rproc device
* @crash_handler: workqueue for handling a crash
* @crash_cnt: crash counter
- * @recovery_disabled: flag that state if recovery was disabled
* @max_notifyid: largest allocated notify id.
* @table_ptr: pointer to the resource table in effect
* @clean_table: copy of the resource table without modifications. Used
* when a remote processor is attached or detached from the core
* @cached_table: copy of the resource table
* @table_sz: size of @cached_table
- * @has_iommu: flag to indicate if remote processor is behind an MMU
- * @auto_boot: flag to indicate if remote processor should be auto-started
- * @sysfs_read_only: flag to make remoteproc sysfs files read only
* @dump_segments: list of segments in the firmware
* @nb_vdev: number of vdev currently handled by rproc
* @elf_class: firmware ELF class
* @elf_machine: firmware ELF machine
* @cdev: character device of the rproc
+ * @recovery_disabled: flag that state if recovery was disabled
+ * @has_iommu: flag to indicate if remote processor is behind an MMU
+ * @auto_boot: flag to indicate if remote processor should be auto-started
+ * @sysfs_read_only: flag to make remoteproc sysfs files read only
* @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
* @features: indicate remoteproc features
*/
@@ -570,21 +570,21 @@ struct rproc {
int index;
struct work_struct crash_handler;
unsigned int crash_cnt;
- bool recovery_disabled;
int max_notifyid;
struct resource_table *table_ptr;
struct resource_table *clean_table;
struct resource_table *cached_table;
size_t table_sz;
- bool has_iommu;
- bool auto_boot;
- bool sysfs_read_only;
struct list_head dump_segments;
int nb_vdev;
u8 elf_class;
u16 elf_machine;
struct cdev cdev;
- bool cdev_put_on_release;
+ bool recovery_disabled :1;
+ bool has_iommu :1;
+ bool auto_boot :1;
+ bool sysfs_read_only :1;
+ bool cdev_put_on_release :1;
DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
};
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
2025-10-05 14:14 ` [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields Peng Fan
@ 2025-10-10 8:44 ` kernel test robot
2025-10-10 13:01 ` Andrew Davis
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-10-10 8:44 UTC (permalink / raw)
To: Peng Fan, Bjorn Andersson, Mathieu Poirier, Andrew Davis,
Arnaud Pouliquen, Daniel Baluta
Cc: oe-kbuild-all, linux-remoteproc, linux-kernel, Peng Fan
Hi Peng,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3b9b1f8df454caa453c7fb07689064edb2eda90a]
url: https://github.com/intel-lab-lkp/linux/commits/Peng-Fan/remoteproc-core-Drop-redundant-initialization-of-ret-in-rproc_shutdown/20251010-012012
base: 3b9b1f8df454caa453c7fb07689064edb2eda90a
patch link: https://lore.kernel.org/r/20251005-remoteproc-cleanup-v1-6-09a9fdea0063%40nxp.com
patch subject: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
config: riscv-randconfig-002-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101653.wulDfnoN-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 9.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510101653.wulDfnoN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510101653.wulDfnoN-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_probe':
>> drivers/remoteproc/stm32_rproc.c:860:42: error: cannot take address of bit-field 'auto_boot'
860 | ret = stm32_rproc_parse_dt(pdev, ddata, &rproc->auto_boot);
| ^
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
Depends on [n]: BINFMT_ELF [=n] && ELF_CORE [=y]
Selected by [y]:
- RISCV [=y]
vim +/auto_boot +860 drivers/remoteproc/stm32_rproc.c
376ffdc044568f Mathieu Poirier 2020-07-14 832
13140de09cc2dd Fabien Dessenne 2019-05-14 833 static int stm32_rproc_probe(struct platform_device *pdev)
13140de09cc2dd Fabien Dessenne 2019-05-14 834 {
13140de09cc2dd Fabien Dessenne 2019-05-14 835 struct device *dev = &pdev->dev;
13140de09cc2dd Fabien Dessenne 2019-05-14 836 struct stm32_rproc *ddata;
13140de09cc2dd Fabien Dessenne 2019-05-14 837 struct device_node *np = dev->of_node;
710028a2e4d76c Arnaud Pouliquen 2025-03-27 838 const char *fw_name;
13140de09cc2dd Fabien Dessenne 2019-05-14 839 struct rproc *rproc;
376ffdc044568f Mathieu Poirier 2020-07-14 840 unsigned int state;
13140de09cc2dd Fabien Dessenne 2019-05-14 841 int ret;
13140de09cc2dd Fabien Dessenne 2019-05-14 842
13140de09cc2dd Fabien Dessenne 2019-05-14 843 ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
13140de09cc2dd Fabien Dessenne 2019-05-14 844 if (ret)
13140de09cc2dd Fabien Dessenne 2019-05-14 845 return ret;
13140de09cc2dd Fabien Dessenne 2019-05-14 846
710028a2e4d76c Arnaud Pouliquen 2025-03-27 847 /* Look for an optional firmware name */
710028a2e4d76c Arnaud Pouliquen 2025-03-27 848 ret = rproc_of_parse_firmware(dev, 0, &fw_name);
710028a2e4d76c Arnaud Pouliquen 2025-03-27 849 if (ret < 0 && ret != -EINVAL)
710028a2e4d76c Arnaud Pouliquen 2025-03-27 850 return ret;
710028a2e4d76c Arnaud Pouliquen 2025-03-27 851
710028a2e4d76c Arnaud Pouliquen 2025-03-27 852 rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, fw_name, sizeof(*ddata));
13140de09cc2dd Fabien Dessenne 2019-05-14 853 if (!rproc)
13140de09cc2dd Fabien Dessenne 2019-05-14 854 return -ENOMEM;
13140de09cc2dd Fabien Dessenne 2019-05-14 855
8210fc873d2f1a Mathieu Poirier 2020-07-14 856 ddata = rproc->priv;
8210fc873d2f1a Mathieu Poirier 2020-07-14 857
3898fc99d19934 Clement Leger 2020-04-10 858 rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
8210fc873d2f1a Mathieu Poirier 2020-07-14 859
8210fc873d2f1a Mathieu Poirier 2020-07-14 @860 ret = stm32_rproc_parse_dt(pdev, ddata, &rproc->auto_boot);
8210fc873d2f1a Mathieu Poirier 2020-07-14 861 if (ret)
8210fc873d2f1a Mathieu Poirier 2020-07-14 862 goto free_rproc;
8210fc873d2f1a Mathieu Poirier 2020-07-14 863
95e32f868aa67c Mathieu Poirier 2020-07-14 864 ret = stm32_rproc_of_memory_translations(pdev, ddata);
95e32f868aa67c Mathieu Poirier 2020-07-14 865 if (ret)
95e32f868aa67c Mathieu Poirier 2020-07-14 866 goto free_rproc;
95e32f868aa67c Mathieu Poirier 2020-07-14 867
376ffdc044568f Mathieu Poirier 2020-07-14 868 ret = stm32_rproc_get_m4_status(ddata, &state);
376ffdc044568f Mathieu Poirier 2020-07-14 869 if (ret)
376ffdc044568f Mathieu Poirier 2020-07-14 870 goto free_rproc;
376ffdc044568f Mathieu Poirier 2020-07-14 871
6e20a05104e55d Arnaud POULIQUEN 2021-03-12 872 if (state == M4_STATE_CRUN)
376ffdc044568f Mathieu Poirier 2020-07-14 873 rproc->state = RPROC_DETACHED;
376ffdc044568f Mathieu Poirier 2020-07-14 874
13140de09cc2dd Fabien Dessenne 2019-05-14 875 rproc->has_iommu = false;
714cf5e3846047 Arnaud Pouliquen 2019-10-25 876 ddata->workqueue = create_workqueue(dev_name(dev));
714cf5e3846047 Arnaud Pouliquen 2019-10-25 877 if (!ddata->workqueue) {
714cf5e3846047 Arnaud Pouliquen 2019-10-25 878 dev_err(dev, "cannot create workqueue\n");
714cf5e3846047 Arnaud Pouliquen 2019-10-25 879 ret = -ENOMEM;
dadbdb9c304c51 Mathieu Poirier 2020-07-14 880 goto free_resources;
714cf5e3846047 Arnaud Pouliquen 2019-10-25 881 }
13140de09cc2dd Fabien Dessenne 2019-05-14 882
13140de09cc2dd Fabien Dessenne 2019-05-14 883 platform_set_drvdata(pdev, rproc);
13140de09cc2dd Fabien Dessenne 2019-05-14 884
4a56e423e0e19b Fabien Dessenne 2019-11-15 885 ret = stm32_rproc_request_mbox(rproc);
4a56e423e0e19b Fabien Dessenne 2019-11-15 886 if (ret)
8210fc873d2f1a Mathieu Poirier 2020-07-14 887 goto free_wkq;
13140de09cc2dd Fabien Dessenne 2019-05-14 888
13140de09cc2dd Fabien Dessenne 2019-05-14 889 ret = rproc_add(rproc);
13140de09cc2dd Fabien Dessenne 2019-05-14 890 if (ret)
13140de09cc2dd Fabien Dessenne 2019-05-14 891 goto free_mb;
13140de09cc2dd Fabien Dessenne 2019-05-14 892
13140de09cc2dd Fabien Dessenne 2019-05-14 893 return 0;
13140de09cc2dd Fabien Dessenne 2019-05-14 894
13140de09cc2dd Fabien Dessenne 2019-05-14 895 free_mb:
13140de09cc2dd Fabien Dessenne 2019-05-14 896 stm32_rproc_free_mbox(rproc);
714cf5e3846047 Arnaud Pouliquen 2019-10-25 897 free_wkq:
714cf5e3846047 Arnaud Pouliquen 2019-10-25 898 destroy_workqueue(ddata->workqueue);
dadbdb9c304c51 Mathieu Poirier 2020-07-14 899 free_resources:
dadbdb9c304c51 Mathieu Poirier 2020-07-14 900 rproc_resource_cleanup(rproc);
13140de09cc2dd Fabien Dessenne 2019-05-14 901 free_rproc:
410119ee29b6c1 Fabien Dessenne 2019-08-26 902 if (device_may_wakeup(dev)) {
410119ee29b6c1 Fabien Dessenne 2019-08-26 903 dev_pm_clear_wake_irq(dev);
410119ee29b6c1 Fabien Dessenne 2019-08-26 904 device_init_wakeup(dev, false);
410119ee29b6c1 Fabien Dessenne 2019-08-26 905 }
13140de09cc2dd Fabien Dessenne 2019-05-14 906 return ret;
13140de09cc2dd Fabien Dessenne 2019-05-14 907 }
13140de09cc2dd Fabien Dessenne 2019-05-14 908
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
2025-10-05 14:14 ` [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields Peng Fan
2025-10-10 8:44 ` kernel test robot
@ 2025-10-10 13:01 ` Andrew Davis
2025-10-10 14:21 ` Peng Fan
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Davis @ 2025-10-10 13:01 UTC (permalink / raw)
To: Peng Fan, Bjorn Andersson, Mathieu Poirier, Arnaud Pouliquen,
Daniel Baluta
Cc: linux-remoteproc, linux-kernel
On 10/5/25 9:14 AM, Peng Fan wrote:
> Per Documentation/process/coding-style.rst rule 17 regarding the use of
> bool types:
> If a structure has many true/false values, consider consolidating them into
> a bitfield with 1-bit members, or using an appropriate fixed-width type
> such as u8.
>
> This commit replaces multiple bool members in struct rproc with 1-bit
> bitfields and groups them together. This change reduces the overall size of
> struct rproc from 0x4d8 to 0x4c8 on ARM64.
>
Most of the series looks good, but this patch I'm not a fan. This isn't
a huge savings and bitfields come with many of their own challenges.
> No functional changes.
If the structure's size changed then that is a functional change. There
also is probably a performance change from extracting the value out of the
bitfield, where before they might have each been an aligned width variable
that could be tested in a single cycle.
Andrew
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> include/linux/remoteproc.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index b4795698d8c2a4e80ccafbe632436c4dfb636a1e..d8468a96edfbd82f4011881c10f59bf7c12e9c1a 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -528,21 +528,21 @@ enum rproc_features {
> * @index: index of this rproc device
> * @crash_handler: workqueue for handling a crash
> * @crash_cnt: crash counter
> - * @recovery_disabled: flag that state if recovery was disabled
> * @max_notifyid: largest allocated notify id.
> * @table_ptr: pointer to the resource table in effect
> * @clean_table: copy of the resource table without modifications. Used
> * when a remote processor is attached or detached from the core
> * @cached_table: copy of the resource table
> * @table_sz: size of @cached_table
> - * @has_iommu: flag to indicate if remote processor is behind an MMU
> - * @auto_boot: flag to indicate if remote processor should be auto-started
> - * @sysfs_read_only: flag to make remoteproc sysfs files read only
> * @dump_segments: list of segments in the firmware
> * @nb_vdev: number of vdev currently handled by rproc
> * @elf_class: firmware ELF class
> * @elf_machine: firmware ELF machine
> * @cdev: character device of the rproc
> + * @recovery_disabled: flag that state if recovery was disabled
> + * @has_iommu: flag to indicate if remote processor is behind an MMU
> + * @auto_boot: flag to indicate if remote processor should be auto-started
> + * @sysfs_read_only: flag to make remoteproc sysfs files read only
> * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
> * @features: indicate remoteproc features
> */
> @@ -570,21 +570,21 @@ struct rproc {
> int index;
> struct work_struct crash_handler;
> unsigned int crash_cnt;
> - bool recovery_disabled;
> int max_notifyid;
> struct resource_table *table_ptr;
> struct resource_table *clean_table;
> struct resource_table *cached_table;
> size_t table_sz;
> - bool has_iommu;
> - bool auto_boot;
> - bool sysfs_read_only;
> struct list_head dump_segments;
> int nb_vdev;
> u8 elf_class;
> u16 elf_machine;
> struct cdev cdev;
> - bool cdev_put_on_release;
> + bool recovery_disabled :1;
> + bool has_iommu :1;
> + bool auto_boot :1;
> + bool sysfs_read_only :1;
> + bool cdev_put_on_release :1;
> DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
> };
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
2025-10-10 13:01 ` Andrew Davis
@ 2025-10-10 14:21 ` Peng Fan
0 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-10-10 14:21 UTC (permalink / raw)
To: Andrew Davis
Cc: Peng Fan, Bjorn Andersson, Mathieu Poirier, Arnaud Pouliquen,
Daniel Baluta, linux-remoteproc, linux-kernel
Hi Andrew,
On Fri, Oct 10, 2025 at 08:01:10AM -0500, Andrew Davis wrote:
>On 10/5/25 9:14 AM, Peng Fan wrote:
>> Per Documentation/process/coding-style.rst rule 17 regarding the use of
>> bool types:
>> If a structure has many true/false values, consider consolidating them into
>> a bitfield with 1-bit members, or using an appropriate fixed-width type
>> such as u8.
>>
>> This commit replaces multiple bool members in struct rproc with 1-bit
>> bitfields and groups them together. This change reduces the overall size of
>> struct rproc from 0x4d8 to 0x4c8 on ARM64.
>>
>
>Most of the series looks good, but this patch I'm not a fan. This isn't
>a huge savings and bitfields come with many of their own challenges.
Thanks for giving a look on the patchset.
I could drop this change in V3. May I get your R-b for other patches?
(V2 was just posted out after fixed stm32_rproc build issue)
>
>> No functional changes.
>
>If the structure's size changed then that is a functional change. There
Got it.
>also is probably a performance change from extracting the value out of the
>bitfield, where before they might have each been an aligned width variable
>that could be tested in a single cycle.
Agree, but performance may not be critical here.
No problem, I could drop this patch. Let me wait to collect more comments.
Thanks,
Peng
>
>Andrew
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-10-10 13:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-05 14:14 [PATCH 0/6] remoteproc: core: misc update Peng Fan
2025-10-05 14:14 ` [PATCH 1/6] remoteproc: core: Drop redundant initialization of 'ret' in rproc_shutdown() Peng Fan
2025-10-05 14:14 ` [PATCH 2/6] remoteproc: core: Sort header includes Peng Fan
2025-10-05 14:14 ` [PATCH 3/6] remoteproc: core: Removed unused headers Peng Fan
2025-10-05 14:14 ` [PATCH 4/6] remoteproc: core: Use cleanup.h macros to simplify lock handling Peng Fan
2025-10-05 14:14 ` [PATCH 5/6] remoteproc: core: Remove unused export of rproc_va_to_pa Peng Fan
2025-10-05 14:14 ` [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields Peng Fan
2025-10-10 8:44 ` kernel test robot
2025-10-10 13:01 ` Andrew Davis
2025-10-10 14:21 ` Peng Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox