WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [Patch] Find serial device and platform type

comments:
On Apr 24, 2006, at 4:22 PM, Maria Butrico wrote:

This patch is intended as a preliminary request for comment, status update and a good place to keep it, in case I destroy my tree. It's work in progress.
Usually people prepend "[rfc]" to the subject.
Thanks for being the community tester on this :)
+struct global_hardware_stuff {
+    int platform_type;
+    #define UNKNOWN_PLATFORM        1
+    #define PMAC_HARDWARE           2
+    #define MAPLE_SIM               3
+    #define MAPLE_HARDWARE          4
+    #define JS2X_HARDWARE           5
Hollis made a good point about the array with strings and init function pointers. tho I did not want anyone to spend a lot of time on this code, I think what use here will last a looong time.

Also, I'm not a Huge fan of #define's in structures and indenting _before_ the '#' is not proper C and am surprised it works without at least a warning.
+    struct {
[...]
+    } serial_port;

this struct is valid outside of the structure you are defining here so please define it elsewhere. You might also want to include other members that are in struct ns16550_defaults.

@@ -152,6 +182,7 @@ static int __init of_getprop(int ph, con
      if (rets[0] == OF_FAILURE) {
         DBG("getprop 0x%x %s -> FAILURE\n", ph, name);
+        if (buflen > 0) memset(buf, 0, 1);
         return OF_FAILURE;
     }

hmm, I think we need to be able to depend on the interface not messing with anything on failure. I'm not sure this would protect you from anything, particularly if the property is expected to be an non-string.
The callers need to check for failure.
+static void search_by_property_value(int p, const char *prop_name,
+                const char *prop_value)
Not sure what you plan on doing with this function...

+/*
+ * Returns the value of the property '/#size-cells'
+ */
+int __init boot_of_get_sizecells_prop()
Sadly, it does not work which way.
The size cell of a property is either defined by your package or an ancestral package, not necessarily in the root. it gets even trickier for the "regs" property where an ancestor's definition over-rides the package's

+{
+    int of_root;
+    u32 cell = 1;
+    +    of_root = of_finddevice("/");
+    if (OF_FAILURE != of_root)
if this fails, you panic()

+int __init boot_of_serial()

+    char ofout_path[256] = {0,};
this is a call to memset please just assign ofout_path[0] = '\0' later.

+    const char serial[] = "serial";
+    const char macio[] = "mac-io";
+    const char escc[] = "escc";
These are copies, since you need to know how big they are (with the possible exception of serial) they should be pointers rather than arrays. The discovery code seems a little dubious but I know its heritage and will let it slide.

diff -r 56e1802303e5 xen/include/xen/sched.h
--- a/xen/include/xen/sched.h   Wed Apr 12 15:41:55 2006 -0500
+++ b/xen/include/xen/sched.h   Fri Apr 21 10:41:42 2006 -0400
@@ -392,6 +392,10 @@ extern struct domain *domain_list;
  /* Domain is being debugged by controller software. */
 #define _DOMF_debugging        4
 #define DOMF_debugging         (1UL<<_DOMF_debugging)
+ /* run domain in prob mode */
+#define _DOMF_prob      7
+#define DOMF_prob       (1UL<<_DOMF_prob)
+

There needs to be a new member in "struct arch_domain" and a new bit representing this state should be there.
-JX

_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel