[PATCH] eXcite nand flash driver

[Josh Boyer]

On Thu, 2007-02-08 at 01:57 +0100, Thomas Koeller wrote:
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 358f55a..5b50396 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -216,10 +216,26 @@ config MTD_NAND_DISKONCHIP_BBTWRITE
>  	  Even if you leave this disabled, you can enable BBT writes at module
>  	  load time (assuming you build diskonchip as a module) with the module
>  	  parameter "inftl_bbt_write=1".
> -
> +	  
>  config MTD_NAND_SHARPSL
>  	tristate "Support for NAND Flash on Sharp SL Series (C7xx + others)"
>  	depends on MTD_NAND && ARCH_PXA
> + 
> +config MTD_NAND_BASLER_EXCITE
> +	tristate  "Support for NAND Flash on Basler eXcite"
> +	depends on MTD_NAND && BASLER_EXCITE
> +	help
> +          This enables the driver for the NAND flash device found on the
> +          Basler eXcite Smart Camera. If built as a module, the driver
> +	  will be named "excite_nandflash.ko".
> +
> +config MTD_NAND_BASLER_EXCITE
> +	tristate  "Support for NAND Flash on Basler eXcite"
> +	depends on MTD_NAND && BASLER_EXCITE
> +	help
> +          This enables the driver for the NAND flash device found on the
> +          Basler eXcite Smart Camera. If built as a module, the driver
> +	  will be named "excite_nandflash.ko".

You have the same config option twice...  Cut and paste error?

> diff --git a/drivers/mtd/nand/excite_nandflash.c 
> b/drivers/mtd/nand/excite_nandflash.c
> new file mode 100644
> index 0000000..d683659
> --- /dev/null
> +++ b/drivers/mtd/nand/excite_nandflash.c

<snip>

> +
> +#define io_readb(__a__)		__raw_readb((__a__))
> +#define io_writeb(__v__, __a__)	__raw_writeb((__v__), (__a__))

Do you really need these defines?  Why can't you call
__raw_{readb,writeb} directly?

> +
> +typedef void __iomem *io_reg_t;

Ugh... typdef?

<snip>

> +static inline io_reg_t
> +excite_nand_map_regs(struct platform_device *d, const char *basename)
> +{
> +	void *result = NULL;
> +	const struct resource *const r =
> +	    excite_nand_get_resource(d, IORESOURCE_MEM, basename);
> +	if (likely(r))
> +		result = ioremap_nocache(r->start, r->end + 1 - r->start);

Does this likely really buy you anything?  I would think doing the
converse (if any sort of *likely at all) would be better.

<snip>

> + */
> +static int __exit excite_nand_remove(struct device *dev)
> +{
> +	struct excite_nand_drvdata * const this = dev_get_drvdata(dev);
> +
> +	dev_set_drvdata(dev, NULL);
> +
> +	if (unlikely(!this)) {
> +		printk(KERN_ERR "%s: called %s without private data!!",
> +		       module_id, __func__);
> +		return -EINVAL;
> +	}
> +
> +	/* first thing we need to do is release our mtd
> +	 * then go through freeing the resource used
> +	 */
> +	nand_release(&this->board_mtd);
> +
> +	/* free the common resources */
> +	if (likely(this->regs)) {
> +		iounmap(this->regs);
> +		this->regs = NULL;
> +	}

Same likely usage comment as above.

josh