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

["Mike Frysinger" <vapier.adi@gmail.com>]

On Fri, Oct 10, 2008 at 17:48, Russell King - ARM Linux wrote:
> On Fri, Oct 10, 2008 at 03:19:16PM +0100, Jamie Lokier wrote:
>> Mike Rapoport wrote:
>> > +           /*
>> > +            * 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.

so the function comment should read something like "make sure the gpio
state has actually changed before returning to the higher nand layers"

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

the Blackfin code should do: SSYNC();
-mike