| Message ID | 20230222172240.3235972-10-vladimir.zapolskiy@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: qcom-qce: Add YAML bindings and support for newer SoCs | expand |
On 22.02.2023 18:22, Vladimir Zapolskiy wrote: > From: Thara Gopinath <thara.gopinath@gmail.com> > > On certain Snapdragon processors, the crypto engine clocks are enabled by > default by security firmware and the driver should not handle the clocks. > Make acquiring of all the clocks optional in crypto engine driver, so that > the driver initializes properly even if no clocks are specified in the dt. > > Tested-by: Jordan Crouse <jorcrous@amazon.com> > Signed-off-by: Thara Gopinath <thara.gopinath@gmail.com> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > [Bhupesh: Massage the commit log] > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- I'm not sure which is the preferred approach, but generally I'd stick with keeping them non-optional for the SoCs that need them.. So perhaps introducing a flag in of_match_data for qcom,sm8150-qce (which was created solely to take care of the no-HLOS-clocks cases) and then skipping the clock operations based on that would be a good idea. Konrad > drivers/crypto/qce/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 0654b94cfb95..5bb2128c95ca 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -209,15 +209,15 @@ static int qce_crypto_probe(struct platform_device *pdev) > if (ret < 0) > return ret; > > - qce->core = devm_clk_get(qce->dev, "core"); > + qce->core = devm_clk_get_optional(qce->dev, "core"); > if (IS_ERR(qce->core)) > return PTR_ERR(qce->core); > > - qce->iface = devm_clk_get(qce->dev, "iface"); > + qce->iface = devm_clk_get_optional(qce->dev, "iface"); > if (IS_ERR(qce->iface)) > return PTR_ERR(qce->iface); > > - qce->bus = devm_clk_get(qce->dev, "bus"); > + qce->bus = devm_clk_get_optional(qce->dev, "bus"); > if (IS_ERR(qce->bus)) > return PTR_ERR(qce->bus); >
Hi Konrad, On 2/22/23 19:33, Konrad Dybcio wrote: > > > On 22.02.2023 18:22, Vladimir Zapolskiy wrote: >> From: Thara Gopinath <thara.gopinath@gmail.com> >> >> On certain Snapdragon processors, the crypto engine clocks are enabled by >> default by security firmware and the driver should not handle the clocks. >> Make acquiring of all the clocks optional in crypto engine driver, so that >> the driver initializes properly even if no clocks are specified in the dt. >> >> Tested-by: Jordan Crouse <jorcrous@amazon.com> >> Signed-off-by: Thara Gopinath <thara.gopinath@gmail.com> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >> [Bhupesh: Massage the commit log] >> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >> --- > I'm not sure which is the preferred approach, but generally I'd > stick with keeping them non-optional for the SoCs that need them.. > So perhaps introducing a flag in of_match_data for qcom,sm8150-qce > (which was created solely to take care of the no-HLOS-clocks cases) > and then skipping the clock operations based on that would be a > good idea. thank you for review. As you can get it from 06/10 the task to distinguish IPs with clocks and without clocks is offloaded to dtb. I believe a better support of two cases should be added to the driver on the basis of QCE IP versions obtained in runtime, or, alternatively and like you propose, it can be taken from a compatible. IMHO the latter one is a weak improvement, since it can be considered as a workaround in the driver to a known to be broken device tree node. -- Best wishes, Vladimir
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c index 0654b94cfb95..5bb2128c95ca 100644 --- a/drivers/crypto/qce/core.c +++ b/drivers/crypto/qce/core.c @@ -209,15 +209,15 @@ static int qce_crypto_probe(struct platform_device *pdev) if (ret < 0) return ret; - qce->core = devm_clk_get(qce->dev, "core"); + qce->core = devm_clk_get_optional(qce->dev, "core"); if (IS_ERR(qce->core)) return PTR_ERR(qce->core); - qce->iface = devm_clk_get(qce->dev, "iface"); + qce->iface = devm_clk_get_optional(qce->dev, "iface"); if (IS_ERR(qce->iface)) return PTR_ERR(qce->iface); - qce->bus = devm_clk_get(qce->dev, "bus"); + qce->bus = devm_clk_get_optional(qce->dev, "bus"); if (IS_ERR(qce->bus)) return PTR_ERR(qce->bus);