NeilBrown <ne...@suse.de> writes: > On Mon, 2 Jul 2012 13:26:38 -0500 Jon Hunter <jon-hun...@ti.com> wrote: > >> >> On 07/02/2012 01:07 PM, Kevin Hilman wrote: >> > + Neil Brown >> > >> > Hi Jon, >> > >> > Jon Hunter <jon-hun...@ti.com> writes: >> > >> >> Currently the gpio _runtime_resume/suspend functions are calling the >> >> get_context_loss_count() platform function if the function is populated >> >> for >> >> a gpio bank. This function is used to determine if the gpio bank logic >> >> state >> >> needs to be restored due to a power transition. This function will be >> >> populated >> >> for all banks, but it should only be called for banks that have the >> >> "loses_context" variable set. It is pointless to call this if >> >> loses_context is >> >> false as we know the context will never be lost and will not need >> >> restoring. >> >> >> >> For all OMAP2+ devices gpio bank-0 is in an always-on power domain and so >> >> will >> >> never lose context. We found that the get_context_loss_count() was being >> >> called >> >> for bank-0 during the probe and returning 1 instead of 0 indicating that >> >> the >> >> context had been lost. This was causing the context restore function to be >> >> called at probe time for this bank and because the context had never been >> >> saved, >> >> was restoring an invalid state. This ultimately resulted in a crash [1]. >> >> >> >> There are multiple bugs here that need to be addressed ... >> >> >> >> 1. Why the always-on power domain returns a context loss count of 1? This >> >> needs >> >> to be fixed in the power domain code. However, the gpio driver should >> >> not >> >> assume the loss count is 0 to begin with. >> >> 2. The omap gpio driver should never be calling get_context_loss_count >> >> for a >> >> gpio bank in a always-on domain. This is pointless and adds >> >> unneccessary >> >> overhead. >> >> 3. The OMAP gpio driver assumes that the initial power domain context >> >> loss count >> >> will be 0 at the time the gpio driver is probed. However, it could be >> >> possible that this is not the case and an invalid context restore >> >> could be >> >> performed during the probe. To avoid this otherwise only populated the >> > >> > The 'To avoid this...' sentence here doesn't read well. Looks like you >> > need to: >> > >> > s/otherwise// >> >> Yes, I meant to have dropped "otherwise" here. Thanks! >> >> > s/populated/populate/ >> >> Yes that too! I must have re-worded and screwed it up royally :-( >> >> > ? >> > >> >> get_context_loss_count() function pointer after the initial call to >> >> pm_runtime_get() has occurred. This will ensure that the first >> >> pm_runtime_put() initialised the loss count correctly. >> >> >> >> This patch addresses issues 2 and 3 above. >> >> [1] http://marc.info/?l=linux-omap&m=134065775323775&w=2 >> >> >> >> Cc: Grant Likely <grant.lik...@secretlab.ca> >> >> Cc: Linus Walleij <linus.wall...@stericsson.com> >> >> Cc: Kevin Hilman <khil...@ti.com> >> >> Cc: Tarun Kanti DebBarma <tarun.ka...@ti.com> >> >> Cc: Franky Lin <fran...@broadcom.com> >> >> >> >> Reported-by: Franky Lin <fran...@broadcom.com> >> >> Signed-off-by: Jon Hunter <jon-hun...@ti.com> >> > >> > Thanks for digging inot this bug Jon. The same bug was brought up by >> > Neil Brown (Cc'd) in a different thread. >> > >> > Neil, it looks to me that this fix will address the problems you were >> > seeing as well. Care to test, and respond with your ack/tested-by if it >> > works for you? Thanks. >> >> Neil let me know your thoughts and if you are ok, I can clean-up the >> changelog and re-send. > > Yes, works for me and looks sensible. > > Tested-by: NeilBrown <ne...@suse.de> >
Great! Thanks for testing. Jon, please make the minor changelog edits, collect the reviewed-by and tested-by tags and repost. I'll then queue this up for Grant. Based on your earlier comments, this only affects v3.5, so no need to push it into stable, correct? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
