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

[Nayna <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]

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 <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>>>> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>
>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>
>>> /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 ?

Thanks & Regards,
     - Nayna

>
> /Jarkko
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot