Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756423AbcJHMrU (ORCPT ); Sat, 8 Oct 2016 08:47:20 -0400 Received: from mga11.intel.com ([192.55.52.93]:62285 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbcJHMrN (ORCPT ); Sat, 8 Oct 2016 08:47:13 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,460,1473145200"; d="scan'208";a="17995944" Date: Sat, 8 Oct 2016 15:47:08 +0300 From: Jarkko Sakkinen To: Tomas Winkler Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe , linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/4] tmp/tpm_crb: implement runtime pm for tpm_crb Message-ID: <20161008124708.GA10317@intel.com> References: <1475927979-23484-1-git-send-email-tomas.winkler@intel.com> <1475927979-23484-5-git-send-email-tomas.winkler@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475927979-23484-5-git-send-email-tomas.winkler@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4032 Lines: 139 On Sat, Oct 08, 2016 at 02:59:39PM +0300, Tomas Winkler wrote: > From: "Winkler, Tomas" > > Utilize runtime_pm for driving tpm crb idle states. > The framework calls cmd_ready from the pm_runtime_resume handler > and go idle from the pm_runtime_suspend handler. > The TPM framework should wake the device before transmit and receive. > In case the runtime_pm framework is not compiled in or enabled, the device > will be in the permanent ready state. > > Signed-off-by: Tomas Winkler I would rather want the fix as a separate patch to make review + testing easier (without and with). Thanks. /Jarkko > --- > V2: new in the series > V3: resend > V4: fix unbalanced runtime pm put and get > > drivers/char/tpm/tpm-interface.c | 5 +++++ > drivers/char/tpm/tpm_crb.c | 42 ++++++++++++++++++++++++++++++++++++---- > 2 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 8de61876f633..7743e8a21b82 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > #include "tpm.h" > #include "tpm_eventlog.h" > @@ -356,6 +357,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz, > if (!(flags & TPM_TRANSMIT_UNLOCKED)) > mutex_lock(&chip->tpm_mutex); > > + pm_runtime_get_sync(chip->dev.parent); > + > rc = chip->ops->send(chip, (u8 *) buf, count); > if (rc < 0) { > dev_err(&chip->dev, > @@ -397,6 +400,8 @@ out_recv: > dev_err(&chip->dev, > "tpm_transmit: tpm_recv: error %zd\n", rc); > out: > + pm_runtime_put_sync(chip->dev.parent); > + > if (!(flags & TPM_TRANSMIT_UNLOCKED)) > mutex_unlock(&chip->tpm_mutex); > return rc; > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index def50b7d47b2..eccd82af6215 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include "tpm.h" > > #define ACPI_SIG_TPM2 "TPM2" > @@ -153,8 +154,6 @@ static int __maybe_unused crb_cmd_ready(struct device *dev, > return 0; > } > > -static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume); > - > static u8 crb_status(struct tpm_chip *chip) > { > struct crb_priv *priv = dev_get_drvdata(&chip->dev); > @@ -437,11 +436,21 @@ static int crb_acpi_add(struct acpi_device *device) > if (rc) > return rc; > > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + > rc = tpm_chip_register(chip); > - if (rc) > + if (rc) { > crb_go_idle(dev, priv); > + pm_runtime_put_noidle(dev); > + pm_runtime_disable(dev); > + return rc; > + } > > - return rc; > + pm_runtime_put(dev); > + > + return 0; > } > > static int crb_acpi_remove(struct acpi_device *device) > @@ -451,9 +460,34 @@ static int crb_acpi_remove(struct acpi_device *device) > > tpm_chip_unregister(chip); > > + pm_runtime_disable(dev); > + > return 0; > } > > +#ifdef CONFIG_PM > +static int crb_pm_runtime_suspend(struct device *dev) > +{ > + struct tpm_chip *chip = dev_get_drvdata(dev); > + struct crb_priv *priv = dev_get_drvdata(&chip->dev); > + > + return crb_go_idle(dev, priv); > +} > + > +static int crb_pm_runtime_resume(struct device *dev) > +{ > + struct tpm_chip *chip = dev_get_drvdata(dev); > + struct crb_priv *priv = dev_get_drvdata(&chip->dev); > + > + return crb_cmd_ready(dev, priv); > +} > +#endif /* CONFIG_PM */ > + > +static const struct dev_pm_ops crb_pm = { > + SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume) > + SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL) > +}; > + > static struct acpi_device_id crb_device_ids[] = { > {"MSFT0101", 0}, > {"", 0}, > -- > 2.7.4 >