Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 06 Nov 2005 22:35:03 -0700 (MST)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        rink@stack.nl
Cc:        ed@fxq.nl, freebsd-arch@FreeBSD.ORG
Subject:   Re: FreeBSD/xbox: updated 7.0 patchset
Message-ID:  <20051106.223503.97843133.imp@bsdimp.com>
In-Reply-To: <20051106222359.GC46752@stack.nl>
References:  <20051106222359.GC46752@stack.nl>

next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20051106222359.GC46752@stack.nl>
            Rink Springer <rink@stack.nl> writes:
: You can download the patches from
: http://rink.nu/downloads/xbox-patches/xbox-7-current.diff. I hope this
: patch will be committed to the FreeBSD source tree. Let me know any
: suggestions for improvements.

OK.  I mostly like this patch.  It is fairly clean and I'll be working
on integrating it into the FreeBSD repo, with your help.

I have just a few questions/comments on your patch.

First, options XBOX shouldn't be in opt_global.h, but rather in
opt_xbox.h that dev/fb/boot_font.c, i386/i386/machdep.c,
i386/i386/pmap.c, i386/i386/vm_machdep, i386/isa/clock.c, and
i386/pci/pci_cfgreg.c should be including.  In general, we try to
reserve opt_global.h for really global stuff.

I don't understand the change to dev/fb/boot_font.c at all.  Can you
explain it in more detail?

There's some style(9) issues with xboxfb.c.  Specifically, there are a
few cases in some switches that are inconsistantly formatted wrt other
case statements.

There are a few really gross hacks that are marked with XXX better
fixes welcome.  I'm a little torn about committing those, but at least
they are all #ifdef XBOX.  My question is how long do you think until
there's a better solution for them?

This is absolutely great work!  Don't let my few quibbles and such get
you down.  I very much want to see xbox support committed to the tree.

Warner





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051106.223503.97843133.imp>