On Mon, Jan 31, 2005 at 03:11:01PM -0600, Bruce Losure wrote:
>
> This patch provide support for the CX port of the SGI TIO chip.
> The CX port of the SGI TIO chip may be connected to FPGA based
> hardware. The changes in this patch may be used in support of device
> driver developers who are writing drivers for that FPGA hardware.
> The module provides driver registration/unregistration routines, device
> registration/unregistration routines, interrupt allocation/deallocation
> and an ioctl.
Comment one: please use the driver model to get rid of all your internal
bookkeeping.
Comment two: without an actual user this won't go into the tree.
> +typedef struct cx_id_s {
> + unsigned int part_num;
> + unsigned int mfg_num;
> + int nasid;
> +} cx_id_t;
please avoid typedefs (also in lots of other places)
> +#define TIOCX_IOCTL_BASE 0xdd // see Documentation/ioctl-number.txt
> +#define TIOCX_IOCTL_CX_RELOAD _IO(TIOCX_IOCTL_BASE, 1)
> +#define TIOCX_IOCTL_DEV_LIST _IO(TIOCX_IOCTL_BASE, 2)
also avoid new ioctl-based interfaces.
> +#include <asm-ia64/sn/types.h>
always use <asm/foo.h>
> +obj-m += tiocx.o
this is broken. Either it's unconditionally built in or should
be a config option - unconditionally modular is not an option.
> DEFINE_PER_CPU(struct pda_s, pda_percpu);
> +EXPORT_PER_CPU_SYMBOL(pda_percpu);
don't think it's something that should be exported. explain what you
need and provide accessors.
> +#include <asm/sn/types.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/version.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/proc_fs.h>
> +#include <asm/sn/sn_sal.h>
> +#include <asm/sn/addrs.h>
> +#include <asm/sn/io.h>
> +#include "shubio.h"
> +#include "tio.h"
> +#include "tiocx.h"
> +#include "xtalk/xwidgetdev.h"
> +#include "xtalk/hubdev.h"
> +#include <asm-ia64/delay.h>
> +#include <asm/uaccess.h>
<linux/*.h> before <asm/*.h> before local headers please.
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
