| Message ID | 1351874667-23959-1-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Tom Rini |
| Headers | show |
Dear Simon Glass, In message <1351874667-23959-1-git-send-email-sjg@chromium.org> you wrote: > From: Taylor Hutt <thutt@chromium.org> > > When building for the Sandbox version, the casts in this change are > necessary to avoid compilation issues. > > Signed-off-by: Taylor Hutt <thutt@chromium.org> > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > drivers/tpm/generic_lpc_tpm.c | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) I see little sense in spending work on this code. After all the time, it is still unused and dead code in mainline - there is not a single configuration which actually enables the necessary CONFIG_ options., I recommend to remove the whole TPM code instead. Best regards, Wolfgang Denk
Hi Wolfgang, On Sat, Nov 3, 2012 at 8:00 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon Glass, > > In message <1351874667-23959-1-git-send-email-sjg@chromium.org> you wrote: >> From: Taylor Hutt <thutt@chromium.org> >> >> When building for the Sandbox version, the casts in this change are >> necessary to avoid compilation issues. >> >> Signed-off-by: Taylor Hutt <thutt@chromium.org> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> drivers/tpm/generic_lpc_tpm.c | 10 +++++----- >> 1 files changed, 5 insertions(+), 5 deletions(-) > > I see little sense in spending work on this code. After all the time, > it is still unused and dead code in mainline - there is not a single > configuration which actually enables the necessary CONFIG_ options., > > I recommend to remove the whole TPM code instead. It is actually used in the x86 Chromebook, but the patch to enable it never made it to mainline. There was quite a bit of push-back on the x86 side at the time and the person working on it finally had enough :-( I have recently taken this up again to see if we can get x86 into a better state for newer Intel chips and the latest x86 Chromebooks. The patch to enable the TPM there is: http://patchwork.ozlabs.org/patch/190813/ The patch to enable on ARM Chromebooks is the third patch in this TPM series: http://patchwork.ozlabs.org/patch/196596/ I accept that it has sat there for a while without a board config to use it. But I would very much like to keep this code and see no sense in removing it now that it is actually in use. Regards, Simon > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > You are an excellent tactician, Captain. You let your second in com- > mand attack while you sit and watch for weakness. > -- Khan Noonian Singh, "Space Seed", stardate 3141.9
Dear Simon, In message <CAPnjgZ0RV991pa7MeVxr463g-TkKa+rzF8Gn7R0_ZbZnOWOsZg@mail.gmail.com> you wrote: > > > I recommend to remove the whole TPM code instead. > > It is actually used in the x86 Chromebook, but the patch to enable it > never made it to mainline. There was quite a bit of push-back on the > x86 side at the time and the person working on it finally had enough > :-( > > I have recently taken this up again to see if we can get x86 into a > better state for newer Intel chips and the latest x86 Chromebooks. The > patch to enable the TPM there is: > > http://patchwork.ozlabs.org/patch/190813/ I don;t see how this uses it in any way in U-Boot. It still would not even compile most of the code, right? > I accept that it has sat there for a while without a board config to > use it. But I would very much like to keep this code and see no sense > in removing it now that it is actually in use. Feel free to re-add it when there are any real users. I've posted a removal patch. Best regards, Wolfgang Denk
Hi Wolfgang, On Sat, Nov 3, 2012 at 5:26 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon, > > In message <CAPnjgZ0RV991pa7MeVxr463g-TkKa+rzF8Gn7R0_ZbZnOWOsZg@mail.gmail.com> you wrote: >> >> > I recommend to remove the whole TPM code instead. >> >> It is actually used in the x86 Chromebook, but the patch to enable it >> never made it to mainline. There was quite a bit of push-back on the >> x86 side at the time and the person working on it finally had enough >> :-( >> >> I have recently taken this up again to see if we can get x86 into a >> better state for newer Intel chips and the latest x86 Chromebooks. The >> patch to enable the TPM there is: >> >> http://patchwork.ozlabs.org/patch/190813/ > > I don;t see how this uses it in any way in U-Boot. It still would not > even compile most of the code, right? It compiles the driver which is the main thing. The command can be enabled/disabled as needed. If it helps I can post a patch to enable it. > >> I accept that it has sat there for a while without a board config to >> use it. But I would very much like to keep this code and see no sense >> in removing it now that it is actually in use. > > Feel free to re-add it when there are any real users. > > I've posted a removal patch. Can you please explain what you mean by 'real users'? Are you looking for an entire verified boot implementation before you will accept the TPM driver? Regards, Simon > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > ...when fits of creativity run strong, more than one programmer or > writer has been known to abandon the desktop for the more spacious > floor. - Fred Brooks, Jr.
Dear Simon, In message <CAPnjgZ2p-Re5Hp13st-Q2CuP49azyZZxwNRZSAfhr_iUSGbNJQ@mail.gmail.com> you wrote: > > >> I accept that it has sat there for a while without a board config to > >> use it. But I would very much like to keep this code and see no sense > >> in removing it now that it is actually in use. > > > > Feel free to re-add it when there are any real users. > > > > I've posted a removal patch. > > Can you please explain what you mean by 'real users'? Are you looking > for an entire verified boot implementation before you will accept the > TPM driver? What exactly is this driver good for? Which function does it provide that can be used on any board in mainline? It looks to me that it's just sitting there, and does nothing good to anybody. I don't know if there are any other legitimate uses for a TPM driver in U-Boot, but if there are, none appear to be implemented by any mainline board, so why should we have this driver in mainline? Best regards, Wolfgang Denk
Hi Wolfgang, On Wed, Nov 7, 2012 at 5:06 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon, > > In message <CAPnjgZ2p-Re5Hp13st-Q2CuP49azyZZxwNRZSAfhr_iUSGbNJQ@mail.gmail.com> you wrote: >> >> >> I accept that it has sat there for a while without a board config to >> >> use it. But I would very much like to keep this code and see no sense >> >> in removing it now that it is actually in use. >> > >> > Feel free to re-add it when there are any real users. >> > >> > I've posted a removal patch. >> >> Can you please explain what you mean by 'real users'? Are you looking >> for an entire verified boot implementation before you will accept the >> TPM driver? > > What exactly is this driver good for? Which function does it provide > that can be used on any board in mainline? It looks to me that it's > just sitting there, and does nothing good to anybody. > > I don't know if there are any other legitimate uses for a TPM driver > in U-Boot, but if there are, none appear to be implemented by any > mainline board, so why should we have this driver in mainline? I replied on the other thread - hopefully we can agree a plan to move this forward. Regards, Simon > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > While money can't buy happiness, it certainly lets you choose your > own form of misery.
diff --git a/drivers/tpm/generic_lpc_tpm.c b/drivers/tpm/generic_lpc_tpm.c index 6c494eb..04ad418 100644 --- a/drivers/tpm/generic_lpc_tpm.c +++ b/drivers/tpm/generic_lpc_tpm.c @@ -135,7 +135,7 @@ static u8 tpm_read_byte(const u8 *ptr) { u8 ret = readb(ptr); debug(PREFIX "Read reg 0x%4.4x returns 0x%2.2x\n", - (u32)ptr - (u32)lpc_tpm_dev, ret); + (u32)(uintptr_t)ptr - (u32)(uintptr_t)lpc_tpm_dev, ret); return ret; } @@ -143,21 +143,21 @@ static u32 tpm_read_word(const u32 *ptr) { u32 ret = readl(ptr); debug(PREFIX "Read reg 0x%4.4x returns 0x%8.8x\n", - (u32)ptr - (u32)lpc_tpm_dev, ret); + (u32)(uintptr_t)ptr - (u32)(uintptr_t)lpc_tpm_dev, ret); return ret; } static void tpm_write_byte(u8 value, u8 *ptr) { debug(PREFIX "Write reg 0x%4.4x with 0x%2.2x\n", - (u32)ptr - (u32)lpc_tpm_dev, value); + (u32)(uintptr_t)ptr - (u32)(uintptr_t)lpc_tpm_dev, value); writeb(value, ptr); } static void tpm_write_word(u32 value, u32 *ptr) { debug(PREFIX "Write reg 0x%4.4x with 0x%8.8x\n", - (u32)ptr - (u32)lpc_tpm_dev, value); + (u32)(uintptr_t)ptr - (u32)(uintptr_t)lpc_tpm_dev, value); writel(value, ptr); } @@ -491,5 +491,5 @@ int tis_sendrecv(const u8 *sendbuf, size_t send_size, return TPM_DRIVER_ERR; } - return tis_readresponse(recvbuf, recv_len); + return tis_readresponse(recvbuf, (u32 *)recv_len); }