[U-Boot] [PATCH] Introduce generic TPM support in u-boot

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111010/4b1d4bb6/attachment.pgp