Re: [PATCH V3] drivers/mtd: add powernv flash MTD abstraction driver

[Neelesh Gupta <neelegup@linux.vnet.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]


[...]
> +/**
> + * @mtd: the device
> + * @erase: the erase info
> + * Returns 0 if erase successful or -ERRNO if an error occurred
> + */
> +static int powernv_flash_erase(struct mtd_info *mtd, struct erase_info *erase)
> +{
> +	int rc;
> +
> +	erase->state = MTD_ERASING;
> +
> +	/* todo: register our own notifier to do a true async implementation */
> +	rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
> +			erase->len, NULL, NULL);
> +
> +	if (rc) {
> +		erase->fail_addr = erase->addr;
> +		erase->state = MTD_ERASE_FAILED;
> +	} else {
> +		erase->state = MTD_ERASE_DONE;
> +	}
> +	mtd_erase_callback(erase);

return rc ? You also document the same  '.... or -ERRNO if an error 
occurred'

> +	return 0;
> +}
> +
> +/**
> + * powernv_flash_set_driver_info - Fill the mtd_info structure and docg3
> + * structure @pdev: The platform device
> + * @mtd: The structure to fill
> + */
> +static int powernv_flash_set_driver_info(struct device *dev,
> +		struct mtd_info *mtd)
> +{
> +	u64 size;
> +	u32 erase_size;
> +	int rc;
> +
> +	rc = of_property_read_u32(dev->of_node, "ibm,flash-block-size",
> +			&erase_size);
> +	if (rc) {
> +		dev_err(dev, "couldn't get resource block size information\n");
> +		return rc;
> +	}
> +
> +	rc = of_property_read_u64(dev->of_node, "reg", &size);
> +	if (rc) {
> +		dev_err(dev, "couldn't get resource size information\n");
> +		return rc;
> +	}
> +
> +	/*
> +	 * Going to have to check what details I need to set and how to
> +	 * get them
> +	 */
> +	mtd->name = of_get_property(dev->of_node, "name", NULL);
> +	mtd->type = MTD_NORFLASH;
> +	mtd->flags = MTD_WRITEABLE;
> +	mtd->size = size;
> +	mtd->erasesize = erase_size;
> +	mtd->writebufsize = mtd->writesize = 1;
> +	mtd->owner = THIS_MODULE;
> +	mtd->_erase = powernv_flash_erase;
> +	mtd->_read = powernv_flash_read;
> +	mtd->_write = powernv_flash_write;
> +	mtd->dev.parent = dev;
> +	return 0;
> +}
> +
> +/**
> + * powernv_flash_probe
> + * @pdev: platform device
> + *
> + * Returns 0 on success, -ENOMEM, -ENXIO on error
> + */
> +static int powernv_flash_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct powernv_flash *data;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	data->mtd.priv = data;

'mtd' is contained within the 'data' so you can cast 'mtd' to get the 'data'
anywhere you want using container_of() macro.. 'priv' can be used to pass
an unrelated structure ....  just a thought, you may ignore it.. :)

Rest looks ok.

Neelesh.



[-- Attachment #2: Type: text/html, Size: 3205 bytes --]