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