lkml.org 
[lkml]   [2005]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    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 09:07    [W:3.594 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site