Hi Afzal, On 06/14/2012 12:40 AM, Mohammed, Afzal wrote: > Hi Jon, > > On Wed, Jun 13, 2012 at 22:08:47, Hunter, Jon wrote: >> On 06/13/2012 12:03 AM, Mohammed, Afzal wrote: > >>> 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). Sorry, I am not sure if I am missing something here, but isn't the chip-select requested during the gpmc probe? If so then we should not be programming the gpmc registers at all until the chip-select has been allocated. Hence, after the probe seems more appropriate. > 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. It is more of where you are doing it. I am not against putting in async mode to begin with. >> >>> 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) Exactly. >> >>> 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 If the chip-select is allocating during the probe, then I don't agree. >> 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 Great. Sorry but with 20+ patch spread across 3 series it is not always easy to find the details. So ideally it should be mentioned in this cover letter too. Thanks Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
