1. Patchew
  2. linux
  3. [PATCH 0/6] remoteproc: core: misc update
  4. View patch

[PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields

Peng Fan posted 6 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
Posted by Peng Fan 2 months, 1 week ago 
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
Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
Posted by Andrew Davis 2 months, 1 week ago 
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);
>   };
>   
>
Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
Posted by Peng Fan 2 months, 1 week ago 
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
>
Re: [PATCH 6/6] remoteproc: core: Consolidate bool flags into 1-bit bitfields
Posted by kernel test robot 2 months, 1 week ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.