| Hi Maria, you didn't include any description with your patch, so I'm not
sure if you intend it for submission or as a request for comments.
Stylistically, there are some typos, some renaming needed, some
formatting (e.g. "if (foo) bar" all on one line), // comments, lack of
punctuation in printf messages, etc. Also, the boot_of_serial() function
is waaay too long, with structs and unions defined in the middle of
it...
Also, it seems you haven't addressed any of the comments I made about
your earlier patch.
More generally, I'm not clear on why we need constants like
PMAC_HARDWARE at all (yes, I know Jimi suggested them). Why not
something like this:
----- setup.c -----
struct platform {
        char *compat;
        int (*init)(void);
} platforms = {
        { .compat = "Power Macintosh", .init = pmac_init },
        { .compat = "Momentum,Maple", .init = maple_init },
boot_of_platform() {
        for platform in platforms {
                if (compatible(platform.compat)
                        && platform.init() == SUCCESS)
                break;
        }
}
------ pmac.c -----
pmac_init() {
        zilog_register();
        return SUCCESS;
}
----- zilog.c -----
struct zilog_port {
        whatever;
} port;
zilog_register() {
        serial_register_uart(0, zilog_driver, &port)
}
-- 
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
 |