Re: [PATCH v6 03/20] tpm: factor out tpm 1.x duration calculation to tpm1-cmd.c

From: Nayna Jain
Date: Wed Oct 17 2018 - 11:12:23 EST

On 10/17/2018 05:54 PM, Winkler, Tomas wrote:

ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
- rc = i2c_nuvoton_wait_for_data_avail(chip,
- tpm_calc_ordinal_duration(chip,
- ordinal),
- &priv->read_queue);
+ duration = tpm1_calc_ordinal_duration(chip, ordinal);

This version of the patch didn't address my previous comment - "The original
code in the nuvoton driver does not differentiate between TPM 1.2 and TPM
2.0 as it does in tpm_tis_core.c.
Before making any changes, I would first fix it, so that it could easily be
backported. Only then do the refactoring."
This patch doesn't change the original behavior, just change the name of the function, so there is no regression.
I would suggest there is another bug in those drivers/devices that is orthogonal to this refactoring and should not block this from merging.

The problem is that you are inadvertently fixing a bug without realizing it - [Patch 04/20]. Bug fixes should be addressed independently
of this change, so that they can be backported properly.

According to what you say it can call just tpm_calc_oridnal_duration() instead of tpm1_calc_ordinal_duration(chip, ordinal),
but I prefer that someone that has those devices will do that change on top of this series as I cannot test it.

The problem is:

1. This patch calls tpm1_calc_ordinal_duration for both the TPM 1.2 and TPM 2.0.
2. In the next patch, it adds a new function tpm_calc_ordinal_duration as a wrapper for both the TPM 1.2 and TPM 2.0. After this change when it calls tpm_calc_ordinal_duration(),
it now calls different functions for TPM 1.2 and TPM 2.0. This is a change in behavior.

Thanks & Regards,
ÂÂ - Nayna