On Wed, Mar 13, 2013 at 11:24:01AM +0000, Santosh Shilimkar wrote: > On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote: > > Hi Santosh, > > > > On Wed, Mar 13, 2013 at 09:28:22AM +0000, Santosh Shilimkar wrote: > >> (Forgot to CC Thomas) > >> > >> On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote: > >>> With recent arm broadcast time clean-up from Mark Rutland, the dummy > >>> broadcast device is always registered with timer subsystem. And since > >>> the rating of the dummy clock event is very high, it is preferred > >>> over a real broad-cast clock event. > >>> > >>> This is a change in behavior from past and not an intended > >>> one. So reduce the rating of the dummy clockevent so that > >>> real broadcast device is selected when available. > >>> > >>> Without this all the C states with C3STOP won't work since > >>> the broad cast notifier will take an abort. > >>> > >>> Cc: Mark Rutland <mark.rutland@xxxxxxx> > >>> Cc: Russell King <linux@xxxxxxxxxxxxxxxx> > >>> > >>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx> > >>> --- > >>> Its a regression so hopefully can get into the 3.9-rcx. Noticed > >>> this one on A15 platform. A9 platform the issue may not be seen > >>> since the local timer check avoids dummy timer registration. > >>> > >> Some one pointed me to a fix made by Mark which was discussed > >> under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject. > >> That patch seems to be more of work around since the root of the > >> problem is incorrect dummy timer rating. Either way, both patches > >> fix the issue. > > > > Is the problem that the dummy timer is being registered as the broadcast > > source, or that it is selected as a local timer in preference of real timers? > > > Dummy timer is preferred over real broadcast timer. Ok. > > > If it is the former, Then I believe my patch solve the issue more generally - > > if you happen to register a dummy timer before other timers, it will become the > > broadcast source. Regardless of how temporary this is, it should never happen, > > and lowering the rating of the dummy won't fix this. > > > Well by the time we need active broadcast functionality, clock-events are > already chosen if the ratings are appropriate. That's a fair point. I still think it's worth having the check in the core broadcast code - rejecting dummy timers is always a sensible thing to do, and it prevents future crashes if new timers are added without sensible rating values. > > > If it is the latter, then this patch would ensure that a real timer with a > > rating over 100 is selected in preference to the dummy, which is certainly what > > we want. The proposed generic dummy timer in Stephen Boyd's local timer API > > removal series [1] similarly uses a low rating to ensure that real timers are > > selected in preference to an always-registered dummy. I note that the > > architected timer has a higher rating (450) than the dummy (400), so this > > shouldn't currently be a problem. > > > Because we always register dummy broadcast on ARM now > and with higher rating, it is picked as broadcast source. We definitely don't > want such a behavior when we have real broadcast device is available. Agreed. We *never* want to pick a dummy timer as a broadcast source, as this never makes sense. > With your patch, we are trying to avoid the registration which goes > against the the whole idea of registering it always and picking > the right clock-event based on rating by clock-event core. > The clock-event core except the proper ratings to be provided based on > the capability of the timer source and its resolution and in that case > dummy should have the lowest rating which is what I tried to patch. > > > Have I missed something? > > > Not much. I was just looking at x86 code as well after your email > to see how the LAPIC issue is handled. They seems to also have > correct rating to take care of the selection. > > Probably we can merge both the fixes but from clock-event core > code perspective, the ratings fix is more than enough. I do agree it'd be worth lowering the dummy timer's rating to ensure it doesn't override a real timer elsewhere. Thomas has already pulled my fix into tip timers/urgent [1]. [1] https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/urgent&id=a7dc19b8652c862d5b7c4d2339bd3c428bd29c4a Thanks, Mark. -- 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
