On Monday 10 October 2011 16:00:18 Vadim Bendebury wrote:
> On Mon, Oct 10, 2011 at 3:45 AM, Wolfgang Denk wrote:
> > > +#define TIS_REG(LOCALITY, REG) \
> > > + (void *)(CONFIG_TPM_TIS_BASE_ADDRESS + (LOCALITY << 12) + REG)
> >
> > We do not allow to access device registers through base address +
> > offset. Please always use C structs instead.
>
> so, this chip has five different areas (localities) which have the
> same structure, and are mapped at certain regular offsets inside the
> chip.
>
> thus the structure describing the chip would be something like
>
> struct locality {
> u16 field_a;
> u8 field_b;
> u32 field_c;
> ..
> u8 padding[<padding size>];
> } __packed;
>
> struct tmp_chip {
> struct locality localities[5];
> } __packed;
>
>
> which is very compiler dependent, but probably not the only place in
> u-boot, so I could live with that if you could.
yes, we deal with this in many places. the pseudo C structs you propose above
sound fine to me.
i'm hoping that pseudo locality struct doesn't reflect reality though as
field_c
is going to be unaligned by 1 byte :).
> Yet another inconvenience though is the requirement to be able to
> trace accesses to the registers. Some of the registers can be accessed
> in 32 bit mode or 8 bit mode, and this determines how many bytes get
> sent into/read from a data fifo. The tracing function should be able
> to tell what mode the register was accessed in. A way I see to do it
> through a structure layout is to define the same fields through
> unions.
i think unions are fine. i've done this before like:
union {
u8 reg8;
u16 reg16;
u32 reg32;
};
and then the code just uses the relevant one based on its needs
> What I am getting at is that the code is much better readable as it is
> now even though it is in violation of the 'use structure to access
> registers' requirement.
every conversion i've seen so far from base+reg offsets was much more readable
(imo) when converted to C structs. i'm not seeing why this tpm code would be
any different ?
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
