On 03/30/2015 04:05 AM, Michael Ellerman wrote:

On Fri, 2015-03-27 at 17:39 +0100, Cédric Le Goater wrote:

quoted

OPAL has its own list of return codes. The patch provides a translation
of such codes in errnos for the opal_sensor_read call, and possibly 
others if needed.

Index: linux.git/arch/powerpc/platforms/powernv/opal.c
===================================================================

--- linux.git.orig/arch/powerpc/platforms/powernv/opal.c
+++ linux.git/arch/powerpc/platforms/powernv/opal.c

@@ -894,6 +894,23 @@ void opal_free_sg_list(struct opal_sg_li
 	}
 }
 
+int opal_error_code(int rc)
+{
+	switch (rc) {
+	case OPAL_SUCCESS:		return 0;

Obviously correct.

He. Initially, I didn't put a case for SUCCESS, but we have code doing :

	ret = be64_to_cpu(msg.params[1]);

quoted

+	case OPAL_PARAMETER:		return -EINVAL;

Yep.

quoted

+	case OPAL_UNSUPPORTED:		return -ENOSYS;

You shouldn't use ENOSYS here, that should only ever mean "no such syscall",
otherwise you get very confusing results like read() returning ENOSYS.

Indeed. How about ENODEV then ? 

quoted

+	case OPAL_ASYNC_COMPLETION:	return -EAGAIN;

EAGAIN means "try what you did again", I don't think that's what
ASYNC_COMPLETION means, is it? It looks like it means, "don't try again, but
you need to wait for the result to be ready".

I'm not sure it maps well to any of the Linux codes, maybe EINPROGRESS ?

Yes. This is better.

quoted

+	case OPAL_BUSY_EVENT:		return -EBUSY;

Yep.

quoted

+	case OPAL_NO_MEM:		return -ENOMEM;

Yep.

quoted

+	case OPAL_HARDWARE:		return -ENOENT;

This is another one which I think you shouldn't use as it can lead to confusing
results at user level. eg:

  $ cat /sysfs/some/file
  Error: No such file or directory

Huh?

Looking at the skiboot code this looks like EIO is a good match.

ok. 

quoted

+	case OPAL_INTERNAL_ERROR:	return -EIO;

Yeah as good as anything I guess.

quoted

+	default:
+		pr_err("%s: unexpected OPAL error %d\n", __func__, rc);
+		return -ERANGE;

I'm not sure about this one honestly, it means "Math result not representable".

I suspect the reason RTAS chose it was just that it's not EINVAL.

This should probably also just be EIO.

ok. I will change it. 

Thanks,

C.