* [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