Messages in this thread | | | Subject | Re: [PATCH v6 03/20] tpm: factor out tpm 1.x duration calculation to tpm1-cmd.c | From | Nayna Jain <> | Date | Wed, 17 Oct 2018 20:37:21 +0530 |
| |
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
> Thanks > Tomas >
| |