[PATCH v2] tee: optee: Change printing during optee_probe

[PATCH v2] tee: optee: Change printing during optee_probe

[Ilias Apalodimas]

Right now the error messages when optee has a version mismatch or shared
memory is not configured are done with a debug().
That's not very convenient since you have to enable debugging to figure
out what's going on, although this is an actual error.

So let's switch the debug() -> dev_err() and report those explicitly.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 drivers/tee/optee/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index b898c32edc0b..73dbb22ba097 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -624,14 +624,14 @@ static int optee_probe(struct udevice *dev)
 	u32 sec_caps;
 
 	if (!is_optee_api(pdata->invoke_fn)) {
-		debug("%s: OP-TEE api uid mismatch\n", __func__);
+		dev_err(dev, "OP-TEE api uid mismatch\n");
 		return -ENOENT;
 	}
 
 	print_os_revision(dev, pdata->invoke_fn);
 
 	if (!api_revision_is_compatible(pdata->invoke_fn)) {
-		debug("%s: OP-TEE api revision mismatch\n", __func__);
+		dev_err(dev, "OP-TEE api revision mismatch\n");
 		return -ENOENT;
 	}
 
@@ -642,7 +642,7 @@ static int optee_probe(struct udevice *dev)
 	 */
 	if (!exchange_capabilities(pdata->invoke_fn, &sec_caps) ||
 	    !(sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)) {
-		debug("%s: OP-TEE capabilities mismatch\n", __func__);
+		dev_err(dev, "OP-TEE capabilities mismatch\n");
 		return -ENOENT;
 	}
 
-- 
2.30.1

[PATCH v2] tee: optee: Change printing during optee_probe

[Simon Glass]

Hi Ilias,

On Wed, 10 Mar 2021 at 06:35, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Right now the error messages when optee has a version mismatch or shared
> memory is not configured are done with a debug().
> That's not very convenient since you have to enable debugging to figure
> out what's going on, although this is an actual error.

The code that probes the device should report the error. If you put
errors in a device driver it bloats the code and also it makes it
impossible for the caller to suppress the  error, e.g. if it is OK for
the device to not probe.


>
> So let's switch the debug() -> dev_err() and report those explicitly.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
>  drivers/tee/optee/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

[PATCH v2] tee: optee: Change printing during optee_probe

[Ilias Apalodimas]

On Thu, Mar 11, 2021 at 09:45:46PM -0700, Simon Glass wrote:
> Hi Ilias,
> 
> On Wed, 10 Mar 2021 at 06:35, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Right now the error messages when optee has a version mismatch or shared
> > memory is not configured are done with a debug().
> > That's not very convenient since you have to enable debugging to figure
> > out what's going on, although this is an actual error.
> 
> The code that probes the device should report the error. 

I am notr sure I am following here. This gets called from the core dm code. Is
there a callback in that, so we can register a specific print per device?
If not now how do you expect the core to habdle each specific driver error
code and print a message that makes sense?

> If you put
> errors in a device driver it bloats the code and also it makes it
> impossible for the caller to suppress the  error, e.g. if it is OK for
> the device to not probe.

Well in the op-tee case it's an explicit node you need to add in the DTB. So i
am not really sure this applies here.

Thanks
/Ilias

> 
> 
> >
> > So let's switch the debug() -> dev_err() and report those explicitly.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > ---
> >  drivers/tee/optee/core.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)

[PATCH v2] tee: optee: Change printing during optee_probe

[Simon Glass]

Hi Ilias,

On Thu, 11 Mar 2021 at 22:07, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Thu, Mar 11, 2021 at 09:45:46PM -0700, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Wed, 10 Mar 2021 at 06:35, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Right now the error messages when optee has a version mismatch or shared
> > > memory is not configured are done with a debug().
> > > That's not very convenient since you have to enable debugging to figure
> > > out what's going on, although this is an actual error.
> >
> > The code that probes the device should report the error.
>
> I am notr sure I am following here. This gets called from the core dm code. Is
> there a callback in that, so we can register a specific print per device?
> If not now how do you expect the core to habdle each specific driver error
> code and print a message that makes sense?

Well, who probes the device? Somewhere there is a call to
uclass_first_device(...) or similar, and that should receive the
error-return value.

>
> > If you put
> > errors in a device driver it bloats the code and also it makes it
> > impossible for the caller to suppress the  error, e.g. if it is OK for
> > the device to not probe.
>
> Well in the op-tee case it's an explicit node you need to add in the DTB. So i
> am not really sure this applies here.

I'm not sure, but you might be thinking of the 'bind' step. For probe
it is OK for devices to fail to probe.

Regards,
Simon


> >
> >
> > >
> > > So let's switch the debug() -> dev_err() and report those explicitly.
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> > > ---
> > >  drivers/tee/optee/core.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)

[PATCH v2] tee: optee: Change printing during optee_probe

[Tom Rini]

On Wed, Mar 10, 2021 at 03:35:11PM +0200, Ilias Apalodimas wrote:

> Right now the error messages when optee has a version mismatch or shared
> memory is not configured are done with a debug().
> That's not very convenient since you have to enable debugging to figure
> out what's going on, although this is an actual error.
> 
> So let's switch the debug() -> dev_err() and report those explicitly.
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210413/640e04ed/attachment.sig>