On Thu, 24 Mar 2011 17:13:50 +0800 Feng Tang <[email protected]> wrote:
> On Wed, 23 Mar 2011 00:57:35 +0800 > Kristen Carlson Accardi <[email protected]> wrote: > > > On Mon, 21 Mar 2011 08:53:16 +0800 > > Feng Tang <[email protected]> wrote: > > > > > Hi Kristen, > > > > > > On Sat, 19 Mar 2011 05:04:38 +0800 > > > Kristen Carlson Accardi <[email protected]> wrote: > > > > > > > On Thu, 17 Mar 2011 14:32:42 +0800 > > > > Feng Tang <[email protected]> wrote: > > > > > > > > > Hi Kristen, > > > > > > > > > > For the logical part about where should we add runtime_put/get > > > > > hook, could you try the following patch? > > > > > > > > > <snip> transfer_list); > > > > > @@ -670,6 +672,11 @@ static int dw_spi_transfer(struct > > > > > spi_device *spi, struct spi_message *msg) return -ESHUTDOWN; > > > > > } > > > > > > > > > > + spin_unlock_irqrestore(&dws->lock, flags); > > > > > + /* make sure the HW is up */ > > > > > + pm_runtime_get_sync(dws->parent_dev); > > > > > + spin_lock_irqsave(&dws->lock, flags); > > > > > + > > > > > msg->actual_length = 0; > > > > > msg->status = -EINPROGRESS; > > > > > msg->state = START_STATE; > > > > > > > > > > use pm_runtime_get_sync will make sure the driver/HW is up > > > > > and ready for the spi_message. > > > > > > > > Hi Feng, > > > > I think that dw_spi_transfer can be called from interrupt context, > > > > and my understanding is the pm_runtime_get_sync may sleep (because > > > > it has to schedule a resume). I am thinking we'll have to > > > > manually call pm_runtime_get_noresume and manually resume if we > > > > are shutdown. > > > > > > Good point. But my understanding is, for a spi device (like > > > max3110), if the dw_spi_transfer is called in interrupt context, > > > then the device itself should be in runtime running mode. And the > > > dw_spi controller as its parent, shouldn't be in runtime suspended > > > at this time, and pm_runtime_get_sync() will simple return without > > > going to sleep, and is safe to be called. > > > > Parents can be suspended independently of children. > > Yes, it could. But we better consider the case here, and can't simply > set parent>dev->power.ignore_children to 1. > > The general system suspend/resume(S3/S4) need to consider the > children/parent consideration, and I think it applies to runtime PM too. > > Thanks, > Feng > > > Personally, I feel that the parent and child *should* do their runtime suspend independently - that way if the child has to do something which doesn't require the parent, it can without making the parent stay awake. And besides that, even if you do set ignore_children to zero, you have the issue that after parent's probe time, there is a time before the child's probe routine has been called where the parent could suspend, in which case it needs to be able to be resumed. There is no reason that I can see that we cannot just always put the transfer on a queue and process it whenever we are not in interrupt. I think that is probably a better design anyway, since limiting the amount of stuff you do at interrupt time is generally preferred. the queue can then be processed after we are sure the hw is up. _______________________________________________ MeeGo-kernel mailing list [email protected] http://lists.meego.com/listinfo/meego-kernel
