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

Arnd Bergmann arnd at arndb.de
Wed Jun 20 09:03:24 EST 2007


On Tuesday 19 June 2007, Akinobu Mita wrote:
> > +	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.

Right.

> > +	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.

yes. EFAULT should only be used when an access to user memory has failed,
so it's wrong practically everywhere in here, your other comments are
obviously correct as well. 

	Arnd <><



More information about the cbe-oss-dev mailing list