On Wednesday 28 April 2010, Alan Stern wrote:
> On Tue, 27 Apr 2010, [UTF-8] Arve Hjønnevåg wrote:
>
> > +For example, in cell phones or other embedded systems, where powering the screen
> > +is a significant drain on the battery, suspend blockers can be used to allow
> > +user-space to decide whether a keystroke received while the system is suspended
> > +should cause the screen to be turned back on or allow the system to go back into
> > +suspend. Use set_irq_wake or a platform specific api to make sure the keypad
> > +interrupt wakes up the cpu. Once the keypad driver has resumed, the sequence of
> > +events can look like this:
> > +
> > +- The Keypad driver gets an interrupt. It then calls suspend_block on the
> > + keypad-scan suspend_blocker and starts scanning the keypad matrix.
> > +- The keypad-scan code detects a key change and reports it to the input-event
> > + driver.
> > +- The input-event driver sees the key change, enqueues an event, and calls
> > + suspend_block on the input-event-queue suspend_blocker.
> > +- The keypad-scan code detects that no keys are held and calls suspend_unblock
> > + on the keypad-scan suspend_blocker.
> > +- The user-space input-event thread returns from select/poll, calls
> > + suspend_block on the process-input-events suspend_blocker and then calls read
> > + on the input-event device.
> > +- The input-event driver dequeues the key-event and, since the queue is now
> > + empty, it calls suspend_unblock on the input-event-queue suspend_blocker.
> > +- The user-space input-event thread returns from read. If it determines that
> > + the key should leave the screen off, it calls suspend_unblock on the
> > + process_input_events suspend_blocker and then calls select or poll. The
> > + system will automatically suspend again, since now no suspend blockers are
> > + active.
> > +
> > + Key pressed Key released
> > + | |
> > +keypad-scan ++++++++++++++++++
> > +input-event-queue +++ +++
> > +process-input-events +++ +++
>
> This is better than before, but it still isn't ideal. Here's what I
> mean:
>
> > suspend blockers can be used to allow
> > +user-space to decide whether a keystroke received while the system is suspended
> > +should cause the screen to be turned back on or allow the system to go back into
> > +suspend.
>
> That's not right. Handling the screen doesn't need suspend blockers:
> The program decides what to do and then either turns on the screen or
> else writes "mem" to /sys/power/state. What suspend blockers add is
> the ability to resolve races and satisfy multiple constraints when
> going into suspend -- which has nothing to do with operating the
> screen.
>
> I _think_ what you're trying to get at can be expressed this way:
>
> Here's an example showing how a cell phone or other embedded
> system can handle keystrokes (or other input events) in the
> presence of suspend blockers. Use set_irq_wake...
>
> ...
>
> - The user-space input-event thread returns from read. It
> carries out whatever activities are appropriate (for example,
> powering up the display screen, running other programs, and so
> on). When it is finished, it calls suspend_unblock on the
> process_input_events suspend_blocker and then calls select or
> poll. The system will automatically suspend again when it is
> idle and no suspend blockers remain active.
Yeah, that sounds better. Arve, what do you think?
> > +/**
> > + * suspend_block() - Block suspend
> > + * @blocker: The suspend blocker to use
> > + *
> > + * It is safe to call this function from interrupt context.
> > + */
> > +void suspend_block(struct suspend_blocker *blocker)
> > +{
> > + unsigned long irqflags;
> > +
> > + if (WARN_ON(!(blocker->flags & SB_INITIALIZED)))
> > + return;
> > +
> > + spin_lock_irqsave(&list_lock, irqflags);
> > + blocker->flags |= SB_ACTIVE;
> > + list_del(&blocker->link);
> > +
> > + if (debug_mask & DEBUG_SUSPEND_BLOCKER)
> > + pr_info("suspend_block: %s\n", blocker->name);
> > +
> > + list_add(&blocker->link, &active_blockers);
>
> Here and in suspend_unblock(), you can use list_move() in place of
> list_del() followed by list_add().
Indeed. And the debug statement might be moved out of the critical section IMHO.
Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
