Dear Mathias leblanc,
In message 1365429818-2090-1-git-send-email-mathias.leblanc@st.com you wrote:
- STMicroelectronics version 1.2.0, Copyright (C) 2013
Who exactly is the copyright holder?
- STMicroelectronics comes with ABSOLUTELY NO WARRANTY.
- This is free software, and you are welcome to redistribute it
- under certain conditions.
And these would be what exactly?
Sorry, this makes no sense. Such statements must be precise to the finest detail , and they belong into the code, and not into the commit message.
In the next step, please run your patch through checkpatch and fix the reported issues.
Please make sure to get rid of all these "__attribute__ ((packed))" stuff. Make sure that structs are properly aligned, and use explicit conversion where and if necessary.
diff --git a/Makefile b/Makefile index 12763ce..ef954a4 100644 --- a/Makefile +++ b/Makefile @@ -314,7 +314,7 @@ endif LIBS-y += drivers/rtc/librtc.o LIBS-y += drivers/serial/libserial.o LIBS-y += drivers/sound/libsound.o -LIBS-$(CONFIG_GENERIC_LPC_TPM) += drivers/tpm/libtpm.o +LIBS-$(CONFIG_TPM) += drivers/tpm/libtpm.o
If you change this, you must also update the realted documentation (in the README) and _all_ related code (I think you missed at least include/configs/coreboot.h).
ALso please make sure to add the documentation to the proper groups, i. e. CONFIG_CMD_TPM should be added to the CONFIG_CMD_* group.
- if (tis_init()) {
puts("tis_init() failed!\n");return -1;- }
- if (tis_open()) {
puts("tis_open() failed!\n");return -1;
Can we have some better diagnostics, for example including the rason for the failure?
- rv = tis_sendrecv(Startup, sizeof(Startup), &response, &rlength);
- if (rv) {
printf("tpm test Startup failed\n");CHECK(tis_close());- }
- rv = tis_sendrecv(SelfTestFull, sizeof(SelfTestFull), &response,
Startup, SelfTestFull, ...
We do not allow CamelCase indentifiers.
+U_BOOT_CMD(tpm_hash, MAX_TRANSACTION_SIZE, 1, do_tpm_hash,
...
+U_BOOT_CMD(tpm_get_flag, 1, 1, do_tpm_get_flag,
Instead of adding a number of different commands, this should be implemented as sub-commands under a common one, i. e. make this:
tpm_hash --> tpm hash ... tpm_get_flag --> tpm get_flag ...
[eventually there is even a better name than "get_flag"?]
...
+++ b/drivers/tpm/tis_i2c.c @@ -0,0 +1,211 @@ +/*
- Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
- Use of this source code is governed by a BSD-style license that can be
- found in the LICENSE file.
This is not acceptable. Please be precise here. We need to know exacly which license terms apply. Please see item # 1 at http://www.denx.de/wiki/view/U-Boot/Patches#Notes for details.
diff --git a/drivers/tpm/tpm.c b/drivers/tpm/tpm.c new file mode 100644 index 0000000..3f4f66e --- /dev/null +++ b/drivers/tpm/tpm.c @@ -0,0 +1,472 @@ +/*
- Copyright (C) 2011 Infineon Technologies
- Authors:
- Peter Huewe huewe.external@infineon.com
- Description:
- Device driver for TCG/TCPA TPM (trusted platform module).
- Specifications at www.trustedcomputinggroup.org
- It is based on the Linux kernel driver tpm.c from Leendert van
- Dorn, Dave Safford, Reiner Sailer, and Kyleen Hall.
Do you have a more precise reference where the code is coming from? What we would like to see is described in bullet # 4 at http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign
Best regards,
Wolfgang Denk