Re: [PATCH] hw/arm/stm32f405: Add preliminary RCC emulation support

[Alistair Francis]

On Thu, Mar 5, 2020 at 4:48 PM Stephanos Ioannidis <address@hidden> wrote:
>
> > Is it portable, though? I thought C bitfield order and packing was 
> > implementation-defined, which would mean that you can't guarantee that this 
> > union will give you the required thing in the uint32_t half.
>
> They are indeed implementation defined. As for packing, it is often 
> controllable; as for order, I do not recall seeing a compiler that dares to 
> change the order by which the fields are specified; though
> I understand the concern if QEMU policy is to be as portable as possible.
>
> Does QEMU have any form of bit field manipulation macros (e.g. 
> GET_BIT_RANGE(a, b), SET_BIT_RANGE(a, b), ...)?

The register fields that I pointed out earlier should do what you want.

You can look at other devices and see what they do.

Alistair

>
> -----Original Message-----
> From: Peter Maydell <address@hidden>
> Sent: Friday, March 6, 2020 4:30 AM
> To: Alistair Francis <address@hidden>
> Cc: Stephanos Ioannidis <address@hidden>; Alistair Francis <address@hidden>; 
> open list:All patches CC here <address@hidden>; open list:ARM TCG CPUs 
> <address@hidden>
> Subject: Re: [PATCH] hw/arm/stm32f405: Add preliminary RCC emulation support
>
> On Thu, 5 Mar 2020 at 19:24, Alistair Francis <address@hidden> wrote:
> >
> > On Sat, Feb 29, 2020 at 6:12 AM Stephanos Ioannidis <address@hidden> wrote:
>
> > > +typedef union {
> > > +    struct {
> > > +        uint32_t hsion : 1;
> > > +        uint32_t hsirdy : 1;
> > > +        uint32_t reserved0 : 1;
> > > +        uint32_t hsitrim : 5;
> > > +        uint32_t hsical : 8;
> > > +        uint32_t hseon : 1;
> > > +        uint32_t hserdy : 1;
> > > +        uint32_t hsebyp : 1;
> > > +        uint32_t csson : 1;
> > > +        uint32_t reserved1 : 4;
> > > +        uint32_t pllon : 1;
> > > +        uint32_t pllrdy : 1;
> > > +        uint32_t plli2son : 1;
> > > +        uint32_t plli2srdy : 1;
> > > +        uint32_t reserved2 : 4;
> > > +    };
> > > +    uint32_t reg;
> > > +} RccCrType;
> >
> > This is a pretty interesting way to represent the registers
>
> Is it portable, though? I thought C bitfield order and packing was 
> implementation-defined, which would mean that you can't guarantee that this 
> union will give you the required thing in the uint32_t half.
>
> I think it would be better to do this the way that other device models do it, 
> and avoid bitfields.
>
> thanks
> -- PMM