On Thu, Jun 02, 2005 at 02:03:59PM -0700, Matt Porter wrote:
> +RIO_LOP_READ(8, u8, 1)
> + RIO_LOP_READ(16, u16, 2)
> + RIO_LOP_READ(32, u32, 4)
> + RIO_LOP_WRITE(8, u8, 1)
> + RIO_LOP_WRITE(16, u16, 2)
> + RIO_LOP_WRITE(32, u32, 4)
> +
> + EXPORT_SYMBOL(__rio_local_read_config_8);
> +EXPORT_SYMBOL(__rio_local_read_config_16);
> +EXPORT_SYMBOL(__rio_local_read_config_32);
> +EXPORT_SYMBOL(__rio_local_write_config_8);
> +EXPORT_SYMBOL(__rio_local_write_config_16);
> +EXPORT_SYMBOL(__rio_local_write_config_32);
Odd indenting here.
And why the __rio* stuff for public functions? You should drop the "__"
part.
> +RIO_OP_READ(8, u8, 1)
> + RIO_OP_READ(16, u16, 2)
> + RIO_OP_READ(32, u32, 4)
> + RIO_OP_WRITE(8, u8, 1)
> + RIO_OP_WRITE(16, u16, 2)
> + RIO_OP_WRITE(32, u32, 4)
> +
> + EXPORT_SYMBOL(rio_mport_read_config_8);
> +EXPORT_SYMBOL(rio_mport_read_config_16);
> +EXPORT_SYMBOL(rio_mport_read_config_32);
> +EXPORT_SYMBOL(rio_mport_write_config_8);
> +EXPORT_SYMBOL(rio_mport_write_config_16);
> +EXPORT_SYMBOL(rio_mport_write_config_32);
Again the odd indenting.
> +EXPORT_SYMBOL(rio_mport_send_doorbell);
Just a question, should these be EXPORT_SYMBOL_GPL() as this is GPL
code? Just to be explicit that is.
> +static ssize_t
> +rio_read_config(struct kobject *kobj, char *buf, loff_t off, size_t count)
<snip>
You might want to compare this to the recent changes in the 2.6.12-rc
kernels in the pci sysfs config code. There were some 64 and endian
issues fixed up there that you might want to make sure are also done
properly here.
> +static struct bin_attribute rio_config_attr = {
> + .attr = {
> + .name = "config",
> + .mode = S_IRUGO | S_IWUSR,
> + .owner = THIS_MODULE,
> + },
> + .size = 0x200000,
> + .read = rio_read_config,
> + .write = rio_write_config,
> +};
Wow, that's a huge config space (just a comment, not an issue...)
thanks,
greg k-h
