Patchwork [U-Boot,1/3] tpm: Add casts for proper compilation

login
register
mail settings
Submitter Simon Glass
Date Nov. 2, 2012, 4:44 p.m.
Message ID <1351874667-23959-1-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/196594/
State Superseded, archived
Delegated to: Tom Rini
Headers show

Comments

Simon Glass - Nov. 2, 2012, 4:44 p.m.
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(-)
Wolfgang Denk - Nov. 3, 2012, 3 p.m.
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
Simon Glass - Nov. 3, 2012, 8:38 p.m.
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
Wolfgang Denk - Nov. 4, 2012, 12:26 a.m.
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
Simon Glass - Nov. 7, 2012, 12:48 a.m.
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.
Wolfgang Denk - Nov. 7, 2012, 1:06 p.m.
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
Simon Glass - Nov. 7, 2012, 4:20 p.m.
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.

Patch

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);
 }