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

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

On 10/20/2016 01:04 PM, 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.

I tried to trace through dev_dbg
- http://lxr.free-electrons.com/source/include/linux/device.h#L1195
which further calls dev_printk from 
http://lxr.free-electrons.com/source/drivers/base/core.c#L2199

I couldn't find it printing __func__ anywhere.. I just see it printing 
"driver" and "dev" as tpm : tpm0.

Please let me know if I am missing something

Thanks & Regards,
- Nayna
>
>>>   		return -EIO;
>>>   	}
>>>
>>>   	/* malloc EventLog space */
>>>   	log->bios_event_log = kmalloc(len, GFP_KERNEL);
>>> -	if (!log->bios_event_log) {
>>> -		printk("%s: ERROR - Not enough  Memory for BIOS
>> measurements\n",
>>> -			__func__);
>>> +	if (!log->bios_event_log)
>>>   		return -ENOMEM;
>>> -	}
>>>
>>>   	log->bios_event_log_end = log->bios_event_log + len;
>>>
>>>   	virt = acpi_os_map_iomem(start, len);
>>> -	if (!virt) {
>>> -		printk("%s: ERROR - Unable to map memory\n", __func__);
>>> +	if (!virt)
>>>   		goto err;
>>> -	}
>>>
>>>   	memcpy_fromio(log->bios_event_log, virt, len);
>>>
>>> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
>>> index 22b8f81..8b7e677 100644
>>> --- a/drivers/char/tpm/tpm_of.c
>>> +++ b/drivers/char/tpm/tpm_of.c
>>> @@ -31,40 +31,30 @@ int read_log_of(struct tpm_chip *chip)
>>>   	log = &chip->log;
>>>   	if (chip->dev.parent->of_node)
>>>   		np = chip->dev.parent->of_node;
>>> -	if (!np) {
>>> -		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
>>> +	if (!np)
>>>   		return -ENODEV;
>>> -	}
>>>
>>>   	sizep = of_get_property(np, "linux,sml-size", NULL);
>>> -	if (sizep == NULL) {
>>> -		pr_err("%s: ERROR - SML size not found\n", __func__);
>>> -		goto cleanup_eio;
>>> -	}
>>> +	if (sizep == NULL)
>>> +		return -EIO;
>>> +
>>>   	if (*sizep == 0) {
>>> -		pr_err("%s: ERROR - event log area empty\n", __func__);
>>> -		goto cleanup_eio;
>>> +		dev_info(&chip->dev, "%s: ERROR - event log area empty\n",
>>> +			__func__);
>>> +		return -EIO;
>>>   	}
>>>
>>>   	basep = of_get_property(np, "linux,sml-base", NULL);
>>> -	if (basep == NULL) {
>>> -		pr_err("%s: ERROR - SML not found\n", __func__);
>>> -		goto cleanup_eio;
>>> -	}
>>> +	if (basep == NULL)
>>> +		return -EIO;
>>>
>>>   	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
>>> -	if (!log->bios_event_log) {
>>> -		pr_err("%s: ERROR - Not enough memory for BIOS
>> measurements\n",
>>> -		       __func__);
>>> +	if (!log->bios_event_log)
>>>   		return -ENOMEM;
>>> -	}
>>>
>>>   	log->bios_event_log_end = log->bios_event_log + *sizep;
>>>
>>>   	memcpy(log->bios_event_log, __va(*basep), *sizep);
>>>
>>>   	return 0;
>>> -
>>> -cleanup_eio:
>>> -	return -EIO;
>>>   }
>>> --
>>> 2.5.0
>>>
>>
>


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