Hi Wolfgang, On Sun, Nov 4, 2012 at 1:54 AM, Wolfgang Denk <w...@denx.de> wrote: > Dear Simon, > > In message > <CAPnjgZ0vRbsFWpGqsZPy=tjgjp9r80vnvv2pcem5werl8br...@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: w...@denx.de > Without facts, the decision cannot be made logically. You must rely > on your human intuition. > -- Spock, "Assignment: Earth", stardate unknown _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
