From: Greg KH <greg@kroah•com>
To: Matt Porter <mporter@kernel•crashing.org>
Cc: akpm@osdl•org, torvalds@osdl•org, linux-kernel@vger•kernel.org,
linuxppc-embedded@ozlabs•org
Subject: Re: [PATCH][1/5] RapidIO support: core
Date: Fri, 3 Jun 2005 00:11:33 -0700 [thread overview]
Message-ID: <20050603071133.GB30292@kroah.com> (raw)
In-Reply-To: <20050602140359.B24818@cox.net>
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
next prev parent reply other threads:[~2005-06-03 7:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-06-02 21:03 [PATCH][1/5] RapidIO support: core Matt Porter
2005-06-02 21:12 ` [PATCH][2/5] RapidIO support: core includes Matt Porter
2005-06-02 21:19 ` [PATCH][3/5] RapidIO support: enumeration Matt Porter
2005-06-02 21:25 ` [PATCH][4/5] RapidIO support: ppc32 Matt Porter
2005-06-02 21:34 ` [PATCH][5/5] RapidIO support: net driver over messaging Matt Porter
2005-06-02 22:05 ` Stephen Hemminger
2005-06-03 22:43 ` Matt Porter
2005-06-03 7:11 ` Greg KH [this message]
2005-06-03 21:24 ` [PATCH][1/5] RapidIO support: core Matt Porter
2005-06-03 7:20 ` Greg KH
2005-06-03 21:35 ` Matt Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20050603071133.GB30292@kroah.com \
--to=greg@kroah$(echo .)com \
--cc=akpm@osdl$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-embedded@ozlabs$(echo .)org \
--cc=mporter@kernel$(echo .)crashing.org \
--cc=torvalds@osdl$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox