lkml.org 
[lkml]   [2018]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 03/20] tpm: factor out tpm 1.x duration calculation to tpm1-cmd.c
From
Date


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
>

\
 
 \ /
  Last update: 2018-10-17 17:13    [W:0.132 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site