| Message ID | 20211110105922.217895-18-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Enable Qualcomm Crypto Engine on sm8150 & sm8250 | expand |
Hi Bhupesh, On 11/10/21 12:59 PM, Bhupesh Sharma wrote: > Print a failure message (dev_err) in case the qcom qce crypto > driver probe() fails. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/crypto/qce/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 98784d63d78c..7c90401a2ef1 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > err_mem_path_disable: > icc_set_bw(qce->mem_path, 0, 0); > err: > + dev_err(dev, "%s failed : %d\n", __func__, ret); > return ret; > } > > in my opinion expressed earlier this change is not needed, but I'll recede, if somebody thinks that the change is useful in any way. -- Best wishes, Vladimir
Hi Vladimir, On Fri, 12 Nov 2021 at 16:12, Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> wrote: > > Hi Bhupesh, > > On 11/10/21 12:59 PM, Bhupesh Sharma wrote: > > Print a failure message (dev_err) in case the qcom qce crypto > > driver probe() fails. > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > drivers/crypto/qce/core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > > index 98784d63d78c..7c90401a2ef1 100644 > > --- a/drivers/crypto/qce/core.c > > +++ b/drivers/crypto/qce/core.c > > @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > > err_mem_path_disable: > > icc_set_bw(qce->mem_path, 0, 0); > > err: > > + dev_err(dev, "%s failed : %d\n", __func__, ret); > > return ret; > > } > > > > > > in my opinion expressed earlier this change is not needed, but I'll recede, > if somebody thinks that the change is useful in any way. As I mentioned in the reply to the review comments to the previous series, the need for this failure message was actually felt to address failures seen with boot-on crypto tests via 'CRYPTO_MANAGER_EXTRA_TESTS'. So, I would suggest keeping this patch in, unless there are some major concerns with the change. Regards, Bhupesh
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote: > Print a failure message (dev_err) in case the qcom qce crypto > driver probe() fails. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/crypto/qce/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 98784d63d78c..7c90401a2ef1 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > err_mem_path_disable: > icc_set_bw(qce->mem_path, 0, 0); > err: > + dev_err(dev, "%s failed : %d\n", __func__, ret); There's two possible outcomes of this style of error logging: 1) You came through a code path with a specific error message, so you will have something that will say: qce: Some useful error text qce: qce_crypto_probe failed: -22 2) You came through a code path without a specific error message: qce: qce_crypto_probe failed: -22 In the first case the second line is just pure spam, in the second case the bare -22 is typically completely useless - given that there tend to be just a few commonly used errno values coming from multiple possible error sources. As such, no thanks. If you have an error case in qce_crypto_probe() that doesn't have a good, useful, error message, please fix that. Regards, Bjorn > return ret; > } > > -- > 2.31.1 >
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c index 98784d63d78c..7c90401a2ef1 100644 --- a/drivers/crypto/qce/core.c +++ b/drivers/crypto/qce/core.c @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev) err_mem_path_disable: icc_set_bw(qce->mem_path, 0, 0); err: + dev_err(dev, "%s failed : %d\n", __func__, ret); return ret; }