On Wed, 2006-04-26 at 15:56 -0400, Maria Butrico wrote:
> Hollis Blanchard wrote:
> > 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)
> > }
> >
> >
>
>
> A specific question about this suggestion. Where/how do we put .h
> related to this stuff in the xen tree? My gut feeling would be to put
> it in arch/ppc for now, but this thing should generalize to other arch.
> Hollis, are you suggesting to keep this for ppc only, and add something
> else for all arch'es one that would, for example, have what has been
> called so far the nohv bit.
Well, this is about more than that one bit: it's about all
platform-specific stuff. For example, Maple won't call zilog_init(); it
will call ns16550_init(). Anything we need to do on Maple but not
everywhere will be hidden inside maple_init().
In general I think x86 systems tend to be pretty homogeneous, so they
probably don't need this infrastructure. It sounds like ia64 systems
vary widely, but they must have worked something out for themselves
already. I guess I would like to make this PPC-specific for now, and
offer to generalize it when/if other architectures need it.
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
Home