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

[Nayna Jain]

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
>