On Fri, May 18, 2012 at 5:26 AM, Kevin Hilman <khilman@xxxxxx> wrote: > Tony Lindgren <tony@xxxxxxxxxxx> writes: > >> * Kevin Hilman <khilman@xxxxxx> [120517 15:29]: >>> >>> I just noticed that this patch has caused some strange problems, notably >>> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.) >>> >>> The patch itself is OK, but it has exposed a bug in other parts of the >>> context restore path that was previously hidden. >>> >>> We seem to have been finding lots of GPIO bugs by just testing the >>> network chips with GPIO IRQs. Can I suggest that a platform with a GPIO >>> IRQ NIC be added to the test platforms you're using? >> >> Yes considering the breakage it's pretty obvious the original series was >> never properly tested on omaps. > > Agreed. > Actually OMAP4 network interface as well uses the GPIO as a interrupt line but that didn't show the issue. But I agree more and more test scenario's are needed for infrastructure components like GPIO/DMA/TImer because of their multiple types of usages. >> Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp, >> and gets zoom3 nfsroot booting going a bit better. >> >> However, at least on zoom3 nfsroot now takes several _minutes_ to get to >> login: with gpio/next + this patch. The system is totally unusable. >> >> It seems that the GPIO interrupt wake-up events are not properly working >> now? >> >> Reverting the "gpio/omap: fix missing check in *_runtime_suspend()" >> patch seems to fix the issue. > > Argh, then $SUBJECT patch here has caused brokeness in multiple ways. > It managed to break both runtime suspend and runtime resume at the same > time. :( > > The change added by this patch to runtime_suspend effectively disables > the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs) > causing the sluggish network problems to reappear, since that GPIO IRQ > is no longer causing wakeups. > That's pretty bad. > Simple fix is below, which just moves the check added in $SUBJECT patch > below the workaround for the edge/level fix. Can you confirm it works > on Zoom3 (applies on gpio/next + my previous fix.) > > I didn't notice the same problem on Overo, but I guess it's because > Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't > bypass the level-triggered IRQ fix. > >>> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode >>> transitions >>> >>> The fix in commit 1b12870 (gpio/omap: fix missing check in >>> *_runtime_suspend()) exposed another bug in the context restore path. >> >> Kevin, looks like commit 1b12870 does not exist in gpio/next? > > Will update the changelog. > > Because of this new problem, I have to add the patch below too, so > I'll get them both queued up for Grant > > In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch. > > Kevin > > [1] > From afb4f0836dc3c432aa999fc98a80bf75e1481e04 Mon Sep 17 00:00:00 2001 > From: Kevin Hilman <khilman@xxxxxx> > Date: Thu, 17 May 2012 16:42:16 -0700 > Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOs > > commit 1b1287032 (gpio/omap: fix missing check in *_runtime_suspend()) > broke wakeups on level-triggered GPIOs by adding the enabled > non-wakeup GPIO check before the workaround that enables wakeups > on level-triggered IRQs, effectively disabling that workaround. > > To fix, move the enabled non-wakeup GPIO check after the > level-triggered IRQ workaround. > > Reported-by: Tony Lindgren <tony@xxxxxxxxxxx> > > Cc: Tarun Kanti DebBarma <tarun.kanti@xxxxxx> > Signed-off-by: Kevin Hilman <khilman@xxxxxx> > --- Thanks for the Fix Kevin. Acked-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx> -- 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
