On Thu, May 27, 2010 at 12:46 AM, Linus WALLEIJ <linus.walleij@xxxxxxxxxxxxxx> wrote: > [Alan Stern] >> On Wed, 26 May 2010, Linus WALLEIJ wrote: >> (...) >> > Now if this mechanism is going in, my main interest is that there >> > is some clearly cut way and design pattern for supporting both >> > runtime PM and suspend blocks. >> >> It's not difficult. You just have to decide what situations should >> block opportunistic suspend. For example, let's say that opportunistic >> suspend should be blocked when your event queue is non-empty. >> >> Then in the routine that adds new events to the queue, while still >> holding the spinlock that protects your queue, you enable your suspend >> blocker. Likewise, in the routine that takes events off the queue and >> sends them to userspace, while holding the spinlock you test whether >> the queue has become empty; if it has you disable your suspend blocker. >> >> That's all. No other changes are needed (except to create and destroy >> the suspend blocker, of course). No other interaction with suspend, >> resume, or runtime PM. > > OK I understand it will be rather OK to do this. For example I have > drivers/spi/amba-pl022.c which is quite clearly taking elements > off a queue and transmitting them. > > Currently, as we do not have suspend blockers, we try to stop the > queue in suspend() and if that fails we return something negative > so that the suspend() is blocked by the negative return code. > > Maybe the behaviour for an SPI bus should rather be to return > -EBUSY as long as there are events in the queue. I don't quite > know frankly. > > (We haven't added runtime PM yet, it will likely be the same > thing.) > > With suspend blockers we take a suspend blocker when we add > elements to the queue and release it when the queue is empty. > > But with suspend blockers the need for suspend() to check if > the queue can be stopped or return -EBUSY if there are elements > in it is not necessary: suspend() simply won't be called > since a suspend blocker is taken. There is no way for > suspend() to stop a running queue like we do today if using > suspend blockers. > > This means that driver writers only targeting configurations > where suspend blockers are used will probably not care about > handling suspend() calls by trying to stop the queue or > checking if there are things in it, because there will never > be anything there. > > So while it is easy for me to have the driver work under both > suspend blockers and runtime PM or common suspend(), the > two configurations actually have different semantics of the > suspend() call: in the suspend blockers case you don't need > to check anything, just suspend, and in the runtime PM > case you have to check that you can actually suspend. > > Is this analysis correct? > Mostly, yes. With the current suspend blocker implementation, if you only start blocking suspend from a user-space thread, or a freezable kernel thread, then your suspend hook will never be called while you block suspend unless you use forced suspend. If you for instance start blocking suspend in response to an interrupt on the other hand, there is no guarantee that drivers you depend on have not already been suspended or that they will not suspend after you block suspend, so you may have to implement a suspend hook to manually abort suspend even when using suspend blockers. You can add a check if your suspend blocker is active within your critical section in your suspend hook. Our alarm driver does this. > If yes then OK, it's not totally elegant but if that is > where we have to go, I can live with it. There will likely > be people who implement for only one or the other semantic > behaviour, but we have worse things to cope with already. > Yes, but you can support both forced suspend and opportunistic suspend in the same driver. Supporting both is harder if the suspend blocker api is not in the mainline kernel. -- Arve Hjønnevåg _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
