Thanks for the review comments. I'm sending my response "on-list",
primarily so that they are captured. (Hmm.... I wonder if there should
be a [EMAIL PROTECTED] list just for this purpose ... I
wouldn't expect many people to participate, but it would certainly be
nice to record this kind of feedback in the archives.)
Richard Lowe wrote:
Richard Lowe wrote:
Hey Garrett,
RL-1:
'support' not 'suppor' in delta comments (if comchk is forcing it,
fix the
bug, not the comments)
I think that was my mistake... I'll fix it with wx reedit.
usr/src/pkgdefs/SUNWcakr.u/prototype_com
RL-2:
L394 - commented entries should be uncommented or removed.
Hrmm.... maybe.... as you can see, I intend the comment only to exist
for a short while.
usr/src/uts/sun4u/douglas/Makefile.douglas.shared
RL-3:
L74 - commented DOUGLAS_KMODS entry for tadpmu, kill until you
have it
(or add it in this wad, preferably, I think)
Part of the reason I left it in is to make it easy for me to figure out
where I needed to add it. I was hoping to minimize the amount of code
in this putback. The tadpmu is likely to be quite a bit larger, and a
bit more complex, and I don't want to dilute its changes with these.
(And tadpmu isn't strictly required for the platform to boot... indeed
I've been using a SPARCLE without any of this so far.)
usr/src/uts/sun4u/douglas/Makefile.files
RL-4:
L38 - See, now this *does* reference tadpmu, One or the other...
Hrmmm... right now the reference is a harmless stub, because nothing
calls it yet. But once I add tadpmu.c, it will get called. I can
remove it now if it is offensive.
usr/src/uts/sun4u/douglas/Makefile.rules
RL-5:
We use pretty much an identical file like this, once per-plat?
Someone should be shot for that. Unless you can figure out a good
reason, file a bug to have us stop? :)
Fixing this requires considerable Makefile-foo, and at this point, I
don't really want to have to deal with that. (The main thing is that
you have to add another variable into the path, $(PLATFORM) or somesuch,
into dependencies. Sun Make doesn't like having variable dependencies.)
usr/src/uts/sun4u/douglas/Makefile.targ.shared
RL-6:
Some platforms do this and Makefile.targ, some just the latter.
Is there a reason for that, I can't see one. But just in case, be
sure
you're doing the right one, since this is the least common.
I copied the current Makefile framework from grover. I noticed the
discrepancy, but I have no idea which is preferred. At some level, I
don' t really care.
usr/src/uts/sun4u/douglas/os/douglas.c
RL-7:
#if 0 is evil, don't do it. (see Developing in ON)
Again, this is intended to be very short term.
usr/src/uts/sun4u/douglas/Makefile
RL-8:
If you don't actually have closed kmods, don't do the
CLOSED_DOUGLAS* bits.
Ah, good point. At some point we may have closed modules, but I guess I
can remove the rules for now. (The reason that we would have closed
modules is that some of the SDcard and memstick code were written under
some fairly stringent NDA from the trade associations.)
RL-9:
L93 still calls this grover
I'd suggest wx grep '[Gg]rover', there maybe other places I missed.
Oops! I'll fix that. I think this is the only occurrence, but I'll
double check. :-)
Generally, I'd suggest delaying this and integrating tadpmu with it.
Not only does this wad appear incomplete without tadpmu, but many of
the comments above related to you trying to do this in two-stages.
usr/src/uts/sun4u/douglas/os/douglas.c:
RL-10:
Enough of this is common with grover, it's tempting to suggest
finding a way to make the common bits truly common.
The whole module is actually so small, that trying to make it common
would probably not be too worthwhile... it would mostly involve
introducing #ifdef's, which I'd rather not do.
RL-11:
It also stands out that you're not using the grbeep bits, is that a
platform difference or oversight?
Its a platform difference.
-- Garrett
-- Rich
_______________________________________________
opensolaris-code mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
