| Message ID | 1476838185-24007-8-git-send-email-nayna@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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@obsidianresearch.com> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /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__); > 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
> 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@obsidianresearch.com> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > /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. > > 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
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@obsidianresearch.com> > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > /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. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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@obsidianresearch.com> >>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> >> >> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> >> /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
On Thu, Oct 20, 2016 at 07:01:51PM +0530, Nayna wrote: > > > 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@obsidianresearch.com> > > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > /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 Sorry I mixed up in my last response. You can filter in dynamic debug with a function name but doesn't print function name. Anyway I would recommend to change this to dev_err in a way that I explained in my response. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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@obsidianresearch.com> >>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> >>> >>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> >>> /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
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 <jgunthorpe@obsidianresearch.com> > > > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > /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? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 <jgunthorpe@obsidianresearch.com> >>>>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> >>>>> >>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>> >>>>> /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 > ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
On Wed, Oct 26, 2016 at 07:52:53AM +0530, Nayna wrote: > > > 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 <jgunthorpe@obsidianresearch.com> > > > > > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > > > > > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > > > > > /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 ? Let me put it this way. Why would anyone expose TCPA to the user space that is empty? What would be the point? /Jarkko ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
On 10/26/2016 04:26 PM, Jarkko Sakkinen wrote: > On Wed, Oct 26, 2016 at 07:52:53AM +0530, Nayna wrote: >> >> >> 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 <jgunthorpe@obsidianresearch.com> >>>>>>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> >>>>>>> >>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>> >>>>>>> /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 ? > > Let me put it this way. Why would anyone expose TCPA to the user space > that is empty? What would be the point? If I understand correctly, the reserved memory for event log would be allocated much earlier and firmware would directly write to this allocated memory. So, if there is a firmware bug, and events are not recorded, it will get exposed to userspace as empty event log and this might also help applications to know that it is broken. Is there any wrong assumption here ? Thanks & Regards, - Nayna > > /Jarkko > ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
On Wed, Oct 26, 2016 at 11:01:00PM +0530, Nayna wrote: > > > On 10/26/2016 04:26 PM, Jarkko Sakkinen wrote: > > On Wed, Oct 26, 2016 at 07:52:53AM +0530, Nayna wrote: > > > > > > > > > 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 <jgunthorpe@obsidianresearch.com> > > > > > > > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > > > > > > > > > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > > > > > > > > > /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 ? > > > > Let me put it this way. Why would anyone expose TCPA to the user space > > that is empty? What would be the point? > > If I understand correctly, the reserved memory for event log would be > allocated much earlier and firmware would directly write to this allocated > memory. So, if there is a firmware bug, and events are not recorded, it will > get exposed to userspace as empty event log and this might also help > applications to know that it is broken. Is there any wrong assumption here > ? I think the right question to ask is can event log be legally empty? If not, then FW_BUG should be used here. /Jarkko ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
On 10/27/2016 07:23 PM, Jarkko Sakkinen wrote: > On Wed, Oct 26, 2016 at 11:01:00PM +0530, Nayna wrote: >> >> >> On 10/26/2016 04:26 PM, Jarkko Sakkinen wrote: >>> On Wed, Oct 26, 2016 at 07:52:53AM +0530, Nayna wrote: >>>> >>>> >>>> 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 <jgunthorpe@obsidianresearch.com> >>>>>>>>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> >>>>>>>>> >>>>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>> >>>>>>>>> /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 ? >>> >>> Let me put it this way. Why would anyone expose TCPA to the user space >>> that is empty? What would be the point? >> >> If I understand correctly, the reserved memory for event log would be >> allocated much earlier and firmware would directly write to this allocated >> memory. So, if there is a firmware bug, and events are not recorded, it will >> get exposed to userspace as empty event log and this might also help >> applications to know that it is broken. Is there any wrong assumption here >> ? > > I think the right question to ask is can event log be legally empty? No > If not, then FW_BUG should be used here. Ok. yeah. and then I think we would want it to fail the probe ? Thanks & Regards, - Nayna > > /Jarkko > ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
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@obsidianresearch.com> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> Why subject line is "replace or remove"? What does that mean? If you have both, maybe using the phrase "clean up" would be better. /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__); > 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 > ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
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__); 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; }
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@obsidianresearch.com> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> --- drivers/char/tpm/tpm_acpi.c | 17 +++++------------ drivers/char/tpm/tpm_of.c | 30 ++++++++++-------------------- 2 files changed, 15 insertions(+), 32 deletions(-)