[PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Brian Norris]

On Wed, Jan 14, 2015 at 08:34:39PM +0800, Zhou Wang wrote:
> On 2015/1/13 11:58, Brian Norris wrote:
> > On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >> +	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);
> > 
> > Hmm, do you really have to reuse IO_ADDR_{R,W} here? Those are only
> > targeted for NAND systems which have a direct MMIO mapping to the NAND
> > I/O pins. See nand_base's {read,write}_buf() and read_{byte,word}()
> > implementations. But you override those.
> 
> There is a hardware buffer in this NAND controller, and the buffer can be
> accessed as MMIO.

Sure.

> IO_ADDR_R/W just indicates the base address of this buffer.

But I was noting that IO_ADDR_{R,W} actually serve a very particular
purpose in nand_base.c, which seems distinct from your HW buffer.

> Maybe I need to use a void __iomem pointer stored in my host struct to use
> this buffer instead of IO_ADDR_R/W as you said below here?

Yes, I think that would be better.

> > It's best if it's obvious if nand_base is somehow inadvertently using
> > these pointers. So leaving them NULL is helpful.
> > 
> > As an alternative, you can just stash another private void __iomem
> > pointer in you your host struct.

Thanks,
Brian