Re: [RFC 4/4] firewire: add mem1394

[Andrew Morton <akpm@osdl.org>]

Johannes Berg <johannes@sipsolutions.net> wrote:
>
> +config IEEE1394_MEMDEV
> +	tristate "IEEE1394 memory device support"
> +	depends on IEEE1394 && EXPERIMENTAL
> +	help
> +	  Say Y here if you want support for the ieee1394 memory device.
> +	  This is useful for debugging systems attached via firewire
> +	  since it usually allows you to read from and write to their memory,
> +	  depending on the controller and machine setup.

1394 is evil.  Does this mean that if a machine is completely
dead-and-crashed, we can still suck all its memory out over 1394 with no
cooperation from the dead machine's kernel?  If not, what limitations are
there?



Triviata:

> +static int mem1394_read(struct file *file, char __user * buffer,
> +			size_t count, loff_t *offset)
> +{
> +	struct mem1394_file_info *fi = (struct mem1394_file_info*)file->private_data;

Unneeded cast.

> +	packet = hpsb_make_readpacket(fi->memdev->ne->host, fi->memdev->ne->nodeid, *offset, submitcount);

xterm too big!

> +static int mem1394_release(struct inode *inode, struct file *file)
> +{
> +	struct mem1394_file_info *fi = (struct mem1394_file_info*)file->private_data;

Unneeded cast.

> +	

Adds trailing whitespace ;)

> +static struct mem1394_dev * alloc_mem1394_dev(struct device *dev)
> +{
> +	struct mem1394_dev *result;
> +	struct node_entry *ne = container_of(dev, struct node_entry, device);
> +	int ret;
> +	struct class_device * mem1394_class_member;

Inconsistent space-after-asterisk policy (no-space is preferred).

> +	mem1394_class_member = class_device_create(mem1394_sysfs_class, NULL, result->cdev.dev,
> +						dev, "fwmem-%d", atomic_read(&mem1394_dev_ctr));

My eyes!

> +	if (IS_ERR(mem1394_class_member)) {
> +		printk(KERN_WARNING "mem1394: class_device_create failed\n");
> +	} else {
> +		class_set_devdata(mem1394_class_member, result);
> +	}

Unneeded braces.

> +	if (IS_ERR(mem1394_sysfs_class)) {
> +		return PTR_ERR(mem1394_sysfs_class);
> +	}

Ditto.