Linux Remote Processor Subsystem development
 help / color / mirror / Atom feed
* [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