From: Thara Gopinath <thara.gopinath@linaro.org> To: Bhupesh Sharma <bhupesh.sharma@linaro.org>, linux-arm-msm@vger.kernel.org Cc: Bjorn Andersson <bjorn.andersson@linaro.org>, Rob Herring <robh+dt@kernel.org>, Andy Gross <agross@kernel.org>, Herbert Xu <herbert@gondor.apana.org.au>, "David S . Miller" <davem@davemloft.net>, Stephen Boyd <sboyd@kernel.org>, Michael Turquette <mturquette@baylibre.com>, Vinod Koul <vkoul@kernel.org>, dmaengine@vger.kernel.org, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, bhupesh.linux@gmail.com Subject: Re: [PATCH v3 13/17] crypto: qce: core: Make clocks optional Date: Thu, 20 May 2021 22:11:53 -0400 [thread overview] Message-ID: <125e1f83-e340-9cd3-91a8-cd1ee3ee8b7f@linaro.org> (raw) In-Reply-To: <20210519143700.27392-14-bhupesh.sharma@linaro.org> Hi Bhupesh, On 5/19/21 10:36 AM, Bhupesh Sharma wrote: > From: Thara Gopinath <thara.gopinath@linaro.org> > > On certain Snapdragon processors, the crypto engine clocks are enabled by > default by security firmware and the driver need not handle the > clocks. Make acquiring of all the clocks optional in crypto enginer driver > so that the driver intializes properly even if no clocks are specified in > the dt. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andy Gross <agross@kernel.org> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: David S. Miller <davem@davemloft.net> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: dmaengine@vger.kernel.org > Cc: linux-clk@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: bhupesh.linux@gmail.com > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > [ bhupesh.sharma@linaro.org: Make clock enablement optional only for qcom parts where > firmware has already initialized them, using a bool variable and fix > error paths ] > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/crypto/qce/core.c | 89 +++++++++++++++++++++++++-------------- > drivers/crypto/qce/core.h | 2 + > 2 files changed, 59 insertions(+), 32 deletions(-) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 905378906ac7..8c3c68ba579e 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -9,6 +9,7 @@ > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/mod_devicetable.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/spinlock.h> > #include <linux/types.h> > @@ -184,10 +185,20 @@ static int qce_check_version(struct qce_device *qce) > return 0; > } > > +static const struct of_device_id qce_crypto_of_match[] = { > + { .compatible = "qcom,ipq6018-qce", }, > + { .compatible = "qcom,sdm845-qce", }, > + { .compatible = "qcom,sm8250-qce", }, Adding qcom,sm8250-qce does not belong in this patch. It deserves a separate patch of it's own. > + {} > +}; > +MODULE_DEVICE_TABLE(of, qce_crypto_of_match); > + > static int qce_crypto_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct qce_device *qce; > + const struct of_device_id *of_id = > + of_match_device(qce_crypto_of_match, &pdev->dev); > int ret; > > qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL); > @@ -198,45 +209,65 @@ static int qce_crypto_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, qce); > > qce->base = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(qce->base)) > - return PTR_ERR(qce->base); > + if (IS_ERR(qce->base)) { > + ret = PTR_ERR(qce->base); > + goto err_out; > + } I don't see the reason for change in error handling here or below. But ,for whatever reason this is changed, it has to be a separate patch. > > ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); > if (ret < 0) > - return ret; > + goto err_out; > > qce->mem_path = devm_of_icc_get(qce->dev, "memory"); > if (IS_ERR(qce->mem_path)) > return dev_err_probe(dev, PTR_ERR(qce->mem_path), > "Failed to get mem path\n"); > > - qce->core = devm_clk_get(qce->dev, "core"); > - if (IS_ERR(qce->core)) > - return PTR_ERR(qce->core); > - > - qce->iface = devm_clk_get(qce->dev, "iface"); > - if (IS_ERR(qce->iface)) > - return PTR_ERR(qce->iface); > - > - qce->bus = devm_clk_get(qce->dev, "bus"); > - if (IS_ERR(qce->bus)) > - return PTR_ERR(qce->bus); > - > ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH); > if (ret) > - return ret; > + goto err_out; > > - ret = clk_prepare_enable(qce->core); > - if (ret) > - return ret; > + /* On some qcom parts the crypto clocks are already configured by > + * the firmware running before linux. In such cases we don't need to > + * enable/configure them again. Check here for the same. > + */ > + if (!strcmp(of_id->compatible, "qcom,ipq6018-qce") || > + !strcmp(of_id->compatible, "qcom,sdm845-qce")) You can avoid this and most of this patch by using devm_clk_get_optional. This patch can be like just three lines of code change. clk_prepare_enable returns 0 if the clock is null. There is no need to check for the compatibles above. Use devm_clk_get_optional instead of devm_clk_get and everything else can be left as is. Warm Regards Thara > + qce->clks_configured_by_fw = false; > + else > + qce->clks_configured_by_fw = true; > + > + if (!qce->clks_configured_by_fw) { > + qce->core = devm_clk_get(qce->dev, "core"); > + if (IS_ERR(qce->core)) { > + ret = PTR_ERR(qce->core); > + goto err_out; > + } > + > + qce->iface = devm_clk_get(qce->dev, "iface"); > + if (IS_ERR(qce->iface)) { > + ret = PTR_ERR(qce->iface); > + goto err_out; > + } > + > + qce->bus = devm_clk_get(qce->dev, "bus"); > + if (IS_ERR(qce->bus)) { > + ret = PTR_ERR(qce->bus); > + goto err_out; > + } > + > + ret = clk_prepare_enable(qce->core); > + if (ret) > + goto err_out; > > - ret = clk_prepare_enable(qce->iface); > - if (ret) > - goto err_clks_core; > + ret = clk_prepare_enable(qce->iface); > + if (ret) > + goto err_clks_core; > > - ret = clk_prepare_enable(qce->bus); > - if (ret) > - goto err_clks_iface; > + ret = clk_prepare_enable(qce->bus); > + if (ret) > + goto err_clks_iface; > + } > > ret = qce_dma_request(qce->dev, &qce->dma); > if (ret) > @@ -268,6 +299,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > clk_disable_unprepare(qce->iface); > err_clks_core: > clk_disable_unprepare(qce->core); > +err_out: > return ret; > } > > @@ -284,13 +316,6 @@ static int qce_crypto_remove(struct platform_device *pdev) > return 0; > } > > -static const struct of_device_id qce_crypto_of_match[] = { > - { .compatible = "qcom,ipq6018-qce", }, > - { .compatible = "qcom,sdm845-qce", }, > - {} > -}; > -MODULE_DEVICE_TABLE(of, qce_crypto_of_match); > - > static struct platform_driver qce_crypto_driver = { > .probe = qce_crypto_probe, > .remove = qce_crypto_remove, > diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h > index 228fcd69ec51..d9bf05babecc 100644 > --- a/drivers/crypto/qce/core.h > +++ b/drivers/crypto/qce/core.h > @@ -23,6 +23,7 @@ > * @dma: pointer to dma data > * @burst_size: the crypto burst size > * @pipe_pair_id: which pipe pair id the device using > + * @clks_configured_by_fw: clocks are already configured by fw > * @async_req_enqueue: invoked by every algorithm to enqueue a request > * @async_req_done: invoked by every algorithm to finish its request > */ > @@ -39,6 +40,7 @@ struct qce_device { > struct qce_dma_data dma; > int burst_size; > unsigned int pipe_pair_id; > + bool clks_configured_by_fw; > int (*async_req_enqueue)(struct qce_device *qce, > struct crypto_async_request *req); > void (*async_req_done)(struct qce_device *qce, int ret); >
next prev parent reply other threads:[~2021-05-21 2:11 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-19 14:36 [PATCH v3 00/17] Enable Qualcomm Crypto Engine on sm8250 Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 01/17] dt-bindings: qcom-bam: Convert binding to YAML Bhupesh Sharma 2021-05-21 1:43 ` Rob Herring 2021-06-04 3:27 ` Bhupesh Sharma 2021-07-29 19:34 ` Rob Herring 2021-05-21 8:08 ` Stephan Gerhold 2021-06-04 3:40 ` Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 02/17] dt-bindings: qcom-bam: Add 'interconnects' & 'interconnect-names' to optional properties Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 03/17] dt-bindings: qcom-bam: Add 'iommus' to required properties Bhupesh Sharma 2021-05-21 1:44 ` Rob Herring 2021-05-21 8:11 ` Stephan Gerhold 2021-05-19 14:36 ` [PATCH v3 04/17] dt-bindings: qcom-qce: Convert bindings to yaml Bhupesh Sharma 2021-05-21 1:45 ` Rob Herring 2021-06-04 3:41 ` Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 05/17] dt-bindings: qcom-qce: Add 'interconnects' and move 'clocks' to optional properties Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 06/17] dt-bindings: qcom-qce: Add 'iommus' to required properties Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 07/17] arm64/dts: qcom: sdm845: Use RPMH_CE_CLK macro directly Bhupesh Sharma 2021-05-21 1:47 ` Thara Gopinath 2021-05-19 14:36 ` [PATCH v3 08/17] dt-bindings: crypto : Add new compatible strings for qcom-qce Bhupesh Sharma 2021-05-21 1:46 ` Rob Herring 2021-06-05 8:33 ` Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 09/17] arm64/dts: qcom: Use new compatibles for crypto nodes Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 10/17] dma: qcom: bam_dma: Add support to initialize interconnect path Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 11/17] crypto: qce: core: " Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 12/17] crypto: qce: Add new compatibles for qce crypto driver Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 13/17] crypto: qce: core: Make clocks optional Bhupesh Sharma 2021-05-21 2:11 ` Thara Gopinath [this message] 2021-06-05 8:31 ` Bhupesh Sharma 2021-05-19 14:36 ` [PATCH v3 14/17] crypto: qce: Print a failure msg in case probe() fails Bhupesh Sharma 2021-05-21 1:55 ` Thara Gopinath 2021-05-19 14:36 ` [PATCH v3 15/17] crypto: qce: Convert the device found dev_dbg() to dev_info() Bhupesh Sharma 2021-05-21 1:50 ` Thara Gopinath 2021-05-19 14:36 ` [PATCH v3 16/17] crypto: qce: Defer probing if BAM dma channel is not yet initialized Bhupesh Sharma 2021-05-21 1:57 ` Thara Gopinath 2021-06-05 8:26 ` Bhupesh Sharma 2021-05-19 14:37 ` [PATCH v3 17/17] arm64/dts: qcom: sm8250: Add dt entries to support crypto engine Bhupesh Sharma [not found] ` <162261866806.4130789.17734233133141728573@swboyd.mtv.corp.google.com> 2021-06-04 3:18 ` [PATCH v3 00/17] Enable Qualcomm Crypto Engine on sm8250 Bhupesh Sharma
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=125e1f83-e340-9cd3-91a8-cd1ee3ee8b7f@linaro.org \ --to=thara.gopinath@linaro.org \ --cc=agross@kernel.org \ --cc=bhupesh.linux@gmail.com \ --cc=bhupesh.sharma@linaro.org \ --cc=bjorn.andersson@linaro.org \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=dmaengine@vger.kernel.org \ --cc=herbert@gondor.apana.org.au \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-crypto@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mturquette@baylibre.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=vkoul@kernel.org \ --subject='Re: [PATCH v3 13/17] crypto: qce: core: Make clocks optional' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).