Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
Richard Henderson writes: > On 5/3/19 8:27 AM, Alex Bennée wrote: >> >> Yoshinori Sato 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.
On Sat, 04 May 2019 00:27:29 +0900,
Alex Bennée wrote:
>
>
> Yoshinori Sato 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
> > ---
> > 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
Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.
On 5/3/19 8:27 AM, Alex Bennée wrote: > > Yoshinori Sato 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.
Yoshinori Sato 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
> ---
> 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
