On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote: > I still think removing all logging traces is a mistake for something > that can consume a significant amount of time in the boot sequence. > It's going to cause lost of people doing boot timings to waste lots of > effort. > > However, removing the log messages makes the above statement a lie, so > one of the two needs fixing. The commit itself makes sense but the implementation was sloppy to say the least in v3. It was like prototype/PoC version of something that could be merged to mainline, not something that can be merged to mainline. For example: * You could have implemented it more cleanly without that new 'tpm_transmit_check' helper function as can be seen. * Many log messages contained a redundant "TPM:" prefix. * There duplicate logs to tpm_transmit_cmd(). Pass a NULL as desc tpm_transmit_cmd() if you want to take care of logging yourself. * The commit has the same short summary as the commit it fixes. This issue still persists. Open for suggestions. Please state if v3 contains a log message that has been removed and is still mandatory and I can add it if it makes sense. > > + rc = tpm_transmit_cmd(chip, NULL, buf.data, > > PAGE_SIZE, 0, 0, > > + "attempting the self test\n"); > > There shouldn't be a \n in the string: the failure message already > appends one. Oops, my bad, will fix it. /Jarkko