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 
> > > > > > > > > <jguntho...@obsidianresearch.com>
> > > > > > > > > Signed-off-by: Nayna Jain <na...@linux.vnet.ibm.com>
> > > > > > > > 
> > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@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
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Reply via email to