Patchwork [U-Boot] TPM: remove dead code

login
register
mail settings
Submitter Wolfgang Denk
Date Nov. 3, 2012, 4:30 p.m.
Message ID <1351960246-31771-1-git-send-email-wd@denx.de>
Download mbox | patch
Permalink /patch/196885/
State Rejected
Delegated to: Tom Rini
Headers show

Comments

Wolfgang Denk - Nov. 3, 2012, 4:30 p.m.
The TPM code was added more than a year or 4 releases ago.  This was
done under the proposition that board support that would actually use
such code would be added soon.  However, nothing happened since.  The
code has no users in mainline, and does not even get build for any
configuration, so we cannot even tell if it compiles at all.

Remove the unused code.  In in some far future actual users show up,
it can be re-added easily.

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Vadim Bendebury <vbendeb@chromium.org>
Cc: Simon Glass <sjg@chromium.org>
---
 README                        |  10 -
 common/Makefile               |   1 -
 common/cmd_tpm.c              | 103 ---------
 doc/driver-model/UDM-tpm.txt  |  48 ----
 drivers/tpm/Makefile          |  43 ----
 drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
 include/tpm.h                 |  71 ------
 7 files changed, 771 deletions(-)
 delete mode 100644 common/cmd_tpm.c
 delete mode 100644 doc/driver-model/UDM-tpm.txt
 delete mode 100644 drivers/tpm/Makefile
 delete mode 100644 drivers/tpm/generic_lpc_tpm.c
 delete mode 100644 include/tpm.h
Simon Glass - Nov. 3, 2012, 8:42 p.m.
Hi Wolfgang,

On Sat, Nov 3, 2012 at 9:30 AM, Wolfgang Denk <wd@denx.de> wrote:
> The TPM code was added more than a year or 4 releases ago.  This was
> done under the proposition that board support that would actually use
> such code would be added soon.  However, nothing happened since.  The
> code has no users in mainline, and does not even get build for any
> configuration, so we cannot even tell if it compiles at all.
>
> Remove the unused code.  In in some far future actual users show up,
> it can be re-added easily.

I think you may have missed the pending patches which make use of
this. it is important functionality for the Chromebooks (secure boot).

Regards,
Simon

>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Vadim Bendebury <vbendeb@chromium.org>
> Cc: Simon Glass <sjg@chromium.org>
> ---
Wolfgang Denk - Nov. 4, 2012, 12:28 a.m.
Dear Simon,

In message <CAPnjgZ0JLarq6r=sHe+cfbjkyuf6hRgGP4BR9u_aSug14MZcrg@mail.gmail.com> you wrote:
> 
> On Sat, Nov 3, 2012 at 9:30 AM, Wolfgang Denk <wd@denx.de> wrote:
> > The TPM code was added more than a year or 4 releases ago.  This was
> > done under the proposition that board support that would actually use
> > such code would be added soon.  However, nothing happened since.  The
> > code has no users in mainline, and does not even get build for any
> > configuration, so we cannot even tell if it compiles at all.
> >
> > Remove the unused code.  In in some far future actual users show up,
> > it can be re-added easily.
> 
> I think you may have missed the pending patches which make use of
> this. it is important functionality for the Chromebooks (secure boot).

No, I have not missed these.  But all the patch does is set
CONFIG_GENERIC_LPC_TPM - there is still not a single user defining
CONFIG_CMD_TPM, so what does this help?  We still have tons of dead
code around.  Dump it!

Best regards,

Wolfgang Denk
Simon Glass - Nov. 4, 2012, 4:34 a.m.
Hi Wolfgang,

On Sat, Nov 3, 2012 at 5:28 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ0JLarq6r=sHe+cfbjkyuf6hRgGP4BR9u_aSug14MZcrg@mail.gmail.com> you wrote:
>>
>> On Sat, Nov 3, 2012 at 9:30 AM, Wolfgang Denk <wd@denx.de> wrote:
>> > The TPM code was added more than a year or 4 releases ago.  This was
>> > done under the proposition that board support that would actually use
>> > such code would be added soon.  However, nothing happened since.  The
>> > code has no users in mainline, and does not even get build for any
>> > configuration, so we cannot even tell if it compiles at all.
>> >
>> > Remove the unused code.  In in some far future actual users show up,
>> > it can be re-added easily.
>>
>> I think you may have missed the pending patches which make use of
>> this. it is important functionality for the Chromebooks (secure boot).
>
> No, I have not missed these.  But all the patch does is set
> CONFIG_GENERIC_LPC_TPM - there is still not a single user defining
> CONFIG_CMD_TPM, so what does this help?  We still have tons of dead
> code around.  Dump it!

So we need a board that defines the command also? I did not realise
that was a requirement - certainly I can add that command to the
boards also.

Upstreaming the code is a step-by-step process. The TPM is an
important component of secure boot, and things have to progress in
some sort of fashion. I do understand the dead code argument, but we
can't submit high-level code without the drivers it uses (and there
are many).

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
> Unix: Some say the learning curve is steep,  but  you  only  have  to
> climb it once.                                      - Karl Lehenbauer
Wolfgang Denk - Nov. 4, 2012, 9:54 a.m.
Dear Simon,

In message <CAPnjgZ0vRbsFWpGqsZPy=TjgJp9r80VnVv2Pcem5werL8bRwmw@mail.gmail.com> you wrote:
> 
> >> I think you may have missed the pending patches which make use of
> >> this. it is important functionality for the Chromebooks (secure boot).
> >
> > No, I have not missed these.  But all the patch does is set
> > CONFIG_GENERIC_LPC_TPM - there is still not a single user defining
> > CONFIG_CMD_TPM, so what does this help?  We still have tons of dead
> > code around.  Dump it!
> 
> So we need a board that defines the command also? I did not realise
> that was a requirement - certainly I can add that command to the
> boards also.

No, you misunderstand.  You stick at the surface, at the formal part
of it.  Indeed enabling this option for a board would be an
improvement - at least then we could be sure that the code compiles.
But that does not actually make it _used_.  Similar to you enabling of
CONFIG_GENERIC_LPC_TPM - the code that needs appears to be just more
unused code itself so this is not actual _use_ in the sense of
providing a functionality needed for the operation of the device.

> Upstreaming the code is a step-by-step process. The TPM is an
> important component of secure boot, and things have to progress in
> some sort of fashion. I do understand the dead code argument, but we
> can't submit high-level code without the drivers it uses (and there
> are many).

Well, we had this very same discussion when the TPM code was
originally added, and I let myself talk into accepting it, based on
the argument that code that needs it is available and will be
mainlined "really soon now".  More than a year has passed, and
_nothing_ happened.  Now I see the next wave of patches adding dead
code in the same are, and I have to admit that this does not make me
enthusiastic.

I think your approach is wrong. You should try and get a minimal
version (obviously with very restricted functionality) of your
"high-level code" into mainline, and all needed drivers in the same
patch series (which makes clear why this should be a minimal
configuration).  Then add drivers and functionality step by step.
Adding a driver here and a driver there one by one with the slight
chance that there might - in some unforeseeable future - a board
submitted that actually uses these all is nothing I'm going to accept.

And especially not after the experience with the TPM code so far,
where my patience has been exhausted after more than a year of nothing
happening.


Finally, I also have license concerns about the newly submitted code.
Statements like "code is governed by a BSD-style license" (in
drivers/tpm/tis_i2c.c) are obviusly not acceptabl at all - there are
several BSD-style licenses - am I to chose myself any of these, or
what???


I have the impression that you are trying to use the end of the merge
window to get a whole a bunch of very green code into mainline,
ignoring quite a number of rules well known to you.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 7, 2012, 1:12 a.m.
Hi Wolfgang,

On Sun, Nov 4, 2012 at 1:54 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ0vRbsFWpGqsZPy=TjgJp9r80VnVv2Pcem5werL8bRwmw@mail.gmail.com> you wrote:
>>
>> >> I think you may have missed the pending patches which make use of
>> >> this. it is important functionality for the Chromebooks (secure boot).
>> >
>> > No, I have not missed these.  But all the patch does is set
>> > CONFIG_GENERIC_LPC_TPM - there is still not a single user defining
>> > CONFIG_CMD_TPM, so what does this help?  We still have tons of dead
>> > code around.  Dump it!
>>
>> So we need a board that defines the command also? I did not realise
>> that was a requirement - certainly I can add that command to the
>> boards also.
>
> No, you misunderstand.  You stick at the surface, at the formal part
> of it.  Indeed enabling this option for a board would be an
> improvement - at least then we could be sure that the code compiles.
> But that does not actually make it _used_.  Similar to you enabling of
> CONFIG_GENERIC_LPC_TPM - the code that needs appears to be just more
> unused code itself so this is not actual _use_ in the sense of
> providing a functionality needed for the operation of the device.

OK, but I still don't quite get it. As I asked in the other thread,
are you not interested in TPM functionality at all until we have a
verified boot implementation, or are you happy to have things progress
in stages? I'm will to work through this, and I have the time at
present, but I believe that drivers for two common TPM chips are a
useful addition to U-Boot regardless.

>
>> Upstreaming the code is a step-by-step process. The TPM is an
>> important component of secure boot, and things have to progress in
>> some sort of fashion. I do understand the dead code argument, but we
>> can't submit high-level code without the drivers it uses (and there
>> are many).
>
> Well, we had this very same discussion when the TPM code was
> originally added, and I let myself talk into accepting it, based on
> the argument that code that needs it is available and will be
> mainlined "really soon now".  More than a year has passed, and
> _nothing_ happened.  Now I see the next wave of patches adding dead
> code in the same are, and I have to admit that this does not make me
> enthusiastic.

It's not obvious how to mainline this rather large feature except in
pieces. For example, I think I can start on a feature to verify a
kernel, but I do want to talk to the TPM as part of that.

>
> I think your approach is wrong. You should try and get a minimal
> version (obviously with very restricted functionality) of your
> "high-level code" into mainline, and all needed drivers in the same
> patch series (which makes clear why this should be a minimal
> configuration).  Then add drivers and functionality step by step.
> Adding a driver here and a driver there one by one with the slight
> chance that there might - in some unforeseeable future - a board
> submitted that actually uses these all is nothing I'm going to accept.

That's going to be a big series.... but in contrast, display, SATA and
GPIO patches are ok, but TPM is not?

I'll see what I can do here, but in principle I feel that adding a
driver should be OK, provided a board uses it, without necessarily the
high-level code that uses it. After all, we don't necessarily expect
to mainline all the scripts that drive the actual boot.

>
> And especially not after the experience with the TPM code so far,
> where my patience has been exhausted after more than a year of nothing
> happening.

Well not nothing. Have been rather tied up getting a product out. I
agree that things have been very quiet on verified boot - perhaps the
original upstream author was exhausted after the effort of getting the
driver into mainline and retired for a rest :-)

>
>
> Finally, I also have license concerns about the newly submitted code.
> Statements like "code is governed by a BSD-style license" (in
> drivers/tpm/tis_i2c.c) are obviusly not acceptabl at all - there are
> several BSD-style licenses - am I to chose myself any of these, or
> what???

Hmmm, that is in one file only, and one that I can easily change
without problem. It is not correct - I will change it to GPL.

>
>
> I have the impression that you are trying to use the end of the merge
> window to get a whole a bunch of very green code into mainline,
> ignoring quite a number of rules well known to you.

Not intentionally. The common/ series includes a few patches that I
have carried for quite a while (2 years in some cases), so I thought
it was time to either mainline it or drop it. I know from experience
that other people's patches can be very useful in certain
circumstances even if not perfect. But coding hiding in a dark corner
is no use to anyone.

However most of the common/ series is code that we depend on and I
think might be generally useful. I do admit that in striving for a
very high level of polish we have attacked problems that perhaps would
not matter for many systems, and thus more patches.

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
> Without facts, the decision cannot be made logically. You  must  rely
> on your human intuition.
>         -- Spock, "Assignment: Earth", stardate unknown
Wolfgang Denk - Nov. 7, 2012, 2:08 p.m.
Dear Simon Glass,

In message <CAPnjgZ2QyRjfjbJTR9MHcsQUEV933oAWPjSTvv-kFrhLCFJRUg@mail.gmail.com> you wrote:
> 
> OK, but I still don't quite get it. As I asked in the other thread,
> are you not interested in TPM functionality at all until we have a
> verified boot implementation, or are you happy to have things progress
> in stages? I'm will to work through this, and I have the time at
> present, but I believe that drivers for two common TPM chips are a
> useful addition to U-Boot regardless.

I'm sorry, but I disagree - the drivers may be useful indeed, but as
long as there are no users in U-Boot they are just dead weight.

So far we have followed the rule not to add dead code - we always asked
to add code (drivers) together with any "consumers", i. e. other code
that provides useful features (at least to one board) that actually
needs and uses such drivers.

With TPM we made an exception from this rule, based on the expectation
that users would be added "soon".  Then a full year passed, and
nothing happened.  Really _nothing_.

Now you suggest to "progress in stages"?  You mean adding more and
more code around, all of which will be unused, until one bright day in
a non-foreseeable future some board might get added, that will then
use this?

This is not exactly a proposal that triggers enthusiasm to me.


> It's not obvious how to mainline this rather large feature except in
> pieces. For example, I think I can start on a feature to verify a
> kernel, but I do want to talk to the TPM as part of that.

Instead of working bottom up (which will necessarily result in an
approach trying to add unused code) you could do it top down: start
indeed with a feature to verify a kernel (oh! we already have that
as psrt of the image verification - with simple CRC in the leggacy
images, and will optionally all other kinds in FIT images).  This
"verification could initially be an empty function just returning a
"true" value.  No TPM is needed for that.  Than work top down for
there.

> That's going to be a big series.... but in contrast, display, SATA and
> GPIO patches are ok, but TPM is not?

Ummm... are you telling me that contrast, display, SATA and GPIO
patches all also add dead code that is not used anywhere?  I have not
looked closer there because I simply did not expoect this.  But if so,
the very same rules apply:  we do not want to add dead code.

> I'll see what I can do here, but in principle I feel that adding a
> driver should be OK, provided a board uses it, without necessarily the
> high-level code that uses it. After all, we don't necessarily expect
> to mainline all the scripts that drive the actual boot.

what exactly do you mean by "a board uses it, without necessarily the
high-level code that uses it"?  If "uses" for you means just
conteining a function call reference so the code gets cimpiled and
pulled in by the linker, then I have to tell you that this NOT my
understanding of "usage".

For me, "using" some code means that it actually performs some
_function_.  As long as the code is not actually running on the
system, it is not being used.

> > And especially not after the experience with the TPM code so far,
> > where my patience has been exhausted after more than a year of nothing
> > happening.
> 
> Well not nothing. Have been rather tied up getting a product out. I

Sorry, but for U-Boot it is completely irrelevant what happens with
your out of tree code.  The code in mainline has just been sitting
there, unused as ever.

> agree that things have been very quiet on verified boot - perhaps the
> original upstream author was exhausted after the effort of getting the
> driver into mainline and retired for a rest :-)

Well, this experience has sunk in.  Exactly what I feared has
happened: the code was added, and it has been dead ever since,
for more than a full year.

It will be pretty difficult to talk me again into accepting unused
code.


> However most of the common/ series is code that we depend on and I
> think might be generally useful. I do admit that in striving for a
> very high level of polish we have attacked problems that perhaps would
> not matter for many systems, and thus more patches.

Mentioning that you depend on some specific code does not actually
make me feel bad, if this is what you (subcounsciously) had in mind.
Quote David Woodhouse: "If you're out of tree, you don't exist."
( http://article.gmane.org/gmane.linux.kernel.embedded/3534 )


All this discussion confirmed my resolution not to allow for dead
code.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 7, 2012, 4:17 p.m.
Hi Wolfgang,

On Wed, Nov 7, 2012 at 6:08 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ2QyRjfjbJTR9MHcsQUEV933oAWPjSTvv-kFrhLCFJRUg@mail.gmail.com> you wrote:
>>
>> OK, but I still don't quite get it. As I asked in the other thread,
>> are you not interested in TPM functionality at all until we have a
>> verified boot implementation, or are you happy to have things progress
>> in stages? I'm will to work through this, and I have the time at
>> present, but I believe that drivers for two common TPM chips are a
>> useful addition to U-Boot regardless.
>
> I'm sorry, but I disagree - the drivers may be useful indeed, but as
> long as there are no users in U-Boot they are just dead weight.
>
> So far we have followed the rule not to add dead code - we always asked
> to add code (drivers) together with any "consumers", i. e. other code
> that provides useful features (at least to one board) that actually
> needs and uses such drivers.
>
> With TPM we made an exception from this rule, based on the expectation
> that users would be added "soon".  Then a full year passed, and
> nothing happened.  Really _nothing_.
>
> Now you suggest to "progress in stages"?  You mean adding more and
> more code around, all of which will be unused, until one bright day in
> a non-foreseeable future some board might get added, that will then
> use this?

Please see my comment below.

>
> This is not exactly a proposal that triggers enthusiasm to me.
>

Enthusiasm is a strong word...

>
>> It's not obvious how to mainline this rather large feature except in
>> pieces. For example, I think I can start on a feature to verify a
>> kernel, but I do want to talk to the TPM as part of that.
>
> Instead of working bottom up (which will necessarily result in an
> approach trying to add unused code) you could do it top down: start
> indeed with a feature to verify a kernel (oh! we already have that
> as psrt of the image verification - with simple CRC in the leggacy
> images, and will optionally all other kinds in FIT images).  This
> "verification could initially be an empty function just returning a
> "true" value.  No TPM is needed for that.  Than work top down for
> there.

OK I will see if I can do something there. The TPM is used to verify
the kernel version to prevent rollback attacks.

>
>> That's going to be a big series.... but in contrast, display, SATA and
>> GPIO patches are ok, but TPM is not?
>
> Ummm... are you telling me that contrast, display, SATA and GPIO
> patches all also add dead code that is not used anywhere?  I have not
> looked closer there because I simply did not expoect this.  But if so,
> the very same rules apply:  we do not want to add dead code.

Let's take an example. We have AHCI/SATA support in U-Boot. There may
be no board code in U-Boot that actually reads from a disk (I am not
sure, but it is plausible, since hard-coded disk/fs access from a
board's C code would be odd). Let's say this code exists only in
commands that may or may not be executed by board scripts, which are
out of tree. I'm sure this isn't considered dead code, but I want to
understand the difference. If there is a TPM command which may or may
not be executed by board scripts (also out of tree), what is the
difference?

Following this dead code argument, whole U-Boot subsystems are dead
code since they are only used indirectly from the command line and not
directly by any board code?

So my thinking now is that I should:

1. Create a way of extracting verification information from a signed
block (e.g. kernel hash, rollback info)
2. Add some commands to access this
3. Create a basic script which uses these commands

Does this sound right?

>
>> I'll see what I can do here, but in principle I feel that adding a
>> driver should be OK, provided a board uses it, without necessarily the
>> high-level code that uses it. After all, we don't necessarily expect
>> to mainline all the scripts that drive the actual boot.
>
> what exactly do you mean by "a board uses it, without necessarily the
> high-level code that uses it"?  If "uses" for you means just
> conteining a function call reference so the code gets cimpiled and
> pulled in by the linker, then I have to tell you that this NOT my
> understanding of "usage".
>
> For me, "using" some code means that it actually performs some
> _function_.  As long as the code is not actually running on the
> system, it is not being used.

OK, see above, I think we are getting to the root of this.

>
>> > And especially not after the experience with the TPM code so far,
>> > where my patience has been exhausted after more than a year of nothing
>> > happening.
>>
>> Well not nothing. Have been rather tied up getting a product out. I
>
> Sorry, but for U-Boot it is completely irrelevant what happens with
> your out of tree code.  The code in mainline has just been sitting
> there, unused as ever.
>
>> agree that things have been very quiet on verified boot - perhaps the
>> original upstream author was exhausted after the effort of getting the
>> driver into mainline and retired for a rest :-)
>
> Well, this experience has sunk in.  Exactly what I feared has
> happened: the code was added, and it has been dead ever since,
> for more than a full year.
>
> It will be pretty difficult to talk me again into accepting unused
> code.
>

So long as I can understand the definition then we should be fine.

>
>> However most of the common/ series is code that we depend on and I
>> think might be generally useful. I do admit that in striving for a
>> very high level of polish we have attacked problems that perhaps would
>> not matter for many systems, and thus more patches.
>
> Mentioning that you depend on some specific code does not actually
> make me feel bad, if this is what you (subcounsciously) had in mind.
> Quote David Woodhouse: "If you're out of tree, you don't exist."
> ( http://article.gmane.org/gmane.linux.kernel.embedded/3534 )
>

Well if it is not generally useful then we shouldn't have it. But it's
good to post things to the list and have people comment.

>
> All this discussion confirmed my resolution not to allow for dead
> code.
>
> Best regards,
>
> Wolfgang Denk
>

Regards,
Simon

> --
> 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
> "The question of whether a computer can think is no more  interesting
> than the question of whether a submarine can swim"
>                                                 - Edsgar W.  Dijkstra
Wolfgang Denk - Nov. 7, 2012, 7:35 p.m.
Dear Simon,

In message <CAPnjgZ0V=MwiVYQ3H-dsVEpNVHMdOPx3Yh3qtPdje0=_Cz=smg@mail.gmail.com> you wrote:
> 
> > This is not exactly a proposal that triggers enthusiasm to me.
> 
> Enthusiasm is a strong word...

I tried to find a strictly non-negative phrasing here, because I felt
that would be somewhat better ;-)

> Let's take an example. We have AHCI/SATA support in U-Boot. There may
> be no board code in U-Boot that actually reads from a disk (I am not
> sure, but it is plausible, since hard-coded disk/fs access from a
> board's C code would be odd). Let's say this code exists only in
> commands that may or may not be executed by board scripts, which are
> out of tree. I'm sure this isn't considered dead code, but I want to

As soon as I can type on the console something like

	ext2load sata 0:1 200000 boot/uImage

and this is actually capable of reading data from a S-ATA attached
storage device, then this is perfectly fine:  the driver provides a
functionality, which can be used in U-Boot to perform certain actions
on the real system.

> understand the difference. If there is a TPM command which may or may
> not be executed by board scripts (also out of tree), what is the
> difference?

If there is a TPM command which actually communicates with the TPM
hardware and can be used for example to verify an image checksum or
such, then it provides a function.

> Following this dead code argument, whole U-Boot subsystems are dead
> code since they are only used indirectly from the command line and not
> directly by any board code?

This is not what I mean.

> So my thinking now is that I should:
> 
> 1. Create a way of extracting verification information from a signed
> block (e.g. kernel hash, rollback info)
> 2. Add some commands to access this
> 3. Create a basic script which uses these commands
> 
> Does this sound right?

3. would actually not be needed, as long as 1. and 2. really work on
some system.

Comment for 1. (but not relateds to this thread, just a note while we
are at it): I imagine such functionality should be integrated with the
FIT image handling, i. e. like we are today able to verify - say - a
sha256 checksum of an image, your code would be able to verify a TPM
backed signature.  Does this match your plans?


> So long as I can understand the definition then we should be fine.

Can we agree on something like: code is considered to be "used" (in the
sense of it is no dead code) when there is at least one board
configuration in mainline where, either as part of the normal
initialization sequence or as reaction to some interactive user
command, said code will be executed and perform the specific function
it was designed for ?

As a side effect this means that there is at least one board
configuration that will actually compile and link this piece of code.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 7, 2012, 8:38 p.m.
Hi Wolfgang,

On Wed, Nov 7, 2012 at 11:35 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ0V=MwiVYQ3H-dsVEpNVHMdOPx3Yh3qtPdje0=_Cz=smg@mail.gmail.com> you wrote:
>>
>> > This is not exactly a proposal that triggers enthusiasm to me.
>>
>> Enthusiasm is a strong word...
>
> I tried to find a strictly non-negative phrasing here, because I felt
> that would be somewhat better ;-)

Much appreciated :-)

>
>> Let's take an example. We have AHCI/SATA support in U-Boot. There may
>> be no board code in U-Boot that actually reads from a disk (I am not
>> sure, but it is plausible, since hard-coded disk/fs access from a
>> board's C code would be odd). Let's say this code exists only in
>> commands that may or may not be executed by board scripts, which are
>> out of tree. I'm sure this isn't considered dead code, but I want to
>
> As soon as I can type on the console something like
>
>         ext2load sata 0:1 200000 boot/uImage
>
> and this is actually capable of reading data from a S-ATA attached
> storage device, then this is perfectly fine:  the driver provides a
> functionality, which can be used in U-Boot to perform certain actions
> on the real system.
>
>> understand the difference. If there is a TPM command which may or may
>> not be executed by board scripts (also out of tree), what is the
>> difference?
>
> If there is a TPM command which actually communicates with the TPM
> hardware and can be used for example to verify an image checksum or
> such, then it provides a function.
>
>> Following this dead code argument, whole U-Boot subsystems are dead
>> code since they are only used indirectly from the command line and not
>> directly by any board code?
>
> This is not what I mean.
>
>> So my thinking now is that I should:
>>
>> 1. Create a way of extracting verification information from a signed
>> block (e.g. kernel hash, rollback info)
>> 2. Add some commands to access this
>> 3. Create a basic script which uses these commands
>>
>> Does this sound right?
>
> 3. would actually not be needed, as long as 1. and 2. really work on
> some system.

OK I think I understand this now. I will take a look at this and see
what I can come up with.

>
> Comment for 1. (but not relateds to this thread, just a note while we
> are at it): I imagine such functionality should be integrated with the
> FIT image handling, i. e. like we are today able to verify - say - a
> sha256 checksum of an image, your code would be able to verify a TPM
> backed signature.  Does this match your plans?

That would be nice. That format seems almost infinitely flexible so we
should be able to make it work.

Still, I believe the FIT image itself needs to be signed. If we try to
sign the components of the FIT image, then I think we will get into
trouble because it will need multiple crypto operations to verify each
piece (and there might be a lot of pieces if we have a lot of FDTs
inside). If we try to put the whole-FIT-image signature inside the FIT
image, then we have a chicken and egg problem - we cannot sign it
until we know what is in it, but we can't know what is in it until we
know the signature.

One option would be to embed a FIT image inside a FIT image, like this:

+---- Top Level FIT Image-----------------------------------+
|                                                           |
|    * Hash of embedded FIT image, signed with secret key   |
|                                                           |
|    +---Embedded FIT Image-------------+                   |
|    |                                  |                   |
|    |  kernels                         |                   |
|    |  fdts                            |                   |
|    |  configs                         |                   |
|    |                                  |                   |
|    +----------------------------------+                   |
|                                                           |
+-----------------------------------------------------------+

But this is not ideal because we have to verify the whole image
(including the large kernel) just to check its version / compatibility
(we may have several kernel options and be trying to pick the best).

Ideally we would be able to split the FIT image into a meta-data piece
and a data piece, so we can verify the meta-data all at once with a
single key, and the data only as needed when we decide we want to load
a particular kernel.

The FDT format sort-of does this, but I believe the kernel is embedded
as a blob, so in practice we can't separate the two. Right?

I will think about it some more.

>
>
>> So long as I can understand the definition then we should be fine.
>
> Can we agree on something like: code is considered to be "used" (in the
> sense of it is no dead code) when there is at least one board
> configuration in mainline where, either as part of the normal
> initialization sequence or as reaction to some interactive user
> command, said code will be executed and perform the specific function
> it was designed for ?
>
> As a side effect this means that there is at least one board
> configuration that will actually compile and link this piece of code.

Yes that seems reasonable, and it makes sense. But 'reaction to some
interactive user command' allows me to create such a command (as I did
with TPM) which exercises the driver and thus makes it undead code.
This is a bit of a grey area.

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
> "Free markets select for winning solutions."        - Eric S. Raymond
Wolfgang Denk - Nov. 8, 2012, 10:31 a.m.
[Note: Subject changed, Threading restarted].

Dear Simon,

In message <CAPnjgZ11wScsnvPG29CFDRYz7Xbtp=WB+tL0UmGvNvHmHmJ-RA@mail.gmail.com> you wrote:
> 
> > Comment for 1. (but not relateds to this thread, just a note while we
> > are at it): I imagine such functionality should be integrated with the
> > FIT image handling, i. e. like we are today able to verify - say - a
> > sha256 checksum of an image, your code would be able to verify a TPM
> > backed signature.  Does this match your plans?
> 
> That would be nice. That format seems almost infinitely flexible so we
> should be able to make it work.
> 
> Still, I believe the FIT image itself needs to be signed. If we try to

I don;t think this is a good idea...

> sign the components of the FIT image, then I think we will get into
> trouble because it will need multiple crypto operations to verify each
> piece (and there might be a lot of pieces if we have a lot of FDTs
> inside). ...

Are you sure that would really be a problem?

First, what makes you sure that checking the signature over N parts is
so much slower than checking it over one summary image with a much
larger size (totalling the sum of the images, plus overhead)?  Is
setting up such a check really such an expensive operation?

Second, who says you must check the signature of all parts?  Even if
you have a large number of DT blobs, you will select only one for
booting, so it should be sufficient to verify only this one - why
should you care about all the other, unused ones?  Actually that could
be _faster_ than checking everything.

Third, there may be many alternative approaches.  For example, you
could put all the DT blobs into a small file system (to be used in
U-Boot as a RAM disk) - then you can verify just a single file system
image, and then just read the needed DT from that.  Think a bit
longer, and more approaches will spring up.

>      ... If we try to put the whole-FIT-image signature inside the FIT
> image, then we have a chicken and egg problem - we cannot sign it
> until we know what is in it, but we can't know what is in it until we
> know the signature.

True.

> One option would be to embed a FIT image inside a FIT image, like this:

This does not make much sense to me, too complicated, and I don't see
the real need yet.

> Ideally we would be able to split the FIT image into a meta-data piece
> and a data piece, so we can verify the meta-data all at once with a
> single key, and the data only as needed when we decide we want to load
> a particular kernel.
> 
> The FDT format sort-of does this, but I believe the kernel is embedded
> as a blob, so in practice we can't separate the two. Right?

Each "object" carries it's own meta-data, but what an object actually
is, is pretty much flexible - of course, the command infrastructre
needs to support it.  If you want to use "bootm" with the FIT image to
boot it directly, then you need to stick with the current format.  But
of course the bootm command can already now be used as individual
sub-commands, and similarly we could (and probably should) add a set
of commands (resp. one "fit" command with a set of sub-commands) to
perform specific operations on the parts of a FIT image.

> > As a side effect this means that there is at least one board
> > configuration that will actually compile and link this piece of code.
> 
> Yes that seems reasonable, and it makes sense. But 'reaction to some
> interactive user command' allows me to create such a command (as I did
> with TPM) which exercises the driver and thus makes it undead code.
> This is a bit of a grey area.

We could try to come up with a lawyer-proof specification of the
requirements for such a command (i. e. that it has to perform a
"real", or "useful" operation etc.), thus trying to prevent the
creation of dummy commands that are ment only to fulfil the "no dead
code" requirement.  I don't want to go that far.  Let's apply common
sense.  If you write a driver, you probably want to test it some way,
so you will have to implement some command interface to exercise the
driver in any case.  I think there are only few cases where such a
command would not be included in the normal U-Boot configuration.

But yes, there are grey areas; but maybe it's enough if we plant big
signs "Here Abide Monsters" on such ways...

Best regards,

Wolfgang Denk
Wolfgang Denk - Nov. 20, 2012, 7:14 a.m.
Dear Tom,

In message <1351960246-31771-1-git-send-email-wd@denx.de> you wrote:
> The TPM code was added more than a year or 4 releases ago.  This was
> done under the proposition that board support that would actually use
> such code would be added soon.  However, nothing happened since.  The
> code has no users in mainline, and does not even get build for any
> configuration, so we cannot even tell if it compiles at all.
> 
> Remove the unused code.  In in some far future actual users show up,
> it can be re-added easily.
> 
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Vadim Bendebury <vbendeb@chromium.org>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  README                        |  10 -
>  common/Makefile               |   1 -
>  common/cmd_tpm.c              | 103 ---------
>  doc/driver-model/UDM-tpm.txt  |  48 ----
>  drivers/tpm/Makefile          |  43 ----
>  drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
>  include/tpm.h                 |  71 ------
>  7 files changed, 771 deletions(-)
>  delete mode 100644 common/cmd_tpm.c
>  delete mode 100644 doc/driver-model/UDM-tpm.txt
>  delete mode 100644 drivers/tpm/Makefile
>  delete mode 100644 drivers/tpm/generic_lpc_tpm.c
>  delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied
yet.

Despite some discussion about why having TPM code is generally useful,
there are still no actual users for it visible, so I really would like
to see this patch applied.

It is trivial to re-activate this code if actual users for it should
be added some day to come.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 22, 2012, 7:10 p.m.
Hi Wolfgang,

On Mon, Nov 19, 2012 at 11:14 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Tom,
>
> In message <1351960246-31771-1-git-send-email-wd@denx.de> you wrote:
>> The TPM code was added more than a year or 4 releases ago.  This was
>> done under the proposition that board support that would actually use
>> such code would be added soon.  However, nothing happened since.  The
>> code has no users in mainline, and does not even get build for any
>> configuration, so we cannot even tell if it compiles at all.
>>
>> Remove the unused code.  In in some far future actual users show up,
>> it can be re-added easily.
>>
>> Signed-off-by: Wolfgang Denk <wd@denx.de>
>> Cc: Vadim Bendebury <vbendeb@chromium.org>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  README                        |  10 -
>>  common/Makefile               |   1 -
>>  common/cmd_tpm.c              | 103 ---------
>>  doc/driver-model/UDM-tpm.txt  |  48 ----
>>  drivers/tpm/Makefile          |  43 ----
>>  drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
>>  include/tpm.h                 |  71 ------
>>  7 files changed, 771 deletions(-)
>>  delete mode 100644 common/cmd_tpm.c
>>  delete mode 100644 doc/driver-model/UDM-tpm.txt
>>  delete mode 100644 drivers/tpm/Makefile
>>  delete mode 100644 drivers/tpm/generic_lpc_tpm.c
>>  delete mode 100644 include/tpm.h
>
> This was sent before end of the merge window, but has not been applied
> yet.
>
> Despite some discussion about why having TPM code is generally useful,
> there are still no actual users for it visible, so I really would like
> to see this patch applied.
>
> It is trivial to re-activate this code if actual users for it should
> be added some day to come.

For what it is worth, this is a NAK from me. There is a patch pending
for the next release to enable this for coreboot, and another driver
for an i2c TPM driver for the ARM Chromebook also pending, also within
the merge window. The TPM support here is light but it is a start, and
reverting it seems to serve no purpose (I am happy to maintain the
code if it needs work, e.g. for device model). On the contrary this
code provides a base on which verified boot can be built, and provides
useful functions for talking to a TPM which may be on others boards as
well.

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
> "I used to think that the brain was the most wonderful  organ  in  my
> body. Then I realized who was telling me this."        - Emo Phillips
Simon Glass - Nov. 22, 2012, 7:42 p.m.
Hi Wolfgang,

On Thu, Nov 22, 2012 at 11:10 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Wolfgang,
>
> On Mon, Nov 19, 2012 at 11:14 PM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Tom,
>>
>> In message <1351960246-31771-1-git-send-email-wd@denx.de> you wrote:
>>> The TPM code was added more than a year or 4 releases ago.  This was
>>> done under the proposition that board support that would actually use
>>> such code would be added soon.  However, nothing happened since.  The
>>> code has no users in mainline, and does not even get build for any
>>> configuration, so we cannot even tell if it compiles at all.
>>>
>>> Remove the unused code.  In in some far future actual users show up,
>>> it can be re-added easily.
>>>
>>> Signed-off-by: Wolfgang Denk <wd@denx.de>
>>> Cc: Vadim Bendebury <vbendeb@chromium.org>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>>  README                        |  10 -
>>>  common/Makefile               |   1 -
>>>  common/cmd_tpm.c              | 103 ---------
>>>  doc/driver-model/UDM-tpm.txt  |  48 ----
>>>  drivers/tpm/Makefile          |  43 ----
>>>  drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
>>>  include/tpm.h                 |  71 ------
>>>  7 files changed, 771 deletions(-)
>>>  delete mode 100644 common/cmd_tpm.c
>>>  delete mode 100644 doc/driver-model/UDM-tpm.txt
>>>  delete mode 100644 drivers/tpm/Makefile
>>>  delete mode 100644 drivers/tpm/generic_lpc_tpm.c
>>>  delete mode 100644 include/tpm.h
>>
>> This was sent before end of the merge window, but has not been applied
>> yet.
>>
>> Despite some discussion about why having TPM code is generally useful,
>> there are still no actual users for it visible, so I really would like
>> to see this patch applied.
>>
>> It is trivial to re-activate this code if actual users for it should
>> be added some day to come.
>
> For what it is worth, this is a NAK from me. There is a patch pending
> for the next release to enable this for coreboot, and another driver
> for an i2c TPM driver for the ARM Chromebook also pending, also within
> the merge window. The TPM support here is light but it is a start, and
> reverting it seems to serve no purpose (I am happy to maintain the
> code if it needs work, e.g. for device model). On the contrary this
> code provides a base on which verified boot can be built, and provides
> useful functions for talking to a TPM which may be on others boards as
> well.

Part of the reason why the TPM was never turned on in coreboot was the
difficulty in upstreaming the code, which resulted in us giving up
about a year ago. That effort has since restarted and I am hopeful
that it will be successful.

Perhaps as a compromise we could have one more cycle to create a
verified boot which uses the TPM drivers, which I think is what is
required here? The timing is good at this end to actually do the work.

That would send that right signal that this work is valued upstream.
That is a very important signal.

Regards,
SImon

>
> 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
>> "I used to think that the brain was the most wonderful  organ  in  my
>> body. Then I realized who was telling me this."        - Emo Phillips
Simon Glass - Nov. 26, 2012, 5:07 a.m.
Hi Wolfgang,

On Thu, Nov 8, 2012 at 2:31 AM, Wolfgang Denk <wd@denx.de> wrote:
> [Note: Subject changed, Threading restarted].
>
> Dear Simon,
>
> In message <CAPnjgZ11wScsnvPG29CFDRYz7Xbtp=WB+tL0UmGvNvHmHmJ-RA@mail.gmail.com> you wrote:
>>
>> > Comment for 1. (but not relateds to this thread, just a note while we
>> > are at it): I imagine such functionality should be integrated with the
>> > FIT image handling, i. e. like we are today able to verify - say - a
>> > sha256 checksum of an image, your code would be able to verify a TPM
>> > backed signature.  Does this match your plans?
>>
>> That would be nice. That format seems almost infinitely flexible so we
>> should be able to make it work.
>>
>> Still, I believe the FIT image itself needs to be signed. If we try to
>
> I don;t think this is a good idea...
>
>> sign the components of the FIT image, then I think we will get into
>> trouble because it will need multiple crypto operations to verify each
>> piece (and there might be a lot of pieces if we have a lot of FDTs
>> inside). ...
>
> Are you sure that would really be a problem?
>
> First, what makes you sure that checking the signature over N parts is
> so much slower than checking it over one summary image with a much
> larger size (totalling the sum of the images, plus overhead)?  Is
> setting up such a check really such an expensive operation?
>
> Second, who says you must check the signature of all parts?  Even if
> you have a large number of DT blobs, you will select only one for
> booting, so it should be sufficient to verify only this one - why
> should you care about all the other, unused ones?  Actually that could
> be _faster_ than checking everything.
>
> Third, there may be many alternative approaches.  For example, you
> could put all the DT blobs into a small file system (to be used in
> U-Boot as a RAM disk) - then you can verify just a single file system
> image, and then just read the needed DT from that.  Think a bit
> longer, and more approaches will spring up.

Yes I hope so. For now I think we could do something like sign the
kernel and FDTs individually (actually probably sign their hashes
since that is likely to be faster) and write the signature in along
with the hash.

>
>>      ... If we try to put the whole-FIT-image signature inside the FIT
>> image, then we have a chicken and egg problem - we cannot sign it
>> until we know what is in it, but we can't know what is in it until we
>> know the signature.
>
> True.
>
>> One option would be to embed a FIT image inside a FIT image, like this:
>
> This does not make much sense to me, too complicated, and I don't see
> the real need yet.
>
>> Ideally we would be able to split the FIT image into a meta-data piece
>> and a data piece, so we can verify the meta-data all at once with a
>> single key, and the data only as needed when we decide we want to load
>> a particular kernel.
>>
>> The FDT format sort-of does this, but I believe the kernel is embedded
>> as a blob, so in practice we can't separate the two. Right?
>
> Each "object" carries it's own meta-data, but what an object actually
> is, is pretty much flexible - of course, the command infrastructre
> needs to support it.  If you want to use "bootm" with the FIT image to
> boot it directly, then you need to stick with the current format.  But
> of course the bootm command can already now be used as individual
> sub-commands, and similarly we could (and probably should) add a set
> of commands (resp. one "fit" command with a set of sub-commands) to
> perform specific operations on the parts of a FIT image.
>

That sounds good.

I feel that we may still need to sign the whole FIT image, or at least
enough to cover the strings and the structure. Otherwise we have a
fair bit of code which is looking at user-supplied unverified data.
The smaller this 'attach surface' is, the better. But for a starting
point I think it is reasonable, and we can perhaps come up with a
whole-image signing mechanism later.

>> > As a side effect this means that there is at least one board
>> > configuration that will actually compile and link this piece of code.
>>
>> Yes that seems reasonable, and it makes sense. But 'reaction to some
>> interactive user command' allows me to create such a command (as I did
>> with TPM) which exercises the driver and thus makes it undead code.
>> This is a bit of a grey area.
>
> We could try to come up with a lawyer-proof specification of the
> requirements for such a command (i. e. that it has to perform a
> "real", or "useful" operation etc.), thus trying to prevent the
> creation of dummy commands that are ment only to fulfil the "no dead
> code" requirement.  I don't want to go that far.  Let's apply common
> sense.  If you write a driver, you probably want to test it some way,
> so you will have to implement some command interface to exercise the
> driver in any case.  I think there are only few cases where such a
> command would not be included in the normal U-Boot configuration.
>
> But yes, there are grey areas; but maybe it's enough if we plant big
> signs "Here Abide Monsters" on such ways...

OK.

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 know, after a woman's raised a family and so on,  she  wants  to
> start living her own life."   "Whose life she's _been_ living, then?"
>                                   - Terry Pratchett, _Witches Abroad_
Tom Rini - Nov. 26, 2012, 8:03 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/22/12 14:42, Simon Glass wrote:
> Hi Wolfgang,
> 
> On Thu, Nov 22, 2012 at 11:10 AM, Simon Glass <sjg@chromium.org> 
> wrote:
>> Hi Wolfgang,
>> 
>> On Mon, Nov 19, 2012 at 11:14 PM, Wolfgang Denk <wd@denx.de> 
>> wrote:
>>> Dear Tom,
>>> 
>>> In message <1351960246-31771-1-git-send-email-wd@denx.de> you 
>>> wrote:
>>>> The TPM code was added more than a year or 4 releases ago. 
>>>> This was done under the proposition that board support that 
>>>> would actually use such code would be added soon.  However, 
>>>> nothing happened since.  The code has no users in mainline, 
>>>> and does not even get build for any configuration, so we 
>>>> cannot even tell if it compiles at all.
>>>> 
>>>> Remove the unused code.  In in some far future actual users 
>>>> show up, it can be re-added easily.
>>>> 
>>>> Signed-off-by: Wolfgang Denk <wd@denx.de> Cc: Vadim
>>>> Bendebury <vbendeb@chromium.org> Cc: Simon Glass
>>>> <sjg@chromium.org> --- README                        |  10 -
>>>> common/Makefile |   1 - common/cmd_tpm.c              | 103
>>>> --------- doc/driver-model/UDM-tpm.txt  |  48 ----
>>>> drivers/tpm/Makefile |  43 ---- drivers/tpm/generic_lpc_tpm.c
>>>> | 495 ------------------------------------------
>>>> include/tpm.h |  71 ------ 7 files changed, 771 deletions(-)
>>>> delete mode 100644 common/cmd_tpm.c delete mode 100644 
>>>> doc/driver-model/UDM-tpm.txt delete mode 100644 
>>>> drivers/tpm/Makefile delete mode 100644 
>>>> drivers/tpm/generic_lpc_tpm.c delete mode 100644 
>>>> include/tpm.h
>>> 
>>> This was sent before end of the merge window, but has not been 
>>> applied yet.
>>> 
>>> Despite some discussion about why having TPM code is generally 
>>> useful, there are still no actual users for it visible, so I 
>>> really would like to see this patch applied.
>>> 
>>> It is trivial to re-activate this code if actual users for it 
>>> should be added some day to come.
>> 
>> For what it is worth, this is a NAK from me. There is a patch 
>> pending for the next release to enable this for coreboot, and 
>> another driver for an i2c TPM driver for the ARM Chromebook also 
>> pending, also within the merge window. The TPM support here is 
>> light but it is a start, and reverting it seems to serve no 
>> purpose (I am happy to maintain the code if it needs work, e.g. 
>> for device model). On the contrary this code provides a base on 
>> which verified boot can be built, and provides useful functions 
>> for talking to a TPM which may be on others boards as well.
> 
> Part of the reason why the TPM was never turned on in coreboot was 
> the difficulty in upstreaming the code, which resulted in us
> giving up about a year ago. That effort has since restarted and I
> am hopeful that it will be successful.
> 
> Perhaps as a compromise we could have one more cycle to create a 
> verified boot which uses the TPM drivers, which I think is what is
>  required here? The timing is good at this end to actually do the 
> work.
> 
> That would send that right signal that this work is valued 
> upstream. That is a very important signal.

I can see where Wolfgang's frustration comes from here.  So yes,
you've got until v2012.04-rc1 comes out (which will be ~2 weeks after
v2013.01 is out) before I pull Wolfgang's patch to have posted a
series that makes use of this (which is to say, you have until the end
of the merge window for the next release to have something that is
mergeable for the release).

- -- 
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQs8sVAAoJENk4IS6UOR1Wyt0P/2GZMc3RAt/3ZC5SaB/lLVG6
UACxxvWfGWDg7QaYxLokNkbMQfnKNQMGCCrMRbNiU7A5rjbRN+6IeaB7kpQ0CQN3
1SUKcdk2hZSpLXH6KGRg4l7i/0MohKyp1tX2qkLJs4ojwAfbcYO3v+Pz+QIuMcz+
rqURzArPOoHA/7yq4WPkQrB820Y8nelQFYo4w1DJ+cEtZ6c1gWJSLj0LAprj8ZcD
DVHYtITA2K3d7SS0dm3TjNwwHXrSuOZ1n/+SQURqQ87nUxRiLdd7W54rPyeB58H7
lRst0AC6y1yPBTL30U898HrWg1owF5r4fTNp+/Qwr2rGG5C4ClnHCkS5vvMYc0J4
hSDPya5z5rv7zQQ4xa2t4lvjZdj+ZQ1hafQfaCJB9+sHs6Qz49MJzKpwLR0cLy6u
9Sl27LskphVXmJ9YosrSlGWjh+RSFoJZhV7hlUpaU3Kvd9qNHafrJvUj8MlCaTuU
yFbfO71J88dsIJZlTTSuGQ3NTf5cYNySe91EW7JSGcW4B+IkpTGVb1crxxpzaw0B
LTkCT9Sc9AkJezfi4C/a/66bLoaQ3ugtFcsEuk641jiCnlsQvHjNsg6vzgJQ+8NX
Cbs5a0lzpUUtFLALyrTakHi6/99SsP20Ez8/p3U9+2FS0xD9XDyiqCf7fxZf48Ek
bnafmm4U+kmyhs0i9FzU
=IDf8
-----END PGP SIGNATURE-----
Simon Glass - Nov. 28, 2012, 10:42 p.m.
Hi Tom,

On Mon, Nov 26, 2012 at 12:03 PM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/22/12 14:42, Simon Glass wrote:
>> Hi Wolfgang,
>>
>> On Thu, Nov 22, 2012 at 11:10 AM, Simon Glass <sjg@chromium.org>
>> wrote:
>>> Hi Wolfgang,
>>>
>>> On Mon, Nov 19, 2012 at 11:14 PM, Wolfgang Denk <wd@denx.de>
>>> wrote:
>>>> Dear Tom,
>>>>
>>>> In message <1351960246-31771-1-git-send-email-wd@denx.de> you
>>>> wrote:
>>>>> The TPM code was added more than a year or 4 releases ago.
>>>>> This was done under the proposition that board support that
>>>>> would actually use such code would be added soon.  However,
>>>>> nothing happened since.  The code has no users in mainline,
>>>>> and does not even get build for any configuration, so we
>>>>> cannot even tell if it compiles at all.
>>>>>
>>>>> Remove the unused code.  In in some far future actual users
>>>>> show up, it can be re-added easily.
>>>>>
>>>>> Signed-off-by: Wolfgang Denk <wd@denx.de> Cc: Vadim
>>>>> Bendebury <vbendeb@chromium.org> Cc: Simon Glass
>>>>> <sjg@chromium.org> --- README                        |  10 -
>>>>> common/Makefile |   1 - common/cmd_tpm.c              | 103
>>>>> --------- doc/driver-model/UDM-tpm.txt  |  48 ----
>>>>> drivers/tpm/Makefile |  43 ---- drivers/tpm/generic_lpc_tpm.c
>>>>> | 495 ------------------------------------------
>>>>> include/tpm.h |  71 ------ 7 files changed, 771 deletions(-)
>>>>> delete mode 100644 common/cmd_tpm.c delete mode 100644
>>>>> doc/driver-model/UDM-tpm.txt delete mode 100644
>>>>> drivers/tpm/Makefile delete mode 100644
>>>>> drivers/tpm/generic_lpc_tpm.c delete mode 100644
>>>>> include/tpm.h
>>>>
>>>> This was sent before end of the merge window, but has not been
>>>> applied yet.
>>>>
>>>> Despite some discussion about why having TPM code is generally
>>>> useful, there are still no actual users for it visible, so I
>>>> really would like to see this patch applied.
>>>>
>>>> It is trivial to re-activate this code if actual users for it
>>>> should be added some day to come.
>>>
>>> For what it is worth, this is a NAK from me. There is a patch
>>> pending for the next release to enable this for coreboot, and
>>> another driver for an i2c TPM driver for the ARM Chromebook also
>>> pending, also within the merge window. The TPM support here is
>>> light but it is a start, and reverting it seems to serve no
>>> purpose (I am happy to maintain the code if it needs work, e.g.
>>> for device model). On the contrary this code provides a base on
>>> which verified boot can be built, and provides useful functions
>>> for talking to a TPM which may be on others boards as well.
>>
>> Part of the reason why the TPM was never turned on in coreboot was
>> the difficulty in upstreaming the code, which resulted in us
>> giving up about a year ago. That effort has since restarted and I
>> am hopeful that it will be successful.
>>
>> Perhaps as a compromise we could have one more cycle to create a
>> verified boot which uses the TPM drivers, which I think is what is
>>  required here? The timing is good at this end to actually do the
>> work.
>>
>> That would send that right signal that this work is valued
>> upstream. That is a very important signal.
>
> I can see where Wolfgang's frustration comes from here.  So yes,
> you've got until v2012.04-rc1 comes out (which will be ~2 weeks after
> v2013.01 is out) before I pull Wolfgang's patch to have posted a
> series that makes use of this (which is to say, you have until the end
> of the merge window for the next release to have something that is
> mergeable for the release).

OK that seems reasonable, it gives me time to make this happen.

Thank you both,
Simon

>
> - --
> Tom
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJQs8sVAAoJENk4IS6UOR1Wyt0P/2GZMc3RAt/3ZC5SaB/lLVG6
> UACxxvWfGWDg7QaYxLokNkbMQfnKNQMGCCrMRbNiU7A5rjbRN+6IeaB7kpQ0CQN3
> 1SUKcdk2hZSpLXH6KGRg4l7i/0MohKyp1tX2qkLJs4ojwAfbcYO3v+Pz+QIuMcz+
> rqURzArPOoHA/7yq4WPkQrB820Y8nelQFYo4w1DJ+cEtZ6c1gWJSLj0LAprj8ZcD
> DVHYtITA2K3d7SS0dm3TjNwwHXrSuOZ1n/+SQURqQ87nUxRiLdd7W54rPyeB58H7
> lRst0AC6y1yPBTL30U898HrWg1owF5r4fTNp+/Qwr2rGG5C4ClnHCkS5vvMYc0J4
> hSDPya5z5rv7zQQ4xa2t4lvjZdj+ZQ1hafQfaCJB9+sHs6Qz49MJzKpwLR0cLy6u
> 9Sl27LskphVXmJ9YosrSlGWjh+RSFoJZhV7hlUpaU3Kvd9qNHafrJvUj8MlCaTuU
> yFbfO71J88dsIJZlTTSuGQ3NTf5cYNySe91EW7JSGcW4B+IkpTGVb1crxxpzaw0B
> LTkCT9Sc9AkJezfi4C/a/66bLoaQ3ugtFcsEuk641jiCnlsQvHjNsg6vzgJQ+8NX
> Cbs5a0lzpUUtFLALyrTakHi6/99SsP20Ez8/p3U9+2FS0xD9XDyiqCf7fxZf48Ek
> bnafmm4U+kmyhs0i9FzU
> =IDf8
> -----END PGP SIGNATURE-----

Patch

diff --git a/README b/README
index 22fd6b7..54a0bdd 100644
--- a/README
+++ b/README
@@ -1161,16 +1161,6 @@  The following options need to be configured:
 			CONFIG_SH_ETHER_CACHE_WRITEBACK
 			If this option is set, the driver enables cache flush.
 
-- TPM Support:
-		CONFIG_GENERIC_LPC_TPM
-		Support for generic parallel port TPM devices. Only one device
-		per system is supported at this time.
-
-			CONFIG_TPM_TIS_BASE_ADDRESS
-			Base address where the generic TPM device is mapped
-			to. Contemporary x86 systems usually map it at
-			0xfed40000.
-
 - USB Support:
 		At the moment only the UHCI host controller is
 		supported (PIP405, MIP405, MPC5200); define
diff --git a/common/Makefile b/common/Makefile
index 9e43322..6bcfdca 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -154,7 +154,6 @@  COBJS-$(CONFIG_CMD_STRINGS) += cmd_strings.o
 COBJS-$(CONFIG_CMD_TERMINAL) += cmd_terminal.o
 COBJS-$(CONFIG_CMD_TIME) += cmd_time.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_test.o
-COBJS-$(CONFIG_CMD_TPM) += cmd_tpm.o
 COBJS-$(CONFIG_CMD_TSI148) += cmd_tsi148.o
 COBJS-$(CONFIG_CMD_UBI) += cmd_ubi.o
 COBJS-$(CONFIG_CMD_UBIFS) += cmd_ubifs.o
diff --git a/common/cmd_tpm.c b/common/cmd_tpm.c
deleted file mode 100644
index 6f5cd48..0000000
--- a/common/cmd_tpm.c
+++ /dev/null
@@ -1,103 +0,0 @@ 
-/*
- * Copyright (c) 2011 The Chromium OS Authors.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <command.h>
-#include <tpm.h>
-
-#define MAX_TRANSACTION_SIZE 30
-
-/*
- * tpm_write() expects a variable number of parameters: the internal address
- * followed by data to write, byte by byte.
- *
- * Returns 0 on success or -1 on errors (wrong arguments or TPM failure).
- */
-static int tpm_process(int argc, char * const argv[], cmd_tbl_t *cmdtp)
-{
-	u8 tpm_buffer[MAX_TRANSACTION_SIZE];
-	u32 write_size, read_size;
-	char *p;
-	int rv = -1;
-
-	for (write_size = 0; write_size < argc; write_size++) {
-		u32 datum = simple_strtoul(argv[write_size], &p, 0);
-		if (*p || (datum > 0xff)) {
-			printf("\n%s: bad data value\n\n", argv[write_size]);
-			cmd_usage(cmdtp);
-			return rv;
-		}
-		tpm_buffer[write_size] = (u8)datum;
-	}
-
-	read_size = sizeof(tpm_buffer);
-	if (!tis_sendrecv(tpm_buffer, write_size, tpm_buffer, &read_size)) {
-		int i;
-		puts("Got TPM response:\n");
-		for (i = 0; i < read_size; i++)
-			printf(" %2.2x", tpm_buffer[i]);
-		puts("\n");
-		rv = 0;
-	} else {
-		puts("tpm command failed\n");
-	}
-	return rv;
-}
-
-static int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	int rv = 0;
-
-	/*
-	 * Verify that in case it is present, the first argument, it is
-	 * exactly one character in size.
-	 */
-	if (argc < 7) {
-		puts("command should be at least six bytes in size\n");
-		return -1;
-	}
-
-	if (tis_init()) {
-		puts("tis_init() failed!\n");
-		return -1;
-	}
-
-	if (tis_open()) {
-		puts("tis_open() failed!\n");
-		return -1;
-	}
-
-	rv = tpm_process(argc - 1, argv + 1, cmdtp);
-
-	if (tis_close()) {
-		puts("tis_close() failed!\n");
-		rv = -1;
-	}
-
-	return rv;
-}
-
-U_BOOT_CMD(tpm, MAX_TRANSACTION_SIZE, 1, do_tpm,
-	   "<byte> [<byte> ...]   - write data and read response",
-	   "send arbitrary data (at least 6 bytes) to the TPM "
-	   "device and read the response"
-);
diff --git a/doc/driver-model/UDM-tpm.txt b/doc/driver-model/UDM-tpm.txt
deleted file mode 100644
index 91a953a..0000000
--- a/doc/driver-model/UDM-tpm.txt
+++ /dev/null
@@ -1,48 +0,0 @@ 
-The U-Boot Driver Model Project
-===============================
-TPM system analysis
-===================
-Marek Vasut <marek.vasut@gmail.com>
-2012-02-23
-
-I) Overview
------------
-
-There is currently only one TPM chip driver available and therefore the API
-controlling it is very much based on this. The API is very simple:
-
-  int tis_open(void);
-  int tis_close(void);
-  int tis_sendrecv(const u8 *sendbuf, size_t send_size,
-                         u8 *recvbuf, size_t *recv_len);
-
-The command operating the TPM chip only provides operations to send and receive
-bytes from the chip.
-
-II) Approach
-------------
-
-The API can't be generalised too much considering there's only one TPM chip
-supported. But it's a good idea to split the tis_sendrecv() function in two
-functions. Therefore the new API will use register the TPM chip by calling:
-
-  tpm_device_register(struct instance *i, const struct tpm_ops *ops);
-
-And the struct tpm_ops will contain the following members:
-
-  struct tpm_ops {
-    int (*tpm_open)(struct instance *i);
-    int (*tpm_close)(struct instance *i);
-    int (*tpm_send)(const uint8_t *buf, const size_t size);
-    int (*tpm_recv)(uint8_t *buf, size_t *size);
-  };
-
-The behaviour of "tpm_open()" and "tpm_close()" will basically copy the
-behaviour of "tis_open()" and "tis_close()". The "tpm_send()" will be based on
-the "tis_senddata()" and "tis_recv()" will be based on "tis_readresponse()".
-
-III) Analysis of in-tree drivers
---------------------------------
-
-There is only one in-tree driver present, the "drivers/tpm/generic_lpc_tpm.c",
-which will be simply converted as outlined in previous chapter.
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
deleted file mode 100644
index be11c8b..0000000
--- a/drivers/tpm/Makefile
+++ /dev/null
@@ -1,43 +0,0 @@ 
-# Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
-#
-# See file CREDITS for list of people who contributed to this
-# project.
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License as
-# published by the Free Software Foundation; either version 2 of
-# the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
-# MA 02111-1307 USA
-#
-
-include $(TOPDIR)/config.mk
-
-LIB := $(obj)libtpm.o
-
-COBJS-$(CONFIG_GENERIC_LPC_TPM) = generic_lpc_tpm.o
-
-COBJS	:= $(COBJS-y)
-SRCS	:= $(COBJS:.o=.c)
-OBJS	:= $(addprefix $(obj),$(COBJS))
-
-all:	$(LIB)
-
-$(LIB): $(obj).depend $(OBJS)
-	$(call cmd_link_o_target, $(OBJS))
-
-#########################################################################
-
-include $(SRCTREE)/rules.mk
-
-sinclude $(obj).depend
-
-#########################################################################
diff --git a/drivers/tpm/generic_lpc_tpm.c b/drivers/tpm/generic_lpc_tpm.c
deleted file mode 100644
index 6c494eb..0000000
--- a/drivers/tpm/generic_lpc_tpm.c
+++ /dev/null
@@ -1,495 +0,0 @@ 
-/*
- * Copyright (c) 2011 The Chromium OS Authors.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-/*
- * The code in this file is based on the article "Writing a TPM Device Driver"
- * published on http://ptgmedia.pearsoncmg.com.
- *
- * One principal difference is that in the simplest config the other than 0
- * TPM localities do not get mapped by some devices (for instance, by Infineon
- * slb9635), so this driver provides access to locality 0 only.
- */
-
-#include <common.h>
-#include <asm/io.h>
-#include <tpm.h>
-
-#define PREFIX "lpc_tpm: "
-
-struct tpm_locality {
-	u32 access;
-	u8 padding0[4];
-	u32 int_enable;
-	u8 vector;
-	u8 padding1[3];
-	u32 int_status;
-	u32 int_capability;
-	u32 tpm_status;
-	u8 padding2[8];
-	u8 data;
-	u8 padding3[3803];
-	u32 did_vid;
-	u8 rid;
-	u8 padding4[251];
-};
-
-/*
- * This pointer refers to the TPM chip, 5 of its localities are mapped as an
- * array.
- */
-#define TPM_TOTAL_LOCALITIES	5
-static struct tpm_locality *lpc_tpm_dev =
-	(struct tpm_locality *)CONFIG_TPM_TIS_BASE_ADDRESS;
-
-/* Some registers' bit field definitions */
-#define TIS_STS_VALID                  (1 << 7) /* 0x80 */
-#define TIS_STS_COMMAND_READY          (1 << 6) /* 0x40 */
-#define TIS_STS_TPM_GO                 (1 << 5) /* 0x20 */
-#define TIS_STS_DATA_AVAILABLE         (1 << 4) /* 0x10 */
-#define TIS_STS_EXPECT                 (1 << 3) /* 0x08 */
-#define TIS_STS_RESPONSE_RETRY         (1 << 1) /* 0x02 */
-
-#define TIS_ACCESS_TPM_REG_VALID_STS   (1 << 7) /* 0x80 */
-#define TIS_ACCESS_ACTIVE_LOCALITY     (1 << 5) /* 0x20 */
-#define TIS_ACCESS_BEEN_SEIZED         (1 << 4) /* 0x10 */
-#define TIS_ACCESS_SEIZE               (1 << 3) /* 0x08 */
-#define TIS_ACCESS_PENDING_REQUEST     (1 << 2) /* 0x04 */
-#define TIS_ACCESS_REQUEST_USE         (1 << 1) /* 0x02 */
-#define TIS_ACCESS_TPM_ESTABLISHMENT   (1 << 0) /* 0x01 */
-
-#define TIS_STS_BURST_COUNT_MASK       (0xffff)
-#define TIS_STS_BURST_COUNT_SHIFT      (8)
-
-/*
- * Error value returned if a tpm register does not enter the expected state
- * after continuous polling. No actual TPM register reading ever returns -1,
- * so this value is a safe error indication to be mixed with possible status
- * register values.
- */
-#define TPM_TIMEOUT_ERR			(-1)
-
-/* Error value returned on various TPM driver errors. */
-#define TPM_DRIVER_ERR		(1)
-
- /* 1 second is plenty for anything TPM does. */
-#define MAX_DELAY_US	(1000 * 1000)
-
-/* Retrieve burst count value out of the status register contents. */
-static u16 burst_count(u32 status)
-{
-	return (status >> TIS_STS_BURST_COUNT_SHIFT) & TIS_STS_BURST_COUNT_MASK;
-}
-
-/*
- * Structures defined below allow creating descriptions of TPM vendor/device
- * ID information for run time discovery. The only device the system knows
- * about at this time is Infineon slb9635.
- */
-struct device_name {
-	u16 dev_id;
-	const char * const dev_name;
-};
-
-struct vendor_name {
-	u16 vendor_id;
-	const char *vendor_name;
-	const struct device_name *dev_names;
-};
-
-static const struct device_name infineon_devices[] = {
-	{0xb, "SLB9635 TT 1.2"},
-	{0}
-};
-
-static const struct vendor_name vendor_names[] = {
-	{0x15d1, "Infineon", infineon_devices},
-};
-
-/*
- * Cached vendor/device ID pair to indicate that the device has been already
- * discovered.
- */
-static u32 vendor_dev_id;
-
-/* TPM access wrappers to support tracing */
-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);
-	return ret;
-}
-
-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);
-	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);
-	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);
-	writel(value, ptr);
-}
-
-/*
- * tis_wait_reg()
- *
- * Wait for at least a second for a register to change its state to match the
- * expected state. Normally the transition happens within microseconds.
- *
- * @reg - pointer to the TPM register
- * @mask - bitmask for the bitfield(s) to watch
- * @expected - value the field(s) are supposed to be set to
- *
- * Returns the register contents in case the expected value was found in the
- * appropriate register bits, or TPM_TIMEOUT_ERR on timeout.
- */
-static u32 tis_wait_reg(u32 *reg, u8 mask, u8 expected)
-{
-	u32 time_us = MAX_DELAY_US;
-
-	while (time_us > 0) {
-		u32 value = tpm_read_word(reg);
-		if ((value & mask) == expected)
-			return value;
-		udelay(1); /* 1 us */
-		time_us--;
-	}
-	return TPM_TIMEOUT_ERR;
-}
-
-/*
- * Probe the TPM device and try determining its manufacturer/device name.
- *
- * Returns 0 on success (the device is found or was found during an earlier
- * invocation) or TPM_DRIVER_ERR if the device is not found.
- */
-int tis_init(void)
-{
-	u32 didvid = tpm_read_word(&lpc_tpm_dev[0].did_vid);
-	int i;
-	const char *device_name = "unknown";
-	const char *vendor_name = device_name;
-	u16 vid, did;
-
-	if (vendor_dev_id)
-		return 0;  /* Already probed. */
-
-	if (!didvid || (didvid == 0xffffffff)) {
-		printf("%s: No TPM device found\n", __func__);
-		return TPM_DRIVER_ERR;
-	}
-
-	vendor_dev_id = didvid;
-
-	vid = didvid & 0xffff;
-	did = (didvid >> 16) & 0xffff;
-	for (i = 0; i < ARRAY_SIZE(vendor_names); i++) {
-		int j = 0;
-		u16 known_did;
-
-		if (vid == vendor_names[i].vendor_id)
-			vendor_name = vendor_names[i].vendor_name;
-
-		while ((known_did = vendor_names[i].dev_names[j].dev_id) != 0) {
-			if (known_did == did) {
-				device_name =
-					vendor_names[i].dev_names[j].dev_name;
-				break;
-			}
-			j++;
-		}
-		break;
-	}
-
-	printf("Found TPM %s by %s\n", device_name, vendor_name);
-	return 0;
-}
-
-/*
- * tis_senddata()
- *
- * send the passed in data to the TPM device.
- *
- * @data - address of the data to send, byte by byte
- * @len - length of the data to send
- *
- * Returns 0 on success, TPM_DRIVER_ERR on error (in case the device does
- * not accept the entire command).
- */
-static u32 tis_senddata(const u8 * const data, u32 len)
-{
-	u32 offset = 0;
-	u16 burst = 0;
-	u32 max_cycles = 0;
-	u8 locality = 0;
-	u32 value;
-
-	value = tis_wait_reg(&lpc_tpm_dev[locality].tpm_status,
-			     TIS_STS_COMMAND_READY, TIS_STS_COMMAND_READY);
-	if (value == TPM_TIMEOUT_ERR) {
-		printf("%s:%d - failed to get 'command_ready' status\n",
-		       __FILE__, __LINE__);
-		return TPM_DRIVER_ERR;
-	}
-	burst = burst_count(value);
-
-	while (1) {
-		unsigned count;
-
-		/* Wait till the device is ready to accept more data. */
-		while (!burst) {
-			if (max_cycles++ == MAX_DELAY_US) {
-				printf("%s:%d failed to feed %d bytes of %d\n",
-				       __FILE__, __LINE__, len - offset, len);
-				return TPM_DRIVER_ERR;
-			}
-			udelay(1);
-			burst = burst_count(tpm_read_word(&lpc_tpm_dev
-						     [locality].tpm_status));
-		}
-
-		max_cycles = 0;
-
-		/*
-		 * Calculate number of bytes the TPM is ready to accept in one
-		 * shot.
-		 *
-		 * We want to send the last byte outside of the loop (hence
-		 * the -1 below) to make sure that the 'expected' status bit
-		 * changes to zero exactly after the last byte is fed into the
-		 * FIFO.
-		 */
-		count = min(burst, len - offset - 1);
-		while (count--)
-			tpm_write_byte(data[offset++],
-				  &lpc_tpm_dev[locality].data);
-
-		value = tis_wait_reg(&lpc_tpm_dev[locality].tpm_status,
-				     TIS_STS_VALID, TIS_STS_VALID);
-
-		if ((value == TPM_TIMEOUT_ERR) || !(value & TIS_STS_EXPECT)) {
-			printf("%s:%d TPM command feed overflow\n",
-			       __FILE__, __LINE__);
-			return TPM_DRIVER_ERR;
-		}
-
-		burst = burst_count(value);
-		if ((offset == (len - 1)) && burst) {
-			/*
-			 * We need to be able to send the last byte to the
-			 * device, so burst size must be nonzero before we
-			 * break out.
-			 */
-			break;
-		}
-	}
-
-	/* Send the last byte. */
-	tpm_write_byte(data[offset++], &lpc_tpm_dev[locality].data);
-	/*
-	 * Verify that TPM does not expect any more data as part of this
-	 * command.
-	 */
-	value = tis_wait_reg(&lpc_tpm_dev[locality].tpm_status,
-			     TIS_STS_VALID, TIS_STS_VALID);
-	if ((value == TPM_TIMEOUT_ERR) || (value & TIS_STS_EXPECT)) {
-		printf("%s:%d unexpected TPM status 0x%x\n",
-		       __FILE__, __LINE__, value);
-		return TPM_DRIVER_ERR;
-	}
-
-	/* OK, sitting pretty, let's start the command execution. */
-	tpm_write_word(TIS_STS_TPM_GO, &lpc_tpm_dev[locality].tpm_status);
-	return 0;
-}
-
-/*
- * tis_readresponse()
- *
- * read the TPM device response after a command was issued.
- *
- * @buffer - address where to read the response, byte by byte.
- * @len - pointer to the size of buffer
- *
- * On success stores the number of received bytes to len and returns 0. On
- * errors (misformatted TPM data or synchronization problems) returns
- * TPM_DRIVER_ERR.
- */
-static u32 tis_readresponse(u8 *buffer, u32 *len)
-{
-	u16 burst;
-	u32 value;
-	u32 offset = 0;
-	u8 locality = 0;
-	const u32 has_data = TIS_STS_DATA_AVAILABLE | TIS_STS_VALID;
-	u32 expected_count = *len;
-	int max_cycles = 0;
-
-	/* Wait for the TPM to process the command. */
-	value = tis_wait_reg(&lpc_tpm_dev[locality].tpm_status,
-			      has_data, has_data);
-	if (value == TPM_TIMEOUT_ERR) {
-		printf("%s:%d failed processing command\n",
-		       __FILE__, __LINE__);
-		return TPM_DRIVER_ERR;
-	}
-
-	do {
-		while ((burst = burst_count(value)) == 0) {
-			if (max_cycles++ == MAX_DELAY_US) {
-				printf("%s:%d TPM stuck on read\n",
-				       __FILE__, __LINE__);
-				return TPM_DRIVER_ERR;
-			}
-			udelay(1);
-			value = tpm_read_word(&lpc_tpm_dev
-					      [locality].tpm_status);
-		}
-
-		max_cycles = 0;
-
-		while (burst-- && (offset < expected_count)) {
-			buffer[offset++] = tpm_read_byte(&lpc_tpm_dev
-							 [locality].data);
-
-			if (offset == 6) {
-				/*
-				 * We got the first six bytes of the reply,
-				 * let's figure out how many bytes to expect
-				 * total - it is stored as a 4 byte number in
-				 * network order, starting with offset 2 into
-				 * the body of the reply.
-				 */
-				u32 real_length;
-				memcpy(&real_length,
-				       buffer + 2,
-				       sizeof(real_length));
-				expected_count = be32_to_cpu(real_length);
-
-				if ((expected_count < offset) ||
-				    (expected_count > *len)) {
-					printf("%s:%d bad response size %d\n",
-					       __FILE__, __LINE__,
-					       expected_count);
-					return TPM_DRIVER_ERR;
-				}
-			}
-		}
-
-		/* Wait for the next portion. */
-		value = tis_wait_reg(&lpc_tpm_dev[locality].tpm_status,
-				     TIS_STS_VALID, TIS_STS_VALID);
-		if (value == TPM_TIMEOUT_ERR) {
-			printf("%s:%d failed to read response\n",
-			       __FILE__, __LINE__);
-			return TPM_DRIVER_ERR;
-		}
-
-		if (offset == expected_count)
-			break;	/* We got all we needed. */
-
-	} while ((value & has_data) == has_data);
-
-	/*
-	 * Make sure we indeed read all there was. The TIS_STS_VALID bit is
-	 * known to be set.
-	 */
-	if (value & TIS_STS_DATA_AVAILABLE) {
-		printf("%s:%d wrong receive status %x\n",
-		       __FILE__, __LINE__, value);
-		return TPM_DRIVER_ERR;
-	}
-
-	/* Tell the TPM that we are done. */
-	tpm_write_word(TIS_STS_COMMAND_READY, &lpc_tpm_dev
-		  [locality].tpm_status);
-	*len = offset;
-	return 0;
-}
-
-int tis_open(void)
-{
-	u8 locality = 0; /* we use locality zero for everything. */
-
-	if (tis_close())
-		return TPM_DRIVER_ERR;
-
-	/* now request access to locality. */
-	tpm_write_word(TIS_ACCESS_REQUEST_USE, &lpc_tpm_dev[locality].access);
-
-	/* did we get a lock? */
-	if (tis_wait_reg(&lpc_tpm_dev[locality].access,
-			 TIS_ACCESS_ACTIVE_LOCALITY,
-			 TIS_ACCESS_ACTIVE_LOCALITY) == TPM_TIMEOUT_ERR) {
-		printf("%s:%d - failed to lock locality %d\n",
-		       __FILE__, __LINE__, locality);
-		return TPM_DRIVER_ERR;
-	}
-
-	tpm_write_word(TIS_STS_COMMAND_READY,
-		       &lpc_tpm_dev[locality].tpm_status);
-	return 0;
-}
-
-int tis_close(void)
-{
-	u8 locality = 0;
-
-	if (tpm_read_word(&lpc_tpm_dev[locality].access) &
-	    TIS_ACCESS_ACTIVE_LOCALITY) {
-		tpm_write_word(TIS_ACCESS_ACTIVE_LOCALITY,
-			       &lpc_tpm_dev[locality].access);
-
-		if (tis_wait_reg(&lpc_tpm_dev[locality].access,
-				 TIS_ACCESS_ACTIVE_LOCALITY, 0) ==
-		    TPM_TIMEOUT_ERR) {
-			printf("%s:%d - failed to release locality %d\n",
-			       __FILE__, __LINE__, locality);
-			return TPM_DRIVER_ERR;
-		}
-	}
-	return 0;
-}
-
-int tis_sendrecv(const u8 *sendbuf, size_t send_size,
-		 u8 *recvbuf, size_t *recv_len)
-{
-	if (tis_senddata(sendbuf, send_size)) {
-		printf("%s:%d failed sending data to TPM\n",
-		       __FILE__, __LINE__);
-		return TPM_DRIVER_ERR;
-	}
-
-	return tis_readresponse(recvbuf, recv_len);
-}
diff --git a/include/tpm.h b/include/tpm.h
deleted file mode 100644
index 6b21e9c..0000000
--- a/include/tpm.h
+++ /dev/null
@@ -1,71 +0,0 @@ 
-/*
- * Copyright (c) 2011 The Chromium OS Authors.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#ifndef _INCLUDE_TPM_H_
-#define _INCLUDE_TPM_H_
-
-#include <common.h>
-
-/*
- * tis_init()
- *
- * Initialize the TPM device. Returns 0 on success or -1 on
- * failure (in case device probing did not succeed).
- */
-int tis_init(void);
-
-/*
- * tis_open()
- *
- * Requests access to locality 0 for the caller. After all commands have been
- * completed the caller is supposed to call tis_close().
- *
- * Returns 0 on success, -1 on failure.
- */
-int tis_open(void);
-
-/*
- * tis_close()
- *
- * terminate the currect session with the TPM by releasing the locked
- * locality. Returns 0 on success of -1 on failure (in case lock
- * removal did not succeed).
- */
-int tis_close(void);
-
-/*
- * tis_sendrecv()
- *
- * Send the requested data to the TPM and then try to get its response
- *
- * @sendbuf - buffer of the data to send
- * @send_size size of the data to send
- * @recvbuf - memory to save the response to
- * @recv_len - pointer to the size of the response buffer
- *
- * Returns 0 on success (and places the number of response bytes at recv_len)
- * or -1 on failure.
- */
-int tis_sendrecv(const uint8_t *sendbuf, size_t send_size, uint8_t *recvbuf,
-			size_t *recv_len);
-
-#endif /* _INCLUDE_TPM_H_ */