Re: [tpmdd-devel] [PATCH v5 7/7] tpm: replace or remove printk error messages

[Nayna <na...@li...>]

On 10/21/2016 08:32 PM, Jarkko Sakkinen wrote:
> On Fri, Oct 21, 2016 at 08:52:14AM +0530, Nayna wrote:
>>
>>
>> On 10/20/2016 04:54 PM, Jarkko Sakkinen wrote:
>>> On Thu, Oct 20, 2016 at 07:34:37AM +0000, Winkler, Tomas wrote:
>>>>> On Tue, Oct 18, 2016 at 08:49:45PM -0400, Nayna Jain wrote:
>>>>>> This patch removes the unnecessary error messages on failing to
>>>>>> allocate memory and replaces pr_err/printk with dev_dbg/dev_info as
>>>>>> applicable.
>>>>>>
>>>>>> Suggested-by: Jason Gunthorpe <jgu...@ob...>
>>>>>> Signed-off-by: Nayna Jain <na...@li...>
>>>>>
>>>>> Reviewed-by: Jarkko Sakkinen <jar...@li...>
>>>>>
>>>>> /Jarkko
>>>>>
>>>>>> ---
>>>>>>    drivers/char/tpm/tpm_acpi.c | 17 +++++------------
>>>>>>    drivers/char/tpm/tpm_of.c   | 30 ++++++++++--------------------
>>>>>>    2 files changed, 15 insertions(+), 32 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
>>>>>> index 859bdba..22e42da 100644
>>>>>> --- a/drivers/char/tpm/tpm_acpi.c
>>>>>> +++ b/drivers/char/tpm/tpm_acpi.c
>>>>>> @@ -60,11 +60,8 @@ int read_log_acpi(struct tpm_chip *chip)
>>>>>>    	status = acpi_get_table(ACPI_SIG_TCPA, 1,
>>>>>>    				(struct acpi_table_header **)&buff);
>>>>>>
>>>>>> -	if (ACPI_FAILURE(status)) {
>>>>>> -		printk(KERN_ERR "%s: ERROR - Could not get TCPA table\n",
>>>>>> -		       __func__);
>>>>>> +	if (ACPI_FAILURE(status))
>>>>>>    		return -EIO;
>>>>>> -	}
>>>>>>
>>>>>>    	switch(buff->platform_class) {
>>>>>>    	case BIOS_SERVER:
>>>>>> @@ -78,25 +75,21 @@ int read_log_acpi(struct tpm_chip *chip)
>>>>>>    		break;
>>>>>>    	}
>>>>>>    	if (!len) {
>>>>>> -		printk(KERN_ERR "%s: ERROR - TCPA log area empty\n",
>>>>> __func__);
>>>>>> +		dev_dbg(&chip->dev, "%s: ERROR - TCPA log area empty\n",
>>>>>> +			__func__);
>>>>
>>>>
>>>> Why to keep __func__ here, dev_dbg already does it for you.
>>>
>>> Good catch. I would actually consider also changing this to
>>>
>>> dev_err(dev, FW_BUG "TCPA log area empty\n");
>>>
>>> If TCPA exists but it's empty, that's most likely a FW bug.
>>
>> If it can be possibly a FW bug, should it fail the probe also just like
>> -ENOMEM error ?
>
> I think so but I hold for second opinion on this. I mean wouldn't
> it seem like a bit broken situation where TCPA tabe would exist but
> would also be empty?

Hmm, is it possible for this to be firmware implementation dependent ?

Thanks & Regards,
    - Nayna
>
> /Jarkko
>