lkml.org 
[lkml]   [2005]   [Jun]   [3]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateFri, 3 Jun 2005 00:11:33 -0700
FromGreg KH <>
SubjectRe: [PATCH][1/5] RapidIO support: core
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-06-03 07:07    [from the cache]
©2003-2008