![[RTEMS Logo]](/images/logo.png){kind=logo}
Opened on 01/27/23 at 16:09:22
Closed on 07/14/23 at 10:01:06
#4828 closed enhancement (fixed)
Macro defined but magic number used instead
| Reported by: | Daniel Páscoa | Owned by: | Sebastian Huber |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.1 |
| Component: | bsps | Version: | 6 |
| Severity: | normal | Keywords: | qualification |
| Cc: | Blocked By: | ||
| Blocking: |
Description
Description:
Each register has a dedicated SHIFT macro, however, it is not utilized in subsequent macro definitions where the shift operation is performed. To improve consistency and maintainability, consider incorporating the defined shift macro in the subsequent macro definitions instead of using hardcoded shift values.
Additional Notes:
These findings are related to the fact that a bunch of macros like AHBSTAT_AHBS_HMASTER_SHIFT are defined and then are not used right in the next few lines.
#define AHBSTAT_AHBS_HMASTER_SHIFT 3
#define AHBSTAT_AHBS_HMASTER_MASK 0x78U
#define AHBSTAT_AHBS_HMASTER_GET( _reg ) \
( ( ( _reg ) >> 3 ) & 0xfU )
#define AHBSTAT_AHBS_HMASTER( _val ) ( ( _val ) << 3 )
As stated, in every one of these files, “This file is part of the RTEMS quality process and was automatically generated". There are YAML files where these registers are defined and the Waf build system generates the header files automatically with the current configuration.
This ticket was raised as an outcome of the Independent SW Verification and Validation (ISVV) for ESA-promoted RTEMS SMP Qualification Data Packs (https://rtems-qual.io.esa.int). Original ISVV reference for this issue is RTEMS-SMP-CODE-VER-013.
File list:
bsps\include\grlib\ahbstat-regs.h
bsps\include\grlib\ahbtrace-regs.h
bsps\include\grlib\apbuart-regs.h
bsps\include\grlib\dsu4-regs.h
bsps\include\grlib\ftmctrl-regs.h
bsps\include\grlib\gptimer-regs.h
bsps\include\grlib\gr1553b-regs.h
bsps\include\grlib\gr740thsens-regs.h
bsps\include\grlib\grcan-regs.h
bsps\include\grlib\grclkgate-regs.h
bsps\include\grlib\grethgbit-regs.h
bsps\include\grlib\grgpio-regs.h
bsps\include\grlib\grgprbank-regs.h
bsps\include\grlib\grgpreg-regs.h
bsps\include\grlib\griommu-regs.h
bsps\include\grlib\grpci2-regs.h
bsps\include\grlib\grspw2-regs.h
bsps\include\grlib\grspwrouter-regs.h
bsps\include\grlib\irqamp-regs.h
bsps\include\grlib\l4stat-regs.h
bsps\include\grlib\memscrub-regs.h
bsps\include\grlib\l2cache-regs.h
bsps\include\grlib\mmctrl-regs.h
bsps\include\grlib\spictrl-regs.h
bsps\include\grlib\spwpnp-regs.h
bsps\include\grlib\spwrmap-regs.h
Change History (6)
comment:1 Changed on 01/28/23 at 17:24:25 by Sebastian Huber
| Keywords: | qualification added |
|---|---|
| Milestone: | → 7.1 |
| Owner: | set to Needs Funding |
| Status: | new → assigned |
| Version: | → 6 |
comment:2 Changed on 01/31/23 at 17:50:55 by Sebastian Huber
comment:3 Changed on 02/08/23 at 10:33:48 by Daniel Páscoa
Hello! Yes, we think this format is better, despite being longer and more extensive; this way, the logic is clearer and unambiguous.
comment:4 Changed on 03/15/23 at 07:30:30 by Sebastian Huber <sebastian.huber@…>
comment:5 Changed on 07/14/23 at 09:52:16 by Sebastian Huber
| Component: | admin → bsps |
|---|---|
| Milestone: | 7.1 → 6.1 |
| Owner: | changed from Needs Funding to Sebastian Huber |
| Status: | assigned → accepted |
| Type: | defect → enhancement |
comment:6 Changed on 07/14/23 at 10:01:06 by Sebastian Huber <sebastian.huber@…>
| Resolution: | → fixed |
|---|---|
| Status: | accepted → closed |
In 2d71cba0/rtems:
The files are generated, so changing the format would be easy. Is this the desired format?
#define AHBSTAT_AHBS_HMASTER_SHIFT 3 #define AHBSTAT_AHBS_HMASTER_MASK 0x78U #define AHBSTAT_AHBS_HMASTER_GET( _reg ) \ ( ( ( _reg ) & AHBSTAT_AHBS_HMASTER_MASK ) >> AHBSTAT_AHBS_HMASTER_SHIFT ) #define AHBSTAT_AHBS_HMASTER( _val ) ( ( _val ) << AHBSTAT_AHBS_HMASTER_SHIFT )It makes the lines longer and the files larger.