2016-09-03 16:45:57

by Iaroslav Gridin

Subject: [PATCH] crypto: qce: Initialize core src clock @100Mhz

Without that, QCE performance is about 2x less.

Signed-off-by: Iaroslav Gridin <[email protected]>
---
drivers/crypto/qce/core.c | 18 +++++++++++++++++-
drivers/crypto/qce/core.h | 2 +-
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 0cde513..657354c 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -193,6 +193,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret < 0)
return ret;

+ qce->core_src = devm_clk_get(qce->dev, "core_src");
+ if (IS_ERR(qce->core_src))
+ return PTR_ERR(qce->core_src);
+
qce->core = devm_clk_get(qce->dev, "core");
if (IS_ERR(qce->core))
return PTR_ERR(qce->core);
@@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (IS_ERR(qce->bus))
return PTR_ERR(qce->bus);

- ret = clk_prepare_enable(qce->core);
+ ret = clk_prepare_enable(qce->core_src);
if (ret)
return ret;

+ ret = clk_set_rate(qce->core_src, 100000000);
+ if (ret) {
+ dev_warn(qce->dev, "Unable to set QCE core src clk @100Mhz, performance might be degraded\n");
+ goto err_clks_core_src;
+ }
+
+ ret = clk_prepare_enable(qce->core);
+ if (ret)
+ goto err_clks_core_src;
+
ret = clk_prepare_enable(qce->iface);
if (ret)
goto err_clks_core;
@@ -247,6 +261,8 @@ err_clks_iface:
clk_disable_unprepare(qce->iface);
err_clks_core:
clk_disable_unprepare(qce->core);
+err_clks_core_src:
+ clk_disable_unprepare(qce->core_src);
return ret;
}

diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index 549965d..c5f8d08 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -42,7 +42,7 @@ struct qce_device {
int result;
void __iomem *base;
struct device *dev;
- struct clk *core, *iface, *bus;
+ struct clk *core, *iface, *bus, *core_src;
struct qce_dma_data dma;
int burst_size;
unsigned int pipe_pair_id;
--
2.9.3

2016-09-07 12:50:17

by Herbert Xu

Subject: Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz

On Sat, Sep 03, 2016 at 07:45:35PM +0300, Iaroslav Gridin wrote:
>
> @@ -247,6 +261,8 @@ err_clks_iface:
> clk_disable_unprepare(qce->iface);
> err_clks_core:
> clk_disable_unprepare(qce->core);
> +err_clks_core_src:
> + clk_disable_unprepare(qce->core_src);

What about qce_crypto_remove?

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-09-07 13:04:01

by Stanimir Varbanov

Subject: Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz

Hi Iaroslav,

On 09/03/2016 07:45 PM, Iaroslav Gridin wrote:
> Without that, QCE performance is about 2x less.

On which platform? The clock rates are per SoC.

>
> Signed-off-by: Iaroslav Gridin <[email protected]>
> ---
> drivers/crypto/qce/core.c | 18 +++++++++++++++++-
> drivers/crypto/qce/core.h | 2 +-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 0cde513..657354c 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -193,6 +193,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + qce->core_src = devm_clk_get(qce->dev, "core_src");
> + if (IS_ERR(qce->core_src))
> + return PTR_ERR(qce->core_src);
> +
> qce->core = devm_clk_get(qce->dev, "core");
> if (IS_ERR(qce->core))
> return PTR_ERR(qce->core);
> @@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (IS_ERR(qce->bus))
> return PTR_ERR(qce->bus);
>
> - ret = clk_prepare_enable(qce->core);
> + ret = clk_prepare_enable(qce->core_src);
> if (ret)
> return ret;
>
> + ret = clk_set_rate(qce->core_src, 100000000);

Could you point me from where you got this number? Also I think you
shouldn't be requesting "core_src" it should be a parent of "core" clock
in the clock tree. Did you tried to set rate on "core" clock?

regards,
Stan

2016-09-07 16:13:22

by Iaroslav Gridin

Subject: Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz

On Wed, Sep 07, 2016 at 04:04:01PM +0300, Stanimir Varbanov wrote:
> Hi Iaroslav,
>
> On 09/03/2016 07:45 PM, Iaroslav Gridin wrote:
> > Without that, QCE performance is about 2x less.
>
> On which platform? The clock rates are per SoC.

Dragonboard 8074. Should clock rate be moved to its DT?

> >
> > Signed-off-by: Iaroslav Gridin <[email protected]>
> > ---
> > drivers/crypto/qce/core.c | 18 +++++++++++++++++-
> > drivers/crypto/qce/core.h | 2 +-
> > 2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> > index 0cde513..657354c 100644
> > --- a/drivers/crypto/qce/core.c
> > +++ b/drivers/crypto/qce/core.c
> > @@ -193,6 +193,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
> > if (ret < 0)
> > return ret;
> >
> > + qce->core_src = devm_clk_get(qce->dev, "core_src");
> > + if (IS_ERR(qce->core_src))
> > + return PTR_ERR(qce->core_src);
> > +
> > qce->core = devm_clk_get(qce->dev, "core");
> > if (IS_ERR(qce->core))
> > return PTR_ERR(qce->core);
> > @@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device *pdev)
> > if (IS_ERR(qce->bus))
> > return PTR_ERR(qce->bus);
> >
> > - ret = clk_prepare_enable(qce->core);
> > + ret = clk_prepare_enable(qce->core_src);
> > if (ret)
> > return ret;
> >
> > + ret = clk_set_rate(qce->core_src, 100000000);
>
> Could you point me from where you got this number? Also I think you
> shouldn't be requesting "core_src" it should be a parent of "core" clock
> in the clock tree. Did you tried to set rate on "core" clock?

Tried it, helps with speed as well.

2016-09-07 17:25:01

by Iaroslav Gridin

Subject: Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz

> > + ret = clk_set_rate(qce->core_src, 100000000);
>
> Could you point me from where you got this number?

I got it from codeaurora qce driver:
https://android.googlesource.com/kernel/msm/+/android-msm-hammerhead-3.4-kk-r1/drivers/crypto/msm/qce50.c#3386

2016-09-13 04:00:12

by Bjorn Andersson

Subject: Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz

On Sat 03 Sep 09:45 PDT 2016, Iaroslav Gridin wrote:

> Without that, QCE performance is about 2x less.
>
> Signed-off-by: Iaroslav Gridin <[email protected]>
> ---
> drivers/crypto/qce/core.c | 18 +++++++++++++++++-
> drivers/crypto/qce/core.h | 2 +-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
[..]
> @@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (IS_ERR(qce->bus))
> return PTR_ERR(qce->bus);
>
> - ret = clk_prepare_enable(qce->core);
> + ret = clk_prepare_enable(qce->core_src);
> if (ret)
> return ret;
>
> + ret = clk_set_rate(qce->core_src, 100000000);
> + if (ret) {
> + dev_warn(qce->dev, "Unable to set QCE core src clk @100Mhz, performance might be degraded\n");

This warning is misleading as you return a failure from probe() when it
happens.

> + goto err_clks_core_src;
> + }
> +
[..]
> +err_clks_core_src:
> + clk_disable_unprepare(qce->core_src);
> return ret;
> }
>

Regards,
Bjorn