Messages in this thread | | | Date | Sun, 31 Dec 2006 15:24:15 +0200 | From | "Pekka Enberg" <> | Subject | Re: [PATCH] Open Firmware device tree virtual filesystem |
| |
On 12/31/06, Mitch Bradley <wmb@firmworks.com> wrote: > diff --git a/arch/i386/kernel/ofw_fs.c b/arch/i386/kernel/ofw_fs.c > new file mode 100644 > index 0000000..30ca359 > --- /dev/null > +++ b/arch/i386/kernel/ofw_fs.c > @@ -0,0 +1,261 @@ > +/* 1275 in little-endian ASCII (for IEEE 1275 - the Open Firmware Standard) */ > +#define OFWFS_MAGIC 0x35373231
Please put this in <linux/magic.h>
On 12/31/06, Mitch Bradley <wmb@firmworks.com> wrote: > + case S_IFREG: > + inode->i_fop = &ofwfs_property_operations; > + inode->i_bytes = ((struct propval *)data)->length; > + inode->i_size = (loff_t)(((struct propval *)data)->length);
As you need the struct twice, use a local variable.
On 12/31/06, Mitch Bradley <wmb@firmworks.com> wrote: > +static int ofwfs_create_props(struct super_block *sb, struct dentry *dir, > + phandle node) > +{ > + char propname[32]; > + int security, len; > + struct propval *propval; > + int ret = 0; > + int flag; > + > + propname[0] = '\0'; > + > + while ((void)callofw("nextprop", 3, 1, node, propname, propname, > + &flag), flag == 1) {
Please do the call within the loop body and use an explicit bereak.
> + security = strncmp(propname, "security-", 9) == 0; > + len = 0;
Redundant assignment, no?
> + if (!security) > + (void)callofw("getproplen", 2, 1, node, propname, &len);
We don't use void casts to suppress return value in Linux. You can just drop it (elsewhere too).
On 12/31/06, Mitch Bradley <wmb@firmworks.com> wrote: > +out: > + if (ret) { > + WARN_ON(1); > +/* > + ofwfs_remove_props(sb, dir); > +*/ > + }
Hmm?
On 12/31/06, Mitch Bradley <wmb@firmworks.com> wrote: > +static int __init ofwfs_init(void) > +{ > + int ret; > + > + ret = register_filesystem(&ofwfs_type); > + if (ret) > + return ret; > + > + kern_mount(&ofwfs_type);
kern_mount can fail so you'd better do IS_ERR/PTR_ERR here.
> diff --git a/include/asm-i386/callofw.h b/include/asm-i386/callofw.h > new file mode 100644 > index 0000000..594cb63 > --- /dev/null > +++ b/include/asm-i386/callofw.h > +#include <linux/types.h> > + > +typedef void *phandle;
The typedef seems useless and confusing at best. Can we drop it? - 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/
| |