Hi Jon,

On Wed, Jun 13, 2012 at 22:08:47, Hunter, Jon wrote:

On 06/13/2012 12:03 AM, Mohammed, Afzal wrote:

quoted

As gpmc_onenand_setup is a callback by onenand driver, we would have
lost the opportunity to configure onenand before driver is probed.

Is that a problem? Looks like it is called early in the probe and so I
would hope no one is attempting to access the onenand itself before the
probe has completed.

During gpmc driver probe, it will configure all the connected peripherals,
if configuration details are not present at that point of time, gpmc driver
will cry out saying that configuration & timings has not been configured,
(please see holler if no configuration patch). And I do not see any reason
why gpmc driver should not be fed with async mode configuration initially,
as it has to be done always.

quoted

This would cause requirement of double GPMC configuring and we lost
the opportunity to configure GPMC before driver is probed.

I am not convinced we need to. Furthermore with your change you do not
actually set async mode in the onenand until _set_sync() is called.

Yes, setting async mode in onenand is done in set_sync function, and it is
always called by onenand driver indirectly.

Seems if setting async mode in onenand is taken out of set_sync & placed
it before set_sync invocation in gpmc_onenand_setup, intention will be
clear, right ? (even though sequence wise same thing is happening now)

quoted

And the first step for onenand configuration is always to set it
to async mode (with the way it is done now), so it seems reasonable
to rely on normal GPMC configuration for async & then do reconfigure
for sync.

Yes but as far as I can see, it seems that this is the intent of the
onenand_setup() function to perform the necessary initialisation.

I believe doing it in gpmc_onenand_init is better, due to the reasons
mentioned above as well as because function name will correspond to what
it is doing, i.e. initialization

Have you tested onenand? Do you have a board with onenand?

Yes, on omap3evm, with help of local patches (as mainline doesn't have
those). It was mentioned in cover letter of gpmc driver conversion series

Regards
Afzal