Re: [PATCH] [MTD] [NAND] GPIO NAND flash driver

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Fri, Oct 10, 2008 at 03:19:16PM +0100, Jamie Lokier wrote:
> Mike Rapoport wrote:
> > +#ifdef CONFIG_ARM
> > +/* gpio_nand_dosync()
> > + *
> > + * needed due to bus-reordering within the PXA itself (see section on
> > + * I/O ordering in PXA manual (section 2.3, p35)
> > + */
> 
> Is CONFIG_ARM the same as "is a PXA"?

It's good enough.

> > +		/*
> > +		 * Linux memory barriers don't cater for what's required here.
> > +		 * What's required is what's here - a read from a separate
> > +		 * region with a dependency on that read.
> > +		 */
> 
> It would be nice if this comment explained _why_ it's needed here, not
> just what it's doing but why this MTD device in particular needs it -
> for the benefit of someone using this driver on another architecture.
> 
> If the problem is that "readl" alone doesn't force a read cycle on
> PXA, it sounds like "readl" on PXA contains a bug which may affect
> other drivers on that architecture too, and that the right place to
> fix it is in "readl".

The problem is that a write to GPIO may pass a write to the static
memory regions, or vice versa.  So, what we do is we insert a read
with a dependency in the execution to ensure that we stall the
pipeline until that read - and therefore the preceding write has
completed.

> > +static inline void gpio_nand_dosync(struct gpiomtd *gpiomtd) {}
> 
> Is this dummy implementation likely to be correct on non-ARM
> architectures, or is this just faking it so the code will compile?

I suspect what's required for various architectures has yet to be
ascertained.  To ask ARM folk to work out what's required for other
architectures is just like asking you what's required for ARM - I'd
guess that you've no idea what ARM would require.  In the same way,
we have no idea what other architectures require to ensure that the
NAND chip sees GPIO changes before actually accessing the NAND.