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

On Wed, Jan 14, 2015 at 08:34:39PM +0800, Zhou Wang wrote:

On 2015/1/13 11:58, Brian Norris wrote:

quoted

On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:

quoted

+	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.

quoted

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