Re: cobalt kernel patch for 2.6.x
On Fri, Jul 02, 2004 at 12:17:36AM +1000, Russell Coker wrote:
> http://www.no-l.org/pages/cobalt-patch.html
>
> At the above URL there is a patch against the Debian 2.6.6 kernel to support
> Cobalt hardware. I have also uploaded a kernel patch package to Debian with
> it. It seems to work OK, and it would be good if we could get it included
> upstream...
- the new files seems to lack copyright statements often, can you get
your upstream to clarify that's their under a free enough license?
- the arch/i386/boot/Makefile is bogus, it overwrites the bzImage with a
gzipped vmlinux. What about adding a vmlinux.gz rule for all
architectures instead? Even grub understands gzipped kernel images
these days..
- for the other arch/i386/kernel/ changes I think they'd better be done
as a sub-architecture ala visw, voyager & co
- please don't add new explicit initialization function calls to
drivers/char/misc.c but use initcalls. (Dito for init/main.c and
drivers/cobalt/init.c)
- Kconfig uses far too much indentation, check other Kconfig files for
the usual style
- drivers/cobalt/acpi.c needs to be run through Lindent and probaly
move under arch/i386/
- in drivers/cobalt/fans.c please kill the ifdef around all the source
code - if CONFIG_COBALT_FANS isn't set it's not built by the Makefile
anyway. Dito in most other files.
- please never use <stddef.h> but always <linux/stddef.h>
- lots of files include <stdarg.h> but I can't actually find varargs
functions in the new code
- drivers/cobalt/i2c.c is a bad idea, please integrate the cobal code
with the generic i2c layer
- please convert all procfs files to the seq_file interface or sysfs,
that'll also get rid of cobalt_gen_proc_read
- in lcd.c please use C99 initializers for struct miscdevice and
struct file_operations. The fans.c ifdef comment applies here aswell.
- any chance to merge drivers/cobalt/led.c with drivers/char/lcd.c for
the mips-based cobald systems?
- drivers/cobalt/systype.c should probably move under arch/i386, too
- drivers/cobalt/wdt.c should move to drivers/char/watchdog/ and get
all the changes the other watchdog drivers got over the last years
(and be submitted via the watchdog maintainer)
- include/cobalt/ should probably go away - most thing should stay
under drivers/cobalt/ and everything that's needed outside that dir
could go into a single <linux/cobalt.h>
I'd also suggest you submit a patch for minimal support to add a cobalt
subarch and get it booting first and then add the optional drivers later
piecemail.
Reply to: