[PATCH 2/2] bsps/powerpc: Fix warnings with PPC_SPECIAL_PURPOSE_REGISTER

Chris Johns chrisj at rtems.org
Tue Apr 11 22:34:46 UTC 2023


On 11/4/2023 10:29 pm, Joel Sherrill wrote:
> Anything to add to the message beyond "fix" to explain this patch? :)
> 
> I think the software engineering guide uses a comment similar to this as an
> anti-pattern. Lol

How about:

- This change avoids the GCC extension of blocks in expressions that
  ISO forbids and is warned about with the pedantic warnings option.

?

Chris

> 
> On Tue, Apr 11, 2023, 12:00 AM <chrisj at rtems.org <mailto:chrisj at rtems.org>> wrote:
> 
>     From: Chris Johns <chrisj at rtems.org <mailto:chrisj at rtems.org>>
> 
>     ---
>      bsps/powerpc/include/libcpu/powerpc-utility.h | 40 +++++++++++--------
>      .../powerpc/shared/exceptions/ppc_exc_print.c | 25 +++++++-----
>      2 files changed, 38 insertions(+), 27 deletions(-)
> 
>     diff --git a/bsps/powerpc/include/libcpu/powerpc-utility.h
>     b/bsps/powerpc/include/libcpu/powerpc-utility.h
>     index 922e5d2407..fb831c9fa8 100644
>     --- a/bsps/powerpc/include/libcpu/powerpc-utility.h
>     +++ b/bsps/powerpc/include/libcpu/powerpc-utility.h
>     @@ -577,15 +577,11 @@ static inline void
>     ppc_set_decrementer_register(uint32_t dec)
>       *
>       * @note This macro uses a GNU C extension.
>       */
>     -#define PPC_SPECIAL_PURPOSE_REGISTER(spr) \
>     -  ({ \
>     -    uint32_t val; \
>     -    __asm__ volatile (\
>     -      "mfspr %0, " PPC_STRINGOF(spr) \
>     -      : "=r" (val) \
>     -    ); \
>     -    val;\
>     -  } )
>     +#define PPC_SPECIAL_PURPOSE_REGISTER(spr, val) \
>     +  __asm__ volatile (\
>     +    "mfspr %0, " PPC_STRINGOF(spr) \
>     +    : "=r" (val) \
>     +  )
> 
>      /**
>       * @brief Sets the Special Purpose Register with number @a spr to the value in
>     @@ -612,7 +608,7 @@ static inline void ppc_set_decrementer_register(uint32_t
>     dec)
>          uint32_t val; \
>          uint32_t mybits = bits; \
>          _ISR_Local_disable(level); \
>     -    val = PPC_SPECIAL_PURPOSE_REGISTER(spr); \
>     +    PPC_SPECIAL_PURPOSE_REGISTER(spr, val); \
>          val |= mybits; \
>          PPC_SET_SPECIAL_PURPOSE_REGISTER(spr, val); \
>          _ISR_Local_enable(level); \
>     @@ -632,7 +628,7 @@ static inline void ppc_set_decrementer_register(uint32_t
>     dec)
>          uint32_t mybits = bits; \
>          uint32_t mymask = mask; \
>          _ISR_Local_disable(level); \
>     -    val = PPC_SPECIAL_PURPOSE_REGISTER(spr); \
>     +    PPC_SPECIAL_PURPOSE_REGISTER(spr, val); \
>          val &= ~mymask; \
>          val |= mybits; \
>          PPC_SET_SPECIAL_PURPOSE_REGISTER(spr, val); \
>     @@ -651,7 +647,7 @@ static inline void ppc_set_decrementer_register(uint32_t
>     dec)
>          uint32_t val; \
>          uint32_t mybits = bits; \
>          _ISR_Local_disable(level); \
>     -    val = PPC_SPECIAL_PURPOSE_REGISTER(spr); \
>     +    PPC_SPECIAL_PURPOSE_REGISTER(spr, val); \
>          val &= ~mybits; \
>          PPC_SET_SPECIAL_PURPOSE_REGISTER(spr, val); \
>          _ISR_Local_enable(level); \
>     @@ -790,7 +786,9 @@ static inline void ppc_set_time_base(uint32_t val)
> 
>      static inline uint32_t ppc_time_base_upper(void)
>      {
>     -  return PPC_SPECIAL_PURPOSE_REGISTER(TBRU);
>     +  uint32_t val;
>     +  PPC_SPECIAL_PURPOSE_REGISTER(TBRU, val);
>     +  return val;
>      }
> 
>      static inline void ppc_set_time_base_upper(uint32_t val)
>     @@ -810,12 +808,16 @@ static inline void ppc_set_time_base_64(uint64_t val)
> 
>      static inline uint32_t ppc_alternate_time_base(void)
>      {
>     -  return PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_ATBL);
>     +  uint32_t val;
>     +  PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_ATBL, val);
>     +  return val;
>      }
> 
>      static inline uint32_t ppc_alternate_time_base_upper(void)
>      {
>     -  return PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_ATBU);
>     +  uint32_t val;
>     +  PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_ATBU, val);
>     +  return val;
>      }
> 
>      static inline uint64_t ppc_alternate_time_base_64(void)
>     @@ -835,7 +837,9 @@ static inline uint64_t ppc_alternate_time_base_64(void)
> 
>      static inline uint32_t ppc_processor_id(void)
>      {
>     -  return PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_PIR);
>     +  uint32_t val;
>     +  PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_PIR, val);
>     +  return val;
>      }
> 
>      static inline void ppc_set_processor_id(uint32_t val)
>     @@ -845,7 +849,9 @@ static inline void ppc_set_processor_id(uint32_t val)
> 
>      static inline uint32_t ppc_fsl_system_version(void)
>      {
>     -  return PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_SVR);
>     +  uint32_t val;
>     +  PPC_SPECIAL_PURPOSE_REGISTER(FSL_EIS_SVR, val);
>     +  return val;
>      }
> 
>      static inline uint32_t ppc_fsl_system_version_cid(uint32_t svr)
>     diff --git a/bsps/powerpc/shared/exceptions/ppc_exc_print.c
>     b/bsps/powerpc/shared/exceptions/ppc_exc_print.c
>     index e4fcc73cb1..ff231beff9 100644
>     --- a/bsps/powerpc/shared/exceptions/ppc_exc_print.c
>     +++ b/bsps/powerpc/shared/exceptions/ppc_exc_print.c
>     @@ -42,18 +42,23 @@ typedef struct LRFrameRec_ {
> 
>      static uint32_t ppc_exc_get_DAR_dflt(void)
>      {
>     -  if (ppc_cpu_is_60x())
>     -    return PPC_SPECIAL_PURPOSE_REGISTER(PPC_DAR);
>     -  else
>     +  uint32_t val;
>     +  if (ppc_cpu_is_60x()) {
>     +    PPC_SPECIAL_PURPOSE_REGISTER(PPC_DAR, val);
>     +    return val;
>     +  } else {
>          switch (ppc_cpu_is_bookE()) {
>            default:
>              break;
>            case PPC_BOOKE_STD:
>            case PPC_BOOKE_E500:
>     -        return PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_DEAR);
>     +        PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_DEAR, val);
>     +        return val;
>            case PPC_BOOKE_405:
>     -        return PPC_SPECIAL_PURPOSE_REGISTER(PPC405_DEAR);
>     +        PPC_SPECIAL_PURPOSE_REGISTER(PPC405_DEAR, val);
>     +        return val;
>          }
>     +  }
>        return 0xdeadbeef;
>      }
> 
>     @@ -170,13 +175,13 @@ void _CPU_Exception_frame_print(const
>     CPU_Exception_frame *excPtr)
>          printk(" %s = 0x%08" PRIx32 "\n", reg, ppc_exc_get_DAR());
>        }
>        if (ppc_cpu_is_bookE()) {
>     -    unsigned esr, mcsr;
>     +    uint32_t esr, mcsr;
>          if (ppc_cpu_is_bookE() == PPC_BOOKE_405) {
>     -      esr  = PPC_SPECIAL_PURPOSE_REGISTER(PPC405_ESR);
>     -      mcsr = PPC_SPECIAL_PURPOSE_REGISTER(PPC405_MCSR);
>     +      PPC_SPECIAL_PURPOSE_REGISTER(PPC405_ESR, esr);
>     +      PPC_SPECIAL_PURPOSE_REGISTER(PPC405_MCSR, mcsr);
>          } else {
>     -      esr  = PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_ESR);
>     -      mcsr = PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_MCSR);
>     +      PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_ESR, esr);
>     +      PPC_SPECIAL_PURPOSE_REGISTER(BOOKE_MCSR, mcsr);
>          }
>          printk("  ESR = 0x%08x\n", esr);
>          printk(" MCSR = 0x%08x\n", mcsr);
>     -- 
>     2.37.1
> 
>     _______________________________________________
>     devel mailing list
>     devel at rtems.org <mailto:devel at rtems.org>
>     http://lists.rtems.org/mailman/listinfo/devel
>     <http://lists.rtems.org/mailman/listinfo/devel>
> 


More information about the devel mailing list