[U-Boot] [PATCH v2 1/2] Introduce generic TPM support in u-boot

[Mike Frysinger]

On Saturday 15 October 2011 17:09:55 Marek Vasut wrote:
> On Saturday, October 15, 2011 08:47:39 PM Vadim Bendebury wrote:
> > On Sat, Oct 15, 2011 at 11:08 AM, Marek Vasut wrote:
> > > On Saturday, October 15, 2011 05:38:50 AM Vadim Bendebury wrote:
> > >> --- /dev/null
> > >> +++ b/drivers/tpm/generic_lpc_tpm.c
> > > 
> > > [...]
> > > 
> > >> +
> > >> +struct lpc_tpm {
> > >> +     struct tpm_locality locality[TPM_TOTAL_LOCALITIES];
> > >> +};
> > > 
> > > Do you need such envelope ?
> > 
> > I think I do - this accurately describes the structure of the chip.
> 
> There's just one member ... it's weird?

i don't think so.  if the hardware is really that way, then a single struct 
that represents it makes sense to me.

> > >> +/* Error value returned on various TPM driver errors */
> > > 
> > > Dot missing at the end.
> > 
> > ok.
> 
> Please fix globally.

this isn't really a standard.  for single sentences/fragments, they often get 
dropped.  left up to the relevant writer what they want to do.

> > >> +/* TPM access going through macros to make tracing easier. */
> > >> +#define tpm_read(ptr) ({ \
> > >> +     u32  __ret; \
> > >> +     __ret = (sizeof(*ptr) == 1) ? readb(ptr) : readl(ptr); \
> > >> +      debug(PREFIX "Read reg 0x%x returns 0x%x\n", \
> > >> +            (u32)ptr - (u32)lpc_tpm_dev, __ret); \
> > >> +                                      __ret; })
> > >> +
> > > 
> > > Make this inline function
> > > 
> > >> +#define tpm_write(value, ptr) ({                \
> > >> +     u32 __v = value;        \
> > >> +     debug(PREFIX "Write reg 0x%x with 0x%x\n", \
> > >> +            (u32)ptr - (u32)lpc_tpm_dev, __v); \
> > >> +     if (sizeof(*ptr) == 1) \
> > >> +             writeb(__v, ptr); \
> > >> +     else \
> > >> +             writel(__v, ptr); })
> > >> +
> > > 
> > > DTTO
> > 
> > Are you sure these will work as inline functions?
> 
> Why not ? Also, why do you introduce the __v ?

the sizeof() magic requires it be a macro.  and the __v indirection is 
required (there should be a __p too, but that'd be a little more work, and is 
less likely to have side effects).
-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/20111016/c3b3d604/attachment.pgp