linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional
Date: Tue, 14 Sep 2021 00:06:07 +0300	[thread overview]
Message-ID: <dd7fbcaafad3259a307f1a2d8a72127e7bcc0630.camel@kernel.org> (raw)
In-Reply-To: <20210913144351.101167-2-amirmizi6@gmail.com>

On Mon, 2021-09-13 at 17:43 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
> 
> Only tpm_tis can use memory-mapped I/O, which is truly mapped into the
> kernel's memory space. Therefore, using ioread16/ioread32/iowrite32 turns
> into a straightforward pointer dereference.
> Some drivers, such as tpm_tis_spi, require more complicated operations to
> read more than one byte at a time and, as a result, will revert to
> read_bytes/write_bytes.
> Therefore, re-implement tpm_tis_{read, write}_{16, 32}, so that they check
> if implementations for {read, write}_{16, 32} in tpm_tis_phys_ops  exist;
> if they do not exist, then revert to {read, write}_bytes().
>  static int tpm_tis_spi_probe(struct spi_device *dev)

This is otherwise good but lacks explanation why this makes sense, other than
somewhat obvious benefit of having less calbacks, which BTW should be stated
explicitly in the commit message too.

You must answer what is your by doing this change, e.g. what you are unable
to do, if the callbacks are removed (just an example).

I get the change, but motivation to do it needs to be there.

/Jarkko


  reply	other threads:[~2021-09-13 21:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional amirmizi6
2021-09-13 21:06   ` Jarkko Sakkinen [this message]
2021-09-13 14:43 ` [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
2021-09-13 21:10   ` Jarkko Sakkinen
2021-09-13 14:43 ` [PATCH v14 3/7] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()" amirmizi6
2021-09-13 14:43 ` [PATCH v14 4/7] tpm: Handle an exception for TPM Firmware Update mode amirmizi6
2021-09-13 14:43 ` [PATCH v14 5/7] tpm: tpm_tis: verify TPM_STS register is valid after locality request amirmizi6
2021-09-13 14:43 ` [PATCH v14 6/7] tpm: tpm_tis: add tpm_tis_i2c driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 7/7] \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=dd7fbcaafad3259a307f1a2d8a72127e7bcc0630.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).