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

[Jamie Lokier <jamie@shareable.org>]

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"?

> +		/*
> +		 * 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".

> +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 can't tell from the comments.

Thanks,
-- Jamie