On Tuesday 09 December 2008, Felipe Balbi wrote:
> +struct davinci_nand_info {
> + /* device lock */
> + spinlock_t lock;
> + struct nand_chip chip;
> + struct mtd_info mtd;
>
> -/*
> - * MTD structure for DaVinici board
> - */
> -static struct mtd_info *nand_davinci_mtd;
> + struct device *dev;
> + struct clk *clk;
> +
> + void __iomem *base;
> + void __iomem *vaddr;
> +};
> +
> +#define to_davinci_nand(m) container_of(m, struct davinci_nand_info, mtd)
Minor nit: put "mtd" first and this becomes a NOP by the
time code generation completes, so code is a bit smaller
and register pressure is a bit less.
Could you briefly explain why the spinlock is needed?
Just for the HW ECC support, or is there more to it?
Most NAND drivers don't use them internally ... if
this really needs a spinlock, say why. I suspect
that you don't need one either, or at least you don't
need one everywhere it's now used.
> @@ -287,7 +319,7 @@ static int nand_davinci_memory_bbt(struct mtd_info
> *mtd, from += blocksize;
> }
>
> - printk(KERN_NOTICE "Bad block scan: %d out of %d blocks are bad.\n",
> + dev_dbg(info->dev, "Bad block scan: %d out of %d blocks are bad.\n",
> mtd->ecc_stats.badblocks, numblocks>>1);
> return 0;
I'd think that should be dev_notice(). And ... dev_*() changes do NOT
relate to $SUBJECT. Such messaging patches would better be split out
from this patch... which is already kind of large.
> @@ -408,10 +451,13 @@ static void nand_davinci_set_eccbytes(struct nand_chip
> *chip)
> #endif
> }
>
> -static void __devinit nand_davinci_flash_init(void)
> +static void __devinit nand_davinci_flash_init(struct davinci_nand_info
> *info) {
> + unsigned long flags;
> u32 regval, tmp;
>
> + spin_lock_irqsave(&info->lock, flags);
> +
> /* Check for correct pin mux, reconfigure if necessary */
> tmp = davinci_readl(DAVINCI_SYSTEM_MODULE_BASE + PINMUX0);
Didn't you say you were wondering why this driver misbhaves on dm355?
That's part of the clue! PINMUX0 on dm355 is for video stuff; I
think it's PINMUX2 that's for AEMIF. This driver is assuming that
it's on a dm6446, I bet. (Not that it should ever do pinmuxing
directly, in any case...)
There's work afoot to generalize the pinmux stuff; not sure when
it'll arrive with dm355 support.
- Dave
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
