[Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.

Yoshinori Sato posted 12 patches 6 years, 9 months ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Posted by Yoshinori Sato 6 years, 9 months ago
Some RX peripheral using 8bit and 16bit registers.
Added 8bit and 16bit APIs.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 include/hw/registerfields.h | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index 2659a58737..51bfd0cf67 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
@@ -40,6 +48,8 @@
 #define FIELD_EX64(storage, reg, field)                                   \
     extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
               R_ ## reg ## _ ## field ## _LENGTH)
+#define FIELD_EX8  FIELD_EX32
+#define FIELD_EX16 FIELD_EX32
 
 /* Extract a field from an array of registers */
 #define ARRAY_FIELD_EX32(regs, reg, field)                                \
@@ -49,6 +59,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 +83,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 };                                                   \
-- 
2.11.0


Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Posted by Alex Bennée 6 years, 9 months ago
Yoshinori Sato <ysato@users.sourceforge.jp> writes:

> Some RX peripheral using 8bit and 16bit registers.
> Added 8bit and 16bit APIs.

Doesn't this mean the build breaks at some point? Features used by other
patches should be introduced first so the build remains bisectable.

>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  include/hw/registerfields.h | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> index 2659a58737..51bfd0cf67 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
> @@ -40,6 +48,8 @@
>  #define FIELD_EX64(storage, reg, field)                                   \
>      extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
>                R_ ## reg ## _ ## field ## _LENGTH)
> +#define FIELD_EX8  FIELD_EX32
> +#define FIELD_EX16 FIELD_EX32

Hmm maybe we should be defining extract16/extract8 in bitops so things
are a) properly types and b) bounds checked to catch errors.

>
>  /* Extract a field from an array of registers */
>  #define ARRAY_FIELD_EX32(regs, reg, field)                                \
> @@ -49,6 +59,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 +83,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 };                                                   \


--
Alex Bennée

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Posted by Richard Henderson 6 years, 9 months ago
On 5/3/19 8:27 AM, Alex Bennée wrote:
> 
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
>> Some RX peripheral using 8bit and 16bit registers.
>> Added 8bit and 16bit APIs.
> 
> Doesn't this mean the build breaks at some point? Features used by other
> patches should be introduced first so the build remains bisectable.

The only bug I would fix in the ordering is to make the change to configure
last, so that the target/rx is not enabled while the patches are staging.


r~

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Posted by Alex Bennée 6 years, 9 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 5/3/19 8:27 AM, Alex Bennée wrote:
>>
>> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
>>
>>> Some RX peripheral using 8bit and 16bit registers.
>>> Added 8bit and 16bit APIs.
>>
>> Doesn't this mean the build breaks at some point? Features used by other
>> patches should be introduced first so the build remains bisectable.
>
> The only bug I would fix in the ordering is to make the change to configure
> last, so that the target/rx is not enabled while the patches are
> staging.

That will do - the key thing is each interim position in the commit log
can build on it's own.

--
Alex Bennée

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Posted by Yoshinori Sato 6 years, 9 months ago
On Sat, 04 May 2019 00:27:29 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
> > Some RX peripheral using 8bit and 16bit registers.
> > Added 8bit and 16bit APIs.
> 
> Doesn't this mean the build breaks at some point? Features used by other
> patches should be introduced first so the build remains bisectable.

Hmm, It changes only added new macros.
So don't broken this changes.

> >
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > ---
> >  include/hw/registerfields.h | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> > index 2659a58737..51bfd0cf67 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
> > @@ -40,6 +48,8 @@
> >  #define FIELD_EX64(storage, reg, field)                                   \
> >      extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
> >                R_ ## reg ## _ ## field ## _LENGTH)
> > +#define FIELD_EX8  FIELD_EX32
> > +#define FIELD_EX16 FIELD_EX32
> 
> Hmm maybe we should be defining extract16/extract8 in bitops so things
> are a) properly types and b) bounds checked to catch errors.

I think so.
I will added extrat8 and extract16 in bitops.h.

> >
> >  /* Extract a field from an array of registers */
> >  #define ARRAY_FIELD_EX32(regs, reg, field)                                \
> > @@ -49,6 +59,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 +83,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 };                                                   \
> 
> 
> --
> Alex Bennée
> 

-- 
Yosinori Sato