fix up log_plpar_hcall_return

This is mostly cosmetic. This updates log_plpar_hcall_return() to use a case statement rather than an if-then-else jumble, and moves it to rtas.c where it can be near the other rtas related functions.

---
 arch/powerpc/kernel/lparcfg.c |   25 -------------------------
 arch/powerpc/kernel/rtas.c    |   29 +++++++++++++++++++++++++++++
 include/asm-powerpc/rtas.h    |    1 +
 3 files changed, 30 insertions(+), 25 deletions(-)

index 89486b6..e89471f 100644
@@ -130,31 +130,6 @@ #ifdef CONFIG_PPC_PSERIES
 /*
  * Methods used to fetch LPAR data when running on a pSeries platform.
  */
-/* find a better place for this function... */
-static void log_plpar_hcall_return(unsigned long rc, char *tag)
-{
-	if (rc == 0)		/* success, return */
-		return;
-/* check for null tag ? */
-	if (rc == H_HARDWARE)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed with hardware fault\n", tag);
-	else if (rc == H_FUNCTION)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed; function not allowed\n", tag);
-	else if (rc == H_AUTHORITY)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed; not authorized to this"
-		       " function\n", tag);
-	else if (rc == H_PARAMETER)
-		printk(KERN_INFO "plpar-hcall (%s) failed; Bad parameter(s)\n",
-		       tag);
-	else
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed with unexpected rc(0x%lx)\n",
-		       tag, rc);
-
-}
 
 /*
  * H_GET_PPP hcall returns info in 4 parms.

index 9d0735a..b215b6b 100644
@@ -506,6 +506,35 @@ int rtas_error_rc(int rtas_rc)
 	return rc;
 }
 
+/* provide a human-readible description of some hcall return codes. */
+void log_plpar_hcall_return(unsigned long rc, char *tag)
+{
+	switch(rc) {
+	case 0:
+		return;
+	case H_HARDWARE:
+		printk(KERN_INFO "plpar-hcall (%s) "
+					"Hardware fault\n", tag);
+		return;
+	case H_FUNCTION:
+		printk(KERN_INFO "plpar-hcall (%s) "
+					"Function not allowed\n", tag);
+		return;
+	case H_AUTHORITY:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Not authorized to this function\n", tag);
+		return;
+	case H_PARAMETER:
+		printk(KERN_INFO "plpar-hcall (%s) "
+					"Bad parameter(s)\n",tag);
+		return;
+	default:
+		printk(KERN_INFO "plpar-hcall (%s) "
+					"Unexpected rc(0x%lx)\n", tag, rc);
+	}
+}
+EXPORT_SYMBOL(log_plpar_hcall_return);
+
 int rtas_get_power_level(int powerdomain, int *level)
 {
 	int token = rtas_token("get-power-level");

index 8eaa7b2..f36aaba 100644
@@ -185,6 +185,7 @@ extern int early_init_dt_scan_rtas(unsig
 		const char *uname, int depth, void *data);
 
 extern void pSeries_log_error(char *buf, unsigned int err_type, int fatal);
+extern void log_plpar_hcall_return(unsigned long rc, char *tag);
 
 /* Error types logged.  */
 #define ERR_FLAG_ALREADY_LOGGED	0x0

[sfr done Blue Hair]

Hi Will,

You are moving a static function from the only file that calls it. You are exporting it when the only callers are in a file that cannot be part of a module and you are (maybe) optimizing something that is only used in error paths.

Sorry, but unless you are doing to introduce others uses, that I don't see that this change helps.

[sfr redons normal hair]

I don't mean to sound harsh :-)

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

Hi Stephen,

/me looks for blue spraypaint..

Yes. This moves the function to somewhere a bit more accessible so
others *can* call it. The comment at the top of the function being
replaced even says something like "find a better place for me...".

Lparcfg *did* begin it's life as a module, and I'm sure I've turned it
back into a module at least once. I dont know when it was last
reverted, but suspect it was done by someone who didnt know about EXPORT_PER_CPU_SYMBOL_GPL, but I digress..
I've got a patch to turn lparcfg back into a module, but wanted to start with this simpler cleanup patch first.

All that, and I've wanted to get rid of that particular if-else mess for a while. You are right that it's an error path optimization, though this change isn't meant as much for optimization as fixing up the horrid coding style that was initially used.

How about Purple? :-)

Not a problem. depending on what other comments I get back, I'll
reorganize a bit and submit a larger patchset with other changes too.

-Will

Except that this has nothing to do with RTAS or do I miss something ?

Ben.

Hi Ben, hello to other folks too,

Milton made a similar comment via IRC.

I am likely blending the rtas/hcall terminology in a bad way, but will clarify what I was thinking so someone can correct me. :-)

The lparcfg function is named log_plpar_hcall_return(), taking a hcall return value and a string, and outputs a human-readable error message that corresponds to that return value.

There is no hcall.c that this function would belong in. HvCall.S and plpar_wrappers.h contain wrappers and the assembly code that does the actual calls; but neither of those places look like good candidates for this function.
The file rtas.c, however, does make some plpar_hcall function calls, and has a number of the 900# magic numbers that *look* like the H_* defines to me.

-Will