[v2,5/7] crypto: omap-sham: Convert to use pm_runtime API

Commit Message

Mark Greer Oct. 24, 2012, 2:36 a.m. UTC

From: "Mark A. Greer" <[e-mail]>

Convert the omap-sham crypto driver to use the
pm_runtime API instead of the clk API.

CC: Kevin Hilman <[e-mail]>
CC: Paul Walmsley <[e-mail]>
CC: Dmitry Kasatkin <[e-mail]>
Signed-off-by: Mark A. Greer <[e-mail]>
---
 drivers/crypto/omap-sham.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

Comments

Kevin Hilman Oct. 26, 2012, 12:34 a.m. UTC | #1

Hi Mark,

"Mark A. Greer" <[e-mail]> writes:

> From: "Mark A. Greer" <[e-mail]>
>
> Convert the omap-sham crypto driver to use the
> pm_runtime API instead of the clk API.
>
> CC: Kevin Hilman <[e-mail]>
> CC: Paul Walmsley <[e-mail]>
> CC: Dmitry Kasatkin <[e-mail]>
> Signed-off-by: Mark A. Greer <[e-mail]>

I can't pretend to fully understand this driver, It looks like the
current code is doing a bit more fine-grained clock gating, and leaving
the IP clocked only when needed.

The proposed version does a 'get' in probe and a 'put' in remove, which
means the IP will always be enabled (and thus preventing low-power
states), even when it's not in use.

If that's really needed, it should be thoroughly described in the
changelog, otherwise I suggest doing the runtime PM 'get' and 'put' in
roughtly the same spots as the current clk enable/disable which makes
this a more straight-forward conversion.

Kevin

Mark Greer Oct. 26, 2012, 5:10 p.m. UTC | #2

On Thu, Oct 25, 2012 at 05:34:02PM -0700, Kevin Hilman wrote:
> Hi Mark,

Hi Kevin.

> "Mark A. Greer" <[e-mail]> writes:
> 
> > From: "Mark A. Greer" <[e-mail]>
> >
> > Convert the omap-sham crypto driver to use the
> > pm_runtime API instead of the clk API.
> >
> > CC: Kevin Hilman <[e-mail]>
> > CC: Paul Walmsley <[e-mail]>
> > CC: Dmitry Kasatkin <[e-mail]>
> > Signed-off-by: Mark A. Greer <[e-mail]>
> 
> I can't pretend to fully understand this driver, It looks like the
> current code is doing a bit more fine-grained clock gating, and leaving
> the IP clocked only when needed.
> 
> The proposed version does a 'get' in probe and a 'put' in remove, which
> means the IP will always be enabled (and thus preventing low-power
> states), even when it's not in use.

You're right, and what I did is wrong.  I'll fix this in the next revision.

Thanks,

Mark
--

Patch

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index a3fd6fc..acb85df 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -22,12 +22,12 @@ 
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
-#include <linux/clk.h>
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/scatterlist.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
 #include <linux/delay.h>
 #include <linux/crypto.h>
 #include <linux/cryptohash.h>
@@ -141,7 +141,6 @@  struct omap_sham_dev {
 	struct device		*dev;
 	void __iomem		*io_base;
 	int			irq;
-	struct clk		*iclk;
 	spinlock_t		lock;
 	int			err;
 	int			dma;
@@ -238,8 +237,6 @@  static void omap_sham_copy_ready_hash(struct ahash_request *req)
 
 static int omap_sham_hw_init(struct omap_sham_dev *dd)
 {
-	clk_enable(dd->iclk);
-
 	if (!test_bit(FLAGS_INIT, &dd->flags)) {
 		omap_sham_write_mask(dd, SHA_REG_MASK,
 			SHA_REG_MASK_SOFTRESET, SHA_REG_MASK_SOFTRESET);
@@ -653,7 +650,6 @@  static void omap_sham_finish_req(struct ahash_request *req, int err)
 	/* atomic operation is not needed here */
 	dd->flags &= ~(BIT(FLAGS_BUSY) | BIT(FLAGS_FINAL) | BIT(FLAGS_CPU) |
 			BIT(FLAGS_DMA_READY) | BIT(FLAGS_OUTPUT_READY));
-	clk_disable(dd->iclk);
 
 	if (req->base.complete)
 		req->base.complete(&req->base, err);
@@ -1198,14 +1194,6 @@  static int __devinit omap_sham_probe(struct platform_device *pdev)
 	if (err)
 		goto dma_err;
 
-	/* Initializing the clock */
-	dd->iclk = clk_get(dev, "ick");
-	if (IS_ERR(dd->iclk)) {
-		dev_err(dev, "clock intialization failed.\n");
-		err = PTR_ERR(dd->iclk);
-		goto clk_err;
-	}
-
 	dd->io_base = ioremap(dd->phys_base, SZ_4K);
 	if (!dd->io_base) {
 		dev_err(dev, "can't ioremap\n");
@@ -1213,11 +1201,12 @@  static int __devinit omap_sham_probe(struct platform_device *pdev)
 		goto io_err;
 	}
 
-	clk_enable(dd->iclk);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
 	dev_info(dev, "hw accel on OMAP rev %u.%u\n",
 		(omap_sham_read(dd, SHA_REG_REV) & SHA_REG_REV_MAJOR) >> 4,
 		omap_sham_read(dd, SHA_REG_REV) & SHA_REG_REV_MINOR);
-	clk_disable(dd->iclk);
 
 	spin_lock(&sham.lock);
 	list_add_tail(&dd->list, &sham.dev_list);
@@ -1235,9 +1224,8 @@  err_algs:
 	for (j = 0; j < i; j++)
 		crypto_unregister_ahash(&algs[j]);
 	iounmap(dd->io_base);
+	pm_runtime_disable(dev);
 io_err:
-	clk_put(dd->iclk);
-clk_err:
 	omap_sham_dma_cleanup(dd);
 dma_err:
 	if (dd->irq >= 0)
@@ -1266,7 +1254,8 @@  static int __devexit omap_sham_remove(struct platform_device *pdev)
 		crypto_unregister_ahash(&algs[i]);
 	tasklet_kill(&dd->done_task);
 	iounmap(dd->io_base);
-	clk_put(dd->iclk);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 	omap_sham_dma_cleanup(dd);
 	if (dd->irq >= 0)
 		free_irq(dd->irq, dd);