David Brownell <[email protected]> writes: > On Thursday 26 February 2009, Andrew Morton wrote: > >> > + * Copyright (C) 2006 Texas Instruments. >> > + * >> > + * Ported to 2.6.23 Copyright (C) 2008 by >> > + * Sander Huijsen <[email protected]> >> > + * Troy Kisky <[email protected]> >> > + * Dirk Behme <[email protected]> >> >> hm. What's the story with authorship, attributions and signoffs here? > > Written by TI (PSP team in India, ISTR) with no individual > authorship credited, and shipped with a MontaVista 2.6.10 > kernel. Ported as noted; I could presumably add my own > copyright given recent updates I've made. Likewise Felipe > Balbi. Kevin Hilman has signed off on various patches as > part of merging them to the DaVinci tree. > > (To the TI team reading this via the DaVinci list: I think > Andrew is hinting that a Signed-off-By from a TI person > would be a Nice Thing. Same for Dirk, and maybe others.) > > >> > ... >> > >> > +#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. > > I will send along a patch to move them to <linux/mtd/...> headers, > now that there seems to be a bit of recognition that the current > ifdef-centric approach in the MTD mapping drivers is trouble. ;) > > >> > >> > ... >> > >> > +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. > > True; resolved in the appended fixup patch. > > >> > +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. > > It seems the best-dressed pirates have parrots named "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? > > I don't know of such a facility: 24-bit integer into 3-byte buffer. > > >> > + return 0; >> > +} >> > + >> > >> > ... >> > >> > +static int __init nand_davinci_probe(struct platform_device *pdev) >> > +{ >> > + ... deletia ... >> > + >> > + 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. > > OK -- that does look buggish. > > (Kevin -- I suggest you merge this to the DaVinci tree to > make the eventual resync-with-mainline easier.) >
Done. It also needs this cleanup bit below which is in DaVinci git now that we've deprecated the use of the davinci cpu_is_* macros in drivers. This could be just folded into current patch if desired. Thanks, Kevin commit 1bacc33ccc9bd0f3c109bf8a8550e9b6f99397bd Author: Kevin Hilman <[email protected]> Date: Thu Feb 26 17:15:18 2009 -0800 MTD: NAND: drop usage of cpu_is_* macro Usage of davinci-specific cpu_is macros is not allowed in drivers. These options should be passed in through platform_data. Signed-off-by: Kevin Hilman <[email protected]> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index aa70b4e..a2f78ad 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -33,7 +33,6 @@ #include <linux/mtd/nand.h> #include <linux/mtd/partitions.h> -#include <mach/cpu.h> #include <mach/nand.h> #include <asm/mach-types.h> @@ -392,8 +391,6 @@ static int __init nand_davinci_probe(struct platform_device *pdev) /* use board-specific ECC config; else, the best available */ if (pdata) ecc_mode = pdata->ecc_mode; - else if (cpu_is_davinci_dm355()) - ecc_mode = NAND_ECC_HW_SYNDROME; else ecc_mode = NAND_ECC_HW; _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
