| Message ID | e9c245df4a0b1cd1f68171c81e0d9e64a13ab0e9.1587704308.git.sbobroff@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/eeh: Release EEH device state synchronously | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (47e80b4d8b45ae1bd3a1fe8577e95571cb8a976e) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 22 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Sam Bobroff <sbobroff@linux.ibm.com> writes: > Export rtas_error_rc() so that it can be used by other users of > rtas_call() (which is already exported). This will do the right thing for your ibm,configure-pe use case in patch 2, but the -900x => errno translations in rtas_error_rc() appear tailored for the indicator- and sensor-related calls that currently use it. From my reading of PAPR+, the meaning of a -900x RTAS status word depends on the call. For example, -9002 commonly means "not authorized", which we would typically translate to -EPERM, but rtas_error_rc() would translate it to -ENODEV. Also the semantics of -9001 as a return value seem to vary a bit. So I don't think rtas_error_rc() should be advertised as a generically useful facility in its current form. (I have had some thoughts about how firmware/hypervisor call status can be translated to meaningful Linux error values without tedious switch statements, which I'm happy to expand on if anyone is interested, but I don't want to hijack your submission for that discussion.)
On Fri, Apr 24, 2020 at 11:07:43AM -0500, Nathan Lynch wrote: > Sam Bobroff <sbobroff@linux.ibm.com> writes: > > Export rtas_error_rc() so that it can be used by other users of > > rtas_call() (which is already exported). > > This will do the right thing for your ibm,configure-pe use case in patch > 2, but the -900x => errno translations in rtas_error_rc() appear > tailored for the indicator- and sensor-related calls that currently use > it. From my reading of PAPR+, the meaning of a -900x RTAS status word > depends on the call. For example, -9002 commonly means "not authorized", > which we would typically translate to -EPERM, but rtas_error_rc() would > translate it to -ENODEV. > > Also the semantics of -9001 as a return value seem to vary a bit. > > So I don't think rtas_error_rc() should be advertised as a generically > useful facility in its current form. > > (I have had some thoughts about how firmware/hypervisor call status can > be translated to meaningful Linux error values without tedious switch > statements, which I'm happy to expand on if anyone is interested, but I > don't want to hijack your submission for that discussion.) Ah, interesting. I'll do another version as you suggest. Cheers, Sam.
Sam Bobroff <sbobroff@linux.ibm.com> writes: > Export rtas_error_rc() so that it can be used by other users of > rtas_call() (which is already exported). > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> > --- > v3 * New in this version. > > arch/powerpc/include/asm/rtas.h | 1 + > arch/powerpc/kernel/rtas.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h > index 3c1887351c71..7c9e4d3635cf 100644 > --- a/arch/powerpc/include/asm/rtas.h > +++ b/arch/powerpc/include/asm/rtas.h > @@ -379,6 +379,7 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time); > > extern unsigned int rtas_busy_delay_time(int status); > extern unsigned int rtas_busy_delay(int status); > +extern int rtas_error_rc(int rtas_rc); > > extern int early_init_dt_scan_rtas(unsigned long node, > const char *uname, int depth, void *data); > diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c > index c5fa251b8950..238bf112d29a 100644 > --- a/arch/powerpc/kernel/rtas.c > +++ b/arch/powerpc/kernel/rtas.c > @@ -518,7 +518,7 @@ unsigned int rtas_busy_delay(int status) > } > EXPORT_SYMBOL(rtas_busy_delay); > > -static int rtas_error_rc(int rtas_rc) > +int rtas_error_rc(int rtas_rc) > { > int rc; > > @@ -546,6 +546,7 @@ static int rtas_error_rc(int rtas_rc) > } > return rc; > } > +EXPORT_SYMBOL(rtas_error_rc); Will it be used in a module somewhere? AFAICS the only caller you add is built-in. cheers
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h index 3c1887351c71..7c9e4d3635cf 100644 --- a/arch/powerpc/include/asm/rtas.h +++ b/arch/powerpc/include/asm/rtas.h @@ -379,6 +379,7 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time); extern unsigned int rtas_busy_delay_time(int status); extern unsigned int rtas_busy_delay(int status); +extern int rtas_error_rc(int rtas_rc); extern int early_init_dt_scan_rtas(unsigned long node, const char *uname, int depth, void *data); diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index c5fa251b8950..238bf112d29a 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -518,7 +518,7 @@ unsigned int rtas_busy_delay(int status) } EXPORT_SYMBOL(rtas_busy_delay); -static int rtas_error_rc(int rtas_rc) +int rtas_error_rc(int rtas_rc) { int rc; @@ -546,6 +546,7 @@ static int rtas_error_rc(int rtas_rc) } return rc; } +EXPORT_SYMBOL(rtas_error_rc); int rtas_get_power_level(int powerdomain, int *level) {
Export rtas_error_rc() so that it can be used by other users of rtas_call() (which is already exported). Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> --- v3 * New in this version. arch/powerpc/include/asm/rtas.h | 1 + arch/powerpc/kernel/rtas.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)