From: Jarkko Sakkinen <jarkko@kernel.org>
To: amirmizi6@gmail.com, Eyal.Cohen@nuvoton.com,
oshrialkoby85@gmail.com, alexander.steffen@infineon.com,
robh+dt@kernel.org, mark.rutland@arm.com, peterhuewe@gmx.de,
jgg@ziepe.ca, arnd@arndb.de, gregkh@linuxfoundation.org,
benoit.houyere@st.com, eajames@linux.ibm.com, joel@jms.id.au
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-integrity@vger.kernel.org, oshri.alkoby@nuvoton.com,
tmaimon77@gmail.com, gcwilson@us.ibm.com, kgoldman@us.ibm.com,
Dan.Morav@nuvoton.com, oren.tanami@nuvoton.com,
shmulik.hager@nuvoton.com, amir.mizinski@nuvoton.com
Subject: Re: [PATCH v16 1/6] tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
Date: Sat, 23 Oct 2021 03:44:01 +0300 [thread overview]
Message-ID: <ae373226e1b7b117f2808facdf0c3b6b0f282a39.camel@kernel.org> (raw)
In-Reply-To: <20211021120557.69234-2-amirmizi6@gmail.com>
On Thu, 2021-10-21 at 15:05 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
>
> Detected an incorrect implementation of the send command.
Nit: please just remove this sentence.
It tells nothing.
> Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it
> reads TPM_STS register again to verify only that TPM_STS.expect field is
> FALSE (i.e., it ignores TPM_STS.stsValid).
> Since TPM_STS.stsValid represents the TPM_STS.expect validity, a check of
> only one of these fields is wrong. Fix this condition so that both fields
> are checked in the same TPM_STS register read.
>
> Modify the signature of 'wait_for_tpm_stat()', adding an additional
Remove single quotes from functions, e.g. just write
wait_form_tpm_stat().
> "result" parameter to its call.
> 'wait_for_tpm_stat()' is now polling the TPM_STS with a mask and waits
> for the value in result. This modification adds the ability to check if
> certain TPM_STS bits have been cleared.
> For example, use the new parameter to check in status that TPM_STS_VALID
> is set and also that TPM_STS_EXPECT is zeroed. This prevents a racy
> check.
>
> In addition, the send command was changed to comply with
> TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf as follows:
> - send all command bytes in one loop
What does this mean? Also, please write proper sentences with
the first letter capitalized etc. Also, I'm missing what is the
thing that this patch is complying with, and why we should care?
A bug fix should never do anything else than fix a bug.
> - remove special handling of the last byte
I have no idea what you mean by send command. Please, refer
to some thing that exists, e.g. a function or what ever.
Nit: try not to use passive form in a commit message. E.g.
"Change ... to comply with" would be better.
>
> Fixes: 27084efee0c3 (tpm: driver for next generation TPM chips)
This is incorrectly formatted, it is missing double quotes. Have you run
your patches through checkpatch.pl? Please review "Describe your
changes" section from Documentation/SubmittingPatches.
/Jarkko
next prev parent reply other threads:[~2021-10-23 0:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-21 12:05 [PATCH v16 0/6] Add tpm i2c ptp driver amirmizi6
2021-10-21 12:05 ` [PATCH v16 1/6] tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
2021-10-23 0:44 ` Jarkko Sakkinen [this message]
2021-10-21 12:05 ` [PATCH v16 2/6] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()" amirmizi6
2021-10-23 0:46 ` Jarkko Sakkinen
2021-10-21 12:05 ` [PATCH v16 3/6] tpm: Handle an exception for TPM Firmware Update mode amirmizi6
2021-10-21 12:05 ` [PATCH v16 4/6] tpm: tpm_tis: verify TPM_STS register is valid after locality request amirmizi6
2021-10-21 12:05 ` [PATCH v16 5/6] tpm: tpm_tis: add tpm_tis_i2c driver amirmizi6
2021-10-21 12:05 ` [PATCH v16 6/6] tpm: Add YAML schema for TPM TIS I2C options amirmizi6
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ae373226e1b7b117f2808facdf0c3b6b0f282a39.camel@kernel.org \
--to=jarkko@kernel.org \
--cc=Dan.Morav@nuvoton.com \
--cc=Eyal.Cohen@nuvoton.com \
--cc=alexander.steffen@infineon.com \
--cc=amir.mizinski@nuvoton.com \
--cc=amirmizi6@gmail.com \
--cc=arnd@arndb.de \
--cc=benoit.houyere@st.com \
--cc=devicetree@vger.kernel.org \
--cc=eajames@linux.ibm.com \
--cc=gcwilson@us.ibm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jgg@ziepe.ca \
--cc=joel@jms.id.au \
--cc=kgoldman@us.ibm.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=oren.tanami@nuvoton.com \
--cc=oshri.alkoby@nuvoton.com \
--cc=oshrialkoby85@gmail.com \
--cc=peterhuewe@gmx.de \
--cc=robh+dt@kernel.org \
--cc=shmulik.hager@nuvoton.com \
--cc=tmaimon77@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).