| Message ID | 1427385887-25403-1-git-send-email-clg@fr.ibm.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, 2015-26-03 at 16:04:45 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote: > OPAL has its own list of return codes. The patch provides a translation > of such codes in errnos for the opal_sensor_read call. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > arch/powerpc/platforms/powernv/opal-sensor.c | 37 ++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > Index: linux.git/arch/powerpc/platforms/powernv/opal-sensor.c > =================================================================== > --- linux.git.orig/arch/powerpc/platforms/powernv/opal-sensor.c > +++ linux.git/arch/powerpc/platforms/powernv/opal-sensor.c > @@ -26,6 +26,38 @@ > +static int convert_opal_code(int ret) > +{ > + switch (ret) { > + case OPAL_SUCCESS: return 0; > + case OPAL_PARAMETER: return -EINVAL; > + case OPAL_UNSUPPORTED: return -ENOSYS; > + case OPAL_ASYNC_COMPLETION: return -EAGAIN; > + case OPAL_BUSY_EVENT: return -EBUSY; > + case OPAL_NO_MEM: return -ENOMEM; > + case OPAL_HARDWARE: return -ENOENT; > + case OPAL_INTERNAL_ERROR: return -EIO; > + default: return -EIO; > + } > +} That looks a bit familiar :) static int rtas_error_rc(int rtas_rc) { int rc; switch (rtas_rc) { case -1: /* Hardware Error */ rc = -EIO; break; case -3: /* Bad indicator/domain/etc */ rc = -EINVAL; break; case -9000: /* Isolation error */ rc = -EFAULT; break; case -9001: /* Outstanding TCE/PTE */ rc = -EEXIST; break; case -9002: /* No usable slot */ rc = -ENODEV; break; default: printk(KERN_ERR "%s: unexpected RTAS error %d\n", __func__, rtas_rc); rc = -ERANGE; break; } return rc; } But I guess we still should have it. Can you put it in opal.h and give it a better name, maybe opal_error_code() ? > /* > * This will return sensor information to driver based on the requested sensor > * handle. A handle is an opaque id for the powernv, read by the driver from the > @@ -46,8 +78,10 @@ int opal_get_sensor_data(u32 sensor_hndl > > mutex_lock(&opal_sensor_mutex); > ret = opal_sensor_read(sensor_hndl, token, &data); > - if (ret != OPAL_ASYNC_COMPLETION) > + if (ret != OPAL_ASYNC_COMPLETION) { > + ret = convert_opal_code(ret); > goto out_token; > + } > > ret = opal_async_wait_response(token, &msg); > if (ret) { > @@ -58,6 +92,7 @@ int opal_get_sensor_data(u32 sensor_hndl > > *sensor_data = be32_to_cpu(data); > ret = be64_to_cpu(msg.params[1]); > + ret = convert_opal_code(ret); I'd do: ret = convert_opal_code(be64_to_cpu(msg.params[1])); cheers
On Fri, 2015-03-27 at 20:59 +1100, Michael Ellerman wrote: > > Can you put it in opal.h and give it a better name, maybe > opal_error_code() ? Do we want it to be inlined all the time ? Feels more like something we should have in opal.c Also we only want to call it when we "forward" the error code up the food chain, there are a number of cases where we look for specific OPAL error codes. Cheers, Ben.
On 03/27/2015 11:36 AM, Benjamin Herrenschmidt wrote: > On Fri, 2015-03-27 at 20:59 +1100, Michael Ellerman wrote: >> >> Can you put it in opal.h and give it a better name, maybe >> opal_error_code() ? > > Do we want it to be inlined all the time ? Feels more like something we > should have in opal.c > > Also we only want to call it when we "forward" the error code up the > food chain, there are a number of cases where we look for specific OPAL > error codes. yes. the forward is not systematic. opal.c looks like a better place. -ERANGE looks also better when the return code is unexpected. C.
On 03/27/2015 10:59 AM, Michael Ellerman wrote: > On Thu, 2015-26-03 at 16:04:45 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote: >> OPAL has its own list of return codes. The patch provides a translation >> of such codes in errnos for the opal_sensor_read call. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >> --- >> arch/powerpc/platforms/powernv/opal-sensor.c | 37 ++++++++++++++++++++++++++- >> 1 file changed, 36 insertions(+), 1 deletion(-) >> >> Index: linux.git/arch/powerpc/platforms/powernv/opal-sensor.c >> =================================================================== >> --- linux.git.orig/arch/powerpc/platforms/powernv/opal-sensor.c >> +++ linux.git/arch/powerpc/platforms/powernv/opal-sensor.c >> @@ -26,6 +26,38 @@ > >> +static int convert_opal_code(int ret) >> +{ >> + switch (ret) { >> + case OPAL_SUCCESS: return 0; >> + case OPAL_PARAMETER: return -EINVAL; >> + case OPAL_UNSUPPORTED: return -ENOSYS; >> + case OPAL_ASYNC_COMPLETION: return -EAGAIN; >> + case OPAL_BUSY_EVENT: return -EBUSY; >> + case OPAL_NO_MEM: return -ENOMEM; >> + case OPAL_HARDWARE: return -ENOENT; >> + case OPAL_INTERNAL_ERROR: return -EIO; >> + default: return -EIO; >> + } >> +} > > That looks a bit familiar :) Ah ! I only looked in opal ... > static int rtas_error_rc(int rtas_rc) > { > int rc; > > switch (rtas_rc) { > case -1: /* Hardware Error */ > rc = -EIO; > break; > case -3: /* Bad indicator/domain/etc */ > rc = -EINVAL; > break; > case -9000: /* Isolation error */ > rc = -EFAULT; > break; > case -9001: /* Outstanding TCE/PTE */ > rc = -EEXIST; > break; > case -9002: /* No usable slot */ > rc = -ENODEV; > break; > default: > printk(KERN_ERR "%s: unexpected RTAS error %d\n", > __func__, rtas_rc); > rc = -ERANGE; this a better code default value. > break; > } > return rc; > } > > > But I guess we still should have it. > > Can you put it in opal.h and give it a better name, maybe opal_error_code() ? Sure. I will change the name but opal.c looks better, knowing that opal.h is shared with skiboot. > >> /* >> * This will return sensor information to driver based on the requested sensor >> * handle. A handle is an opaque id for the powernv, read by the driver from the >> @@ -46,8 +78,10 @@ int opal_get_sensor_data(u32 sensor_hndl >> >> mutex_lock(&opal_sensor_mutex); >> ret = opal_sensor_read(sensor_hndl, token, &data); >> - if (ret != OPAL_ASYNC_COMPLETION) >> + if (ret != OPAL_ASYNC_COMPLETION) { >> + ret = convert_opal_code(ret); >> goto out_token; >> + } >> >> ret = opal_async_wait_response(token, &msg); >> if (ret) { >> @@ -58,6 +92,7 @@ int opal_get_sensor_data(u32 sensor_hndl >> >> *sensor_data = be32_to_cpu(data); >> ret = be64_to_cpu(msg.params[1]); >> + ret = convert_opal_code(ret); > > I'd do: > ret = convert_opal_code(be64_to_cpu(msg.params[1])); Yes. the double 'ret =' is ugly. Thanks, C.
Index: linux.git/arch/powerpc/platforms/powernv/opal-sensor.c =================================================================== --- linux.git.orig/arch/powerpc/platforms/powernv/opal-sensor.c +++ linux.git/arch/powerpc/platforms/powernv/opal-sensor.c @@ -26,6 +26,38 @@ static DEFINE_MUTEX(opal_sensor_mutex); + +/* + * opal_sensor_read() return codes + * + * OPAL_PARAMETER - invalid sensor handler + * OPAL_UNSUPPORTED - plateform does not support reading sensors. + * + * in case of communication with the FSP on IBM systems + * + * OPAL_ASYNC_COMPLETION - a request was sent and an async completion will + * be triggered with the @token argument + * OPAL_PARTIAL - the request completed but the data returned is invalid + * OPAL_BUSY_EVENT - a previous request is still pending + * OPAL_NO_MEM - allocation failed + * OPAL_INTERNAL_ERROR - communication failure with the FSP + * OPAL_HARDWARE - FSP is not available + */ +static int convert_opal_code(int ret) +{ + switch (ret) { + case OPAL_SUCCESS: return 0; + case OPAL_PARAMETER: return -EINVAL; + case OPAL_UNSUPPORTED: return -ENOSYS; + case OPAL_ASYNC_COMPLETION: return -EAGAIN; + case OPAL_BUSY_EVENT: return -EBUSY; + case OPAL_NO_MEM: return -ENOMEM; + case OPAL_HARDWARE: return -ENOENT; + case OPAL_INTERNAL_ERROR: return -EIO; + default: return -EIO; + } +} + /* * This will return sensor information to driver based on the requested sensor * handle. A handle is an opaque id for the powernv, read by the driver from the @@ -46,8 +78,10 @@ int opal_get_sensor_data(u32 sensor_hndl mutex_lock(&opal_sensor_mutex); ret = opal_sensor_read(sensor_hndl, token, &data); - if (ret != OPAL_ASYNC_COMPLETION) + if (ret != OPAL_ASYNC_COMPLETION) { + ret = convert_opal_code(ret); goto out_token; + } ret = opal_async_wait_response(token, &msg); if (ret) { @@ -58,6 +92,7 @@ int opal_get_sensor_data(u32 sensor_hndl *sensor_data = be32_to_cpu(data); ret = be64_to_cpu(msg.params[1]); + ret = convert_opal_code(ret); out_token: mutex_unlock(&opal_sensor_mutex);
OPAL has its own list of return codes. The patch provides a translation of such codes in errnos for the opal_sensor_read call. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- arch/powerpc/platforms/powernv/opal-sensor.c | 37 ++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)