Messages in this thread | | | Date | Thu, 26 Feb 2009 12:33:41 -0800 | From | Andrew Morton <> | Subject | Re: [patch/RESEND 2.6.29-rc3-git] NAND: davinci_nand driver |
| |
On Tue, 24 Feb 2009 15:09:54 -0800 David Brownell <david-b@pacbell.net> wrote:
> From: David Brownell <dbrownell@users.sourceforge.net> > > This is a device driver for the NAND flash controller found on the > various DaVinci family chips. It handles up to four SoC chipselects, > and some flavors of secondary chipselect (e.g. based on upper bits > of the address bus) as used with some multichip packages. (Including > the 2 GByte chips used on some TI devel boards.) > > The 1-bit ECC hardware is supported (3 bytes ECC per 512 bytes data); > but not yet the newer 4-bit ECC (10 bytes ECC per 512 bytes data), as > available on chips like the DM355 or OMAP-L137 and needed with the > more error-prone MLC NAND chips. > > This is a cleaned-up version of code that's been in use for several > years now; sanity checked with the new drivers/mtd/tests. > > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net> > > ... > > + * Copyright (C) 2006 Texas Instruments. > + * > + * Ported to 2.6.23 Copyright (C) 2008 by > + * Sander Huijsen <Shuijsen@optelecom-nkf.com> > + * Troy Kisky <troy.kisky@boundarydevices.com> > + * Dirk Behme <Dirk.Behme@gmail.com>
hm. What's the story with authorship, attributions and signoffs here?
> > ... > > +#ifdef CONFIG_MTD_PARTITIONS > +static inline int mtd_has_partitions(void) { return 1; } > +#else > +static inline int mtd_has_partitions(void) { return 0; } > +#endif > + > +#ifdef CONFIG_MTD_CMDLINE_PARTS > +static inline int mtd_has_cmdlinepart(void) { return 1; } > +#else > +static inline int mtd_has_cmdlinepart(void) { return 0; } > +#endif
These definitions shouldn't be buried in a .c file.
> > ... > > +static void nand_davinci_hwctl_1bit(struct mtd_info *mtd, int mode) > +{ > + struct davinci_nand_info *info; > + u32 retval;
The identifier `retval' is usually used to identify the value which this function will return.
> + unsigned long flags; > + > + info = to_davinci_nand(mtd); > + > + /* Reset ECC hardware */ > + nand_davinci_readecc_1bit(mtd); > + > + spin_lock_irqsave(&davinci_nand_lock, flags); > + > + /* Restart ECC hardware */ > + retval = davinci_nand_readl(info, NANDFCR_OFFSET); > + retval |= BIT(8 + info->core_chipsel); > + davinci_nand_writel(info, NANDFCR_OFFSET, retval); > + > + spin_unlock_irqrestore(&davinci_nand_lock, flags); > +} > + > +/* > + * Read hardware ECC value and pack into three bytes > + */ > +static int nand_davinci_calculate_1bit(struct mtd_info *mtd, > + const u_char *dat, u_char *ecc_code) > +{ > + unsigned int ecc_val = nand_davinci_readecc_1bit(mtd); > + unsigned int tmp = (ecc_val & 0x0fff) | ((ecc_val & 0x0fff0000) >> 4);
argh.
> + /* invert so that erased block ecc is correct */ > + tmp = ~tmp; > + ecc_code[0] = (u_char)(tmp); > + ecc_code[1] = (u_char)(tmp >> 8); > + ecc_code[2] = (u_char)(tmp >> 16);
Is there some library facility which is being open-coded here?
> + return 0; > +} > + > > ... > > +static int __init nand_davinci_probe(struct platform_device *pdev) > +{ > + struct davinci_nand_pdata *pdata = pdev->dev.platform_data; > + struct davinci_nand_info *info; > + struct resource *res1; > + struct resource *res2; > + void __iomem *vaddr; > + void __iomem *base; > + int ret; > + u32 val; > + nand_ecc_modes_t ecc_mode; > + > + /* which external chipselect will we be managing? */ > + if (pdev->id < 0 || pdev->id > 3) > + return -ENODEV; > + > + info = kzalloc(sizeof(*info), GFP_KERNEL); > + if (!info) { > + dev_err(&pdev->dev, "unable to allocate memory\n"); > + ret = -ENOMEM; > + goto err_nomem; > + } > + > + platform_set_drvdata(pdev, info); > + > + res1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res1 || !res2) { > + dev_err(&pdev->dev, "resource missing\n"); > + ret = -EINVAL; > + goto err_res;
This leaks `info'.
Please check all the error path unwinding here.
> + } > + > > ... >
| |