Re: [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional

From: Jarkko Sakkinen
Date: Mon Sep 13 2021 - 17:06:13 EST


On Mon, 2021-09-13 at 17:43 +0300, amirmizi6@xxxxxxxxx wrote:
> From: Amir Mizinski <amirmizi6@xxxxxxxxx>
>
> 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