diff mbox series

[v13,10/12] hw/registerfields.h: Add 8bit and16bit register macros.

Message ID 20190516055244.95559-11-ysato@users.sourceforge.jp
State New
Headers show
Series Add RX archtecture support | expand

Commit Message

Yoshinori Sato May 16, 2019, 5:52 a.m. UTC 
Some RX peripheral using 8bit and 16bit registers.
Added 8bit and 16bit APIs.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/hw/registerfields.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé May 16, 2019, 12:47 p.m. UTC | #1 
Hi Yoshinori,

On 5/16/19 7:52 AM, Yoshinori Sato wrote:
> Some RX peripheral using 8bit and 16bit registers.
> Added 8bit and 16bit APIs.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/hw/registerfields.h | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> index 2659a58737..a0bb0654d6 100644
> --- a/include/hw/registerfields.h
> +++ b/include/hw/registerfields.h
> @@ -22,6 +22,14 @@
>      enum { A_ ## reg = (addr) };                                          \
>      enum { R_ ## reg = (addr) / 4 };
>  
> +#define REG8(reg, addr)                                                  \
> +    enum { A_ ## reg = (addr) };                                          \
> +    enum { R_ ## reg = (addr) };

This macro is used in your devices (previous patches) and we can not
build the previous patches without this one:

  CC      rx-softmmu/hw/char/renesas_sci.o
./hw/char/renesas_sci.c:34:10: error: expected ‘)’ before numeric constant
 REG8(SMR, 0)
          ^~
          )
./hw/char/renesas_sci.c:42:10: error: expected ‘)’ before numeric constant
 REG8(BRR, 1)
          ^~
          )
./hw/char/renesas_sci.c:51:10: error: expected ‘)’ before numeric constant
 REG8(TDR, 3)
          ^~
          )
./hw/char/renesas_sci.c:62:10: error: expected ‘)’ before numeric constant
 REG8(RDR, 5)
          ^~
          )
./hw/char/renesas_sci.c:68:11: error: expected ‘)’ before numeric constant
 REG8(SEMR, 7)
           ^~
           )
./hw/char/renesas_sci.c: In function ‘can_receive’:
./hw/char/renesas_sci.c:78:16: error: implicit declaration of function
‘FIELD_EX8’; did you mean ‘FIELD_EX64’?
[-Werror=implicit-function-declaration]
         return FIELD_EX8(sci->scr, SCR, RE);
                ^~~~~~~~~
                FIELD_EX64
./hw/char/renesas_sci.c:78:16: error: nested extern declaration of
‘FIELD_EX8’ [-Werror=nested-externs]
./hw/char/renesas_sci.c:78:36: error: ‘SCR’ undeclared (first use in
this function)
         return FIELD_EX8(sci->scr, SCR, RE);
                                    ^~~
./hw/char/renesas_sci.c:78:36: note: each undeclared identifier is
reported only once for each function it appears in
./hw/char/renesas_sci.c:78:41: error: ‘RE’ undeclared (first use in this
function)
         return FIELD_EX8(sci->scr, SCR, RE);
                                         ^~
./hw/char/renesas_sci.c: In function ‘receive’:
./hw/char/renesas_sci.c:86:29: error: ‘SSR’ undeclared (first use in
this function)
     if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
                             ^~~
./hw/char/renesas_sci.c:86:34: error: ‘RDRF’ undeclared (first use in
this function)
     if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
                                  ^~~~
./hw/char/renesas_sci.c:87:20: error: implicit declaration of function
‘FIELD_DP8’; did you mean ‘FIELD_DP32’?
[-Werror=implicit-function-declaration]
         sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
                    ^~~~~~~~~
                    FIELD_DP32
./hw/char/renesas_sci.c:87:20: error: nested extern declaration of
‘FIELD_DP8’ [-Werror=nested-externs]
./hw/char/renesas_sci.c:87:45: error: ‘ORER’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
                                             ^~~~
./hw/char/renesas_sci.c:88:33: error: ‘SCR’ undeclared (first use in
this function)
         if (FIELD_EX8(sci->scr, SCR, RIE)) {
                                 ^~~
./hw/char/renesas_sci.c:88:38: error: ‘RIE’ undeclared (first use in
this function); did you mean ‘PIE’?
         if (FIELD_EX8(sci->scr, SCR, RIE)) {
                                      ^~~
                                      PIE
./hw/char/renesas_sci.c: In function ‘send_byte’:
./hw/char/renesas_sci.c:107:36: error: ‘SSR’ undeclared (first use in
this function)
     sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 0);
                                    ^~~
./hw/char/renesas_sci.c:107:41: error: ‘TEND’ undeclared (first use in
this function); did you mean ‘TEI’?
     sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 0);
                                         ^~~~
                                         TEI
./hw/char/renesas_sci.c:108:41: error: ‘TDRE’ undeclared (first use in
this function); did you mean ‘TRUE’?
     sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 1);
                                         ^~~~
                                         TRUE
./hw/char/renesas_sci.c:110:29: error: ‘SCR’ undeclared (first use in
this function)
     if (FIELD_EX8(sci->scr, SCR, TIE)) {
                             ^~~
./hw/char/renesas_sci.c:110:34: error: ‘TIE’ undeclared (first use in
this function); did you mean ‘PIE’?
     if (FIELD_EX8(sci->scr, SCR, TIE)) {
                                  ^~~
                                  PIE
./hw/char/renesas_sci.c: In function ‘txend’:
./hw/char/renesas_sci.c:118:30: error: ‘SSR’ undeclared (first use in
this function)
     if (!FIELD_EX8(sci->ssr, SSR, TDRE)) {
                              ^~~
./hw/char/renesas_sci.c:118:35: error: ‘TDRE’ undeclared (first use in
this function); did you mean ‘TRUE’?
     if (!FIELD_EX8(sci->ssr, SSR, TDRE)) {
                                   ^~~~
                                   TRUE
./hw/char/renesas_sci.c:121:45: error: ‘TEND’ undeclared (first use in
this function); did you mean ‘TEI’?
         sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 1);
                                             ^~~~
                                             TEI
./hw/char/renesas_sci.c:122:33: error: ‘SCR’ undeclared (first use in
this function)
         if (FIELD_EX8(sci->scr, SCR, TEIE)) {
                                 ^~~
./hw/char/renesas_sci.c:122:38: error: ‘TEIE’ undeclared (first use in
this function); did you mean ‘TEI’?
         if (FIELD_EX8(sci->scr, SCR, TEIE)) {
                                      ^~~~
                                      TEI
./hw/char/renesas_sci.c: In function ‘update_trtime’:
./hw/char/renesas_sci.c:131:43: error: ‘SMR’ undeclared (first use in
this function)
     sci->trtime = 8 - FIELD_EX8(sci->smr, SMR, CHR);
                                           ^~~
./hw/char/renesas_sci.c:131:48: error: ‘CHR’ undeclared (first use in
this function)
     sci->trtime = 8 - FIELD_EX8(sci->smr, SMR, CHR);
                                                ^~~
./hw/char/renesas_sci.c:132:45: error: ‘PE’ undeclared (first use in
this function); did you mean ‘PIE’?
     sci->trtime += FIELD_EX8(sci->smr, SMR, PE);
                                             ^~
                                             PIE
./hw/char/renesas_sci.c:133:45: error: ‘STOP’ undeclared (first use in
this function)
     sci->trtime += FIELD_EX8(sci->smr, SMR, STOP) + 1;
                                             ^~~~
./hw/char/renesas_sci.c:136:55: error: ‘CKS’ undeclared (first use in
this function)
     sci->trtime *= 1 << (2 * FIELD_EX8(sci->smr, SMR, CKS));
                                                       ^~~
./hw/char/renesas_sci.c: In function ‘sci_write’:
./hw/char/renesas_sci.c:150:10: error: ‘A_SMR’ undeclared (first use in
this function); did you mean ‘AF_SMC’?
     case A_SMR:
          ^~~~~
          AF_SMC
./hw/char/renesas_sci.c:142:21: error: ‘SCR’ undeclared (first use in
this function)
     (FIELD_EX8(scr, SCR, TE) || FIELD_EX8(scr, SCR, RE))
                     ^~~
./hw/char/renesas_sci.c:151:14: note: in expansion of macro ‘IS_TR_ENABLED’
         if (!IS_TR_ENABLED(sci->scr)) {
              ^~~~~~~~~~~~~
./hw/char/renesas_sci.c:142:26: error: ‘TE’ undeclared (first use in
this function); did you mean ‘TEI’?
     (FIELD_EX8(scr, SCR, TE) || FIELD_EX8(scr, SCR, RE))
                          ^~
./hw/char/renesas_sci.c:151:14: note: in expansion of macro ‘IS_TR_ENABLED’
         if (!IS_TR_ENABLED(sci->scr)) {
              ^~~~~~~~~~~~~
./hw/char/renesas_sci.c:142:53: error: ‘RE’ undeclared (first use in
this function)
     (FIELD_EX8(scr, SCR, TE) || FIELD_EX8(scr, SCR, RE))
                                                     ^~
./hw/char/renesas_sci.c:151:14: note: in expansion of macro ‘IS_TR_ENABLED’
         if (!IS_TR_ENABLED(sci->scr)) {
              ^~~~~~~~~~~~~
./hw/char/renesas_sci.c:156:10: error: ‘A_BRR’ undeclared (first use in
this function)
     case A_BRR:
          ^~~~~
./hw/char/renesas_sci.c:162:10: error: ‘A_SCR’ undeclared (first use in
this function)
     case A_SCR:
          ^~~~~
./hw/char/renesas_sci.c:165:44: error: ‘SSR’ undeclared (first use in
this function)
             sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 1);
                                            ^~~
./hw/char/renesas_sci.c:165:49: error: ‘TDRE’ undeclared (first use in
this function); did you mean ‘TRUE’?
             sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 1);
                                                 ^~~~
                                                 TRUE
./hw/char/renesas_sci.c:166:49: error: ‘TEND’ undeclared (first use in
this function); did you mean ‘TEI’?
             sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 1);
                                                 ^~~~
                                                 TEI
./hw/char/renesas_sci.c:167:42: error: ‘TIE’ undeclared (first use in
this function); did you mean ‘PIE’?
             if (FIELD_EX8(sci->scr, SCR, TIE)) {
                                          ^~~
                                          PIE
./hw/char/renesas_sci.c:171:39: error: ‘TEIE’ undeclared (first use in
this function); did you mean ‘TEI’?
         if (!FIELD_EX8(sci->scr, SCR, TEIE)) {
                                       ^~~~
                                       TEI
./hw/char/renesas_sci.c:174:39: error: ‘RIE’ undeclared (first use in
this function); did you mean ‘PIE’?
         if (!FIELD_EX8(sci->scr, SCR, RIE)) {
                                       ^~~
                                       PIE
./hw/char/renesas_sci.c:178:10: error: ‘A_TDR’ undeclared (first use in
this function)
     case A_TDR:
          ^~~~~
./hw/char/renesas_sci.c:186:10: error: ‘A_SSR’ undeclared (first use in
this function); did you mean ‘A_PSW’?
     case A_SSR:
          ^~~~~
          A_PSW
./hw/char/renesas_sci.c:187:45: error: ‘MPBT’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, MPBT,
                                             ^~~~
./hw/char/renesas_sci.c:189:45: error: ‘ERR’ undeclared (first use in
this function); did you mean ‘ERI’?
         sci->ssr = FIELD_DP8(sci->ssr, SSR, ERR,
                                             ^~~
                                             ERI
./hw/char/renesas_sci.c:196:10: error: ‘A_RDR’ undeclared (first use in
this function); did you mean ‘O_RDWR’?
     case A_RDR:
          ^~~~~
          O_RDWR
./hw/char/renesas_sci.c:199:10: error: ‘A_SCMR’ undeclared (first use in
this function); did you mean ‘S_ISCHR’?
     case A_SCMR:
          ^~~~~~
          S_ISCHR
./hw/char/renesas_sci.c:201:10: error: ‘A_SEMR’ undeclared (first use in
this function); did you mean ‘AF_SMC’?
     case A_SEMR: /* SEMR */
          ^~~~~~
          AF_SMC
./hw/char/renesas_sci.c: In function ‘sci_read’:
./hw/char/renesas_sci.c:215:10: error: ‘A_SMR’ undeclared (first use in
this function); did you mean ‘AF_SMC’?
     case A_SMR:
          ^~~~~
          AF_SMC
./hw/char/renesas_sci.c:217:10: error: ‘A_BRR’ undeclared (first use in
this function)
     case A_BRR:
          ^~~~~
./hw/char/renesas_sci.c:219:10: error: ‘A_SCR’ undeclared (first use in
this function)
     case A_SCR:
          ^~~~~
./hw/char/renesas_sci.c:221:10: error: ‘A_TDR’ undeclared (first use in
this function)
     case A_TDR:
          ^~~~~
./hw/char/renesas_sci.c:223:10: error: ‘A_SSR’ undeclared (first use in
this function); did you mean ‘A_PSW’?
     case A_SSR:
          ^~~~~
          A_PSW
./hw/char/renesas_sci.c:226:10: error: ‘A_RDR’ undeclared (first use in
this function); did you mean ‘O_RDWR’?
     case A_RDR:
          ^~~~~
          O_RDWR
./hw/char/renesas_sci.c:227:40: error: ‘SSR’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 0);
                                        ^~~
./hw/char/renesas_sci.c:227:45: error: ‘RDRF’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 0);
                                             ^~~~
./hw/char/renesas_sci.c:229:10: error: ‘A_SCMR’ undeclared (first use in
this function); did you mean ‘S_ISCHR’?
     case A_SCMR:
          ^~~~~~
          S_ISCHR
./hw/char/renesas_sci.c:231:10: error: ‘A_SEMR’ undeclared (first use in
this function); did you mean ‘AF_SMC’?
     case A_SEMR:
          ^~~~~~
          AF_SMC
./hw/char/renesas_sci.c: In function ‘sci_event’:
./hw/char/renesas_sci.c:266:40: error: ‘SSR’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, FER, 1);
                                        ^~~
./hw/char/renesas_sci.c:266:45: error: ‘FER’ undeclared (first use in
this function)
         sci->ssr = FIELD_DP8(sci->ssr, SSR, FER, 1);
                                             ^~~
./hw/char/renesas_sci.c:267:33: error: ‘SCR’ undeclared (first use in
this function)
         if (FIELD_EX8(sci->scr, SCR, RIE)) {
                                 ^~~
./hw/char/renesas_sci.c:267:38: error: ‘RIE’ undeclared (first use in
this function); did you mean ‘PIE’?
         if (FIELD_EX8(sci->scr, SCR, RIE)) {
                                      ^~~
                                      PIE
./hw/char/renesas_sci.c: In function ‘can_receive’:
./hw/char/renesas_sci.c:80:1: error: control reaches end of non-void
function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors

I already commented that in a previous review.

> +
> +#define REG16(reg, addr)                                                  \
> +    enum { A_ ## reg = (addr) };                                          \
> +    enum { R_ ## reg = (addr) / 2 };

I wonder why you didn't include those macros before REG32 to keep them
ordered, not a big deal.

> +
>  /* Define SHIFT, LENGTH and MASK constants for a field within a register */
>  
>  /* This macro will define R_FOO_BAR_MASK, R_FOO_BAR_SHIFT and R_FOO_BAR_LENGTH
> @@ -34,6 +42,12 @@
>                                          MAKE_64BIT_MASK(shift, length)};
>  
>  /* Extract a field from a register */
> +#define FIELD_EX8(storage, reg, field)                                    \
> +    extract8((storage), R_ ## reg ## _ ## field ## _SHIFT,                \

Then this function ...

> +              R_ ## reg ## _ ## field ## _LENGTH)
> +#define FIELD_EX16(storage, reg, field)                                   \
> +    extract16((storage), R_ ## reg ## _ ## field ## _SHIFT,               \

... and this one are added in the next patch, so you series is not
bisectable:

  CC      rx-softmmu/hw/char/renesas_sci.o
In file included from ./target/rx/cpu.h:24,
                 from ./hw/char/renesas_sci.c:26:
./hw/char/renesas_sci.c: In function ‘can_receive’:
./include/hw/registerfields.h:46:5: error: implicit declaration of
function ‘extract8’; did you mean ‘extract64’?
[-Werror=implicit-function-declaration]
     extract8((storage), R_ ## reg ## _ ## field ## _SHIFT,                \
     ^~~~~~~~
./hw/char/renesas_sci.c:78:16: note: in expansion of macro ‘FIELD_EX8’
         return FIELD_EX8(sci->scr, SCR, RE);
                ^~~~~~~~~
./include/hw/registerfields.h:46:5: error: nested extern declaration of
‘extract8’ [-Werror=nested-externs]
     extract8((storage), R_ ## reg ## _ ## field ## _SHIFT,                \
     ^~~~~~~~
./hw/char/renesas_sci.c:78:16: note: in expansion of macro ‘FIELD_EX8’
         return FIELD_EX8(sci->scr, SCR, RE);
                ^~~~~~~~~
cc1: all warnings being treated as errors

Richard: Can you reorder the series with:

- patch 11 first "Add extract8 and extract16"
- patch 10 then "Add 8bit and 16bit register macros"
  (while here, can you strip the trailing dot in patch subject?)
- rest of this series, patches 1...9
- finally patch 12 (personally I'd have squashed this one
  in patch 9 "Add rx-softmmu")

> +              R_ ## reg ## _ ## field ## _LENGTH)
>  #define FIELD_EX32(storage, reg, field)                                   \
>      extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
>                R_ ## reg ## _ ## field ## _LENGTH)
> @@ -49,6 +63,22 @@
>   * Assigning values larger then the target field will result in
>   * compilation warnings.
>   */
> +#define FIELD_DP8(storage, reg, field, val) ({                            \
> +    struct {                                                              \
> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
> +    } v = { .v = val };                                                   \
> +    uint8_t d;                                                            \
> +    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
> +    d; })
> +#define FIELD_DP16(storage, reg, field, val) ({                           \
> +    struct {                                                              \
> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
> +    } v = { .v = val };                                                   \
> +    uint16_t d;                                                           \
> +    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
> +    d; })
>  #define FIELD_DP32(storage, reg, field, val) ({                           \
>      struct {                                                              \
>          unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
> @@ -57,7 +87,7 @@
>      d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
>                    R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
>      d; })
> -#define FIELD_DP64(storage, reg, field, val) ({                           \
> +#define FIELD_DP64(storage, reg, field, val) ({                         \

I suppose this is change is a mistake. Richard, do you mind dropping it?

>      struct {                                                              \
>          unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
>      } v = { .v = val };                                                   \
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index 2659a58737..a0bb0654d6 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -22,6 +22,14 @@ 
     enum { A_ ## reg = (addr) };                                          \
     enum { R_ ## reg = (addr) / 4 };
 
+#define REG8(reg, addr)                                                  \
+    enum { A_ ## reg = (addr) };                                          \
+    enum { R_ ## reg = (addr) };
+
+#define REG16(reg, addr)                                                  \
+    enum { A_ ## reg = (addr) };                                          \
+    enum { R_ ## reg = (addr) / 2 };
+
 /* Define SHIFT, LENGTH and MASK constants for a field within a register */
 
 /* This macro will define R_FOO_BAR_MASK, R_FOO_BAR_SHIFT and R_FOO_BAR_LENGTH
@@ -34,6 +42,12 @@ 
                                         MAKE_64BIT_MASK(shift, length)};
 
 /* Extract a field from a register */
+#define FIELD_EX8(storage, reg, field)                                    \
+    extract8((storage), R_ ## reg ## _ ## field ## _SHIFT,                \
+              R_ ## reg ## _ ## field ## _LENGTH)
+#define FIELD_EX16(storage, reg, field)                                   \
+    extract16((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
+              R_ ## reg ## _ ## field ## _LENGTH)
 #define FIELD_EX32(storage, reg, field)                                   \
     extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
               R_ ## reg ## _ ## field ## _LENGTH)
@@ -49,6 +63,22 @@ 
  * Assigning values larger then the target field will result in
  * compilation warnings.
  */
+#define FIELD_DP8(storage, reg, field, val) ({                            \
+    struct {                                                              \
+        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
+    } v = { .v = val };                                                   \
+    uint8_t d;                                                            \
+    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
+                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
+    d; })
+#define FIELD_DP16(storage, reg, field, val) ({                           \
+    struct {                                                              \
+        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
+    } v = { .v = val };                                                   \
+    uint16_t d;                                                           \
+    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
+                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
+    d; })
 #define FIELD_DP32(storage, reg, field, val) ({                           \
     struct {                                                              \
         unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
@@ -57,7 +87,7 @@ 
     d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
                   R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
     d; })
-#define FIELD_DP64(storage, reg, field, val) ({                           \
+#define FIELD_DP64(storage, reg, field, val) ({                         \
     struct {                                                              \
         unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
     } v = { .v = val };                                                   \