[Cbe-oss-dev] [patch 3/5] cell: updated driver for DDR2 memory on AXON

[Akinobu Mita]

> +static int
> +axon_ram_probe(struct of_device *device, const struct of_device_id *device_id)
> +{
> +	static int axon_ram_bank_id = -1;
> +	struct axon_ram_bank *bank;
> +	struct resource resource;
> +	int rc = 0;
> +
> +	axon_ram_bank_id++;
> +
> +	dev_info(&device->dev, "Found memory controller on %s\n",
> +			device->node->full_name);
> +
> +	bank = kzalloc(sizeof(struct axon_ram_bank), GFP_KERNEL);
> +	if (bank == NULL) {
> +		dev_err(&device->dev, "Out of memory\n");
> +		rc = -ENOMEM;
> +		goto failed;
> +	}
> +
> +	device->dev.platform_data = bank;
> +
> +	bank->device = device;
> +
> +	if (of_address_to_resource(device->node, 0, &resource) != 0) {
> +		dev_err(&device->dev, "Cannot access device tree\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}

of_address_to_resource() returns error code on failure:

	rc = of_address_to_resource(device->node, 0, &resource);
	if (rc) {
		dev_err(&device->dev, "Cannot access device tree\n");
		goto failed;
	}

is better.

> +
> +	bank->size = resource.end - resource.start + 1;
> +
> +	if (bank->size == 0) {
> +		dev_err(&device->dev, "No DDR2 memory found for %s%d\n",
> +				AXON_RAM_DEVICE_NAME, axon_ram_bank_id);
> +		rc = -ENODEV;
> +		goto failed;
> +	}
> +
> +	dev_info(&device->dev, "Register DDR2 memory device %s%d with %luMB\n",
> +			AXON_RAM_DEVICE_NAME, axon_ram_bank_id, bank->size >> 20);
> +
> +	bank->ph_addr = resource.start;
> +	bank->io_addr = (unsigned long) ioremap_flags(
> +			bank->ph_addr, bank->size, _PAGE_NO_CACHE);
> +	if (bank->io_addr == 0) {
> +		dev_err(&device->dev, "ioremap() failed\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}
> +
> +	bank->disk = alloc_disk(AXON_RAM_MINORS_PER_DISK);
> +	if (bank->disk == NULL) {
> +		dev_err(&device->dev, "Cannot register disk\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}

-ENOMEM is better than -EFAULT. Because alloc_disk() failure happens
only when it runs out of memory.

> +
> +	bank->disk->first_minor = 0;
> +	bank->disk->fops = &axon_ram_devops;
> +	bank->disk->private_data = bank;
> +	bank->disk->driverfs_dev = &device->dev;
> +
> +	sprintf(bank->disk->disk_name, "%s%d",
> +			AXON_RAM_DEVICE_NAME, axon_ram_bank_id);
> +	bank->disk->major = register_blkdev(0, bank->disk->disk_name);
> +	if (bank->disk->major < 0) {
> +		dev_err(&device->dev, "Cannot register block device\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}

rc = bank->disk->major is better than -EFAULT. Because register_blkdev()
returns adequate error code on failure.

> +
> +	bank->disk->queue = blk_alloc_queue(GFP_KERNEL);
> +	if (bank->disk->queue == NULL) {
> +		dev_err(&device->dev, "Cannot register disk queue\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}

-ENOMEM is better. blk_alloc_queue() only fails when it runs out of memory.

> +
> +	set_capacity(bank->disk, bank->size >> AXON_RAM_SECTOR_SHIFT);
> +	blk_queue_make_request(bank->disk->queue, axon_ram_make_request);
> +	blk_queue_hardsect_size(bank->disk->queue, AXON_RAM_SECTOR_SIZE);
> +	add_disk(bank->disk);
> +
> +	bank->irq_correctable = irq_of_parse_and_map(device->node, 0);
> +	bank->irq_uncorrectable = irq_of_parse_and_map(device->node, 1);
> +	if ((bank->irq_correctable <= 0) || (bank->irq_uncorrectable <= 0)) {
> +		dev_err(&device->dev, "Cannot access ECC interrupt ID\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}
> +
> +	rc = request_irq(bank->irq_correctable, axon_ram_irq_handler,
> +			AXON_RAM_IRQ_FLAGS, bank->disk->disk_name, device);
> +	if (rc != 0) {
> +		dev_err(&device->dev, "Cannot register ECC interrupt handler\n");
> +		bank->irq_correctable = bank->irq_uncorrectable = 0;
> +		rc = -EFAULT;
> +		goto failed;
> +	}

request_irq() returns error code on failure.
There is no need to set -EFAULT.

> +
> +	rc = request_irq(bank->irq_uncorrectable, axon_ram_irq_handler,
> +			AXON_RAM_IRQ_FLAGS, bank->disk->disk_name, device);
> +	if (rc != 0) {
> +		dev_err(&device->dev, "Cannot register ECC interrupt handler\n");
> +		bank->irq_uncorrectable = 0;
> +		rc = -EFAULT;
> +		goto failed;
> +	}

Ditto.

> +
> +	rc = device_create_file(&device->dev, &dev_attr_ecc);
> +	if (rc != 0) {
> +		dev_err(&device->dev, "Cannot create sysfs file\n");
> +		rc = -EFAULT;
> +		goto failed;
> +	}

device_create_file() returns error code.
There is no need to set -EFAULT.