Re: [Qemu-devel] [ARM] Unused OMAP NAND support - can we remove?

[Peter Crosthwaite]

On Fri, Jan 10, 2014 at 7:02 PM, Peter Maydell <address@hidden> wrote:
> On 10 January 2014 08:47, Peter Crosthwaite
> <address@hidden> wrote:
>> I'm trying to apply a QOMification change pattern to NAND and I notice
>> that OMAP gpmc has NAND support. However no one is using it - there
>> are no calls to the omap_gpmc_attach_nand, nor does the code call
>> nand_init, leaving me to believe this is dead code. The fact that it
>> is so far out of stylistic date makes me think its best deleted and
>> can be updated and re-added later if wanted. Otherwise I need to apply
>> my change pattern to dead code which is not ideal.
>
> This is used by the omap3 patchset (git://git.linaro.org/qemu/qemu-linaro.git
> 'rebasing' branch). At the time I got the GPMC changes upstream I was
> planning to get the remainder of OMAP3 upstream in the immediate
> future, which is why the support is there. Unfortunately priorities
> changed and OMAP3 upstreaming got put on the back burner (partly
> because it's a huge job). I'd still like to get OMAP3 upstream someday,
> so if it's not too awful to apply your change to omap_gpmc_attach_nand()
> I think that would be preferable.
>

So on reviewing this controller properly just this morning, it's
actually one of the better examples for the conversion. My change
pattern is the BUSification of NAND and in doing so, it is naturally
adding support for multiple NANDs on the one bus, which is actually
what GPMC is try to emulate.

Currently GPMC is just an array of N (DeviceState *) pointers to the N
NAND devices and bangs them individually depending on which CS window
was hit. Instead following my change, GPMC would have only one
(NANDBus *), and correctly drive the N CS lines as GPIOs (will need to
be added) in the same way real HW does.

It will be a functional change as we are now modelling the proper
interleaving CS behaviour on GPMC, but if anything, it's actually
going to be an increase in modeling fidelity.

Regards,
Peter

> The specific users of the function in the omap3 code are the board
> model files like hw/arm/beagle.c, which do:
>
>      s->nand = nand_init(dmtd ? dmtd->bdrv : NULL, NAND_MFR_MICRON, 0xba);
>      nand_setpins(s->nand, 0, 0, 0, 1, 0); /* no write-protect */
>      omap_gpmc_attach_nand(s->cpu->gpmc, BEAGLE_NAND_CS, s->nand);
>
> thanks
> -- PMM
>