On Thursday, September 02, 2010, Alan Stern wrote:
> On Thu, 2 Sep 2010, Rafael J. Wysocki wrote:
>
> > > Ah, but the way this is designed means that the value to be used _must_
> > > be stored in dev_pm_info. This will become obvious when you see the
> > > code. I can send a preliminary patch if you are interested.
> >
> > Well, I guess it won't hurt. :-)
>
> Here it is. No documentation written yet, but there is some kerneldoc
> for the new routines.
OK, so my opinion is that the way it modifies pm_schedule_suspend() is
confusing. Especially this function looks awkward:
+int pm_runtime_put_delay(struct device *dev)
+{
+ int retval = 0;
+
+ if (atomic_dec_and_test(&dev->power.usage_count))
+ retval = pm_schedule_suspend(dev, 0);
+ return retval;
+}
because the "delay" doesn't even appear here and there's the 0 passed
to pm_schedule_suspend() (besides, the pm_runtime_put_* family of functions
calls _idle() rather than _suspend()).
So, instead, I'd introduce pm_request_suspend() (we don't have that yet!) that
would take the user space provided delay into account _if_ use_suspend_delay
is set. Then, the driver might either use the new pm_request_suspend(),
or override suspend_delay using pm_schedule_suspend(). [Perhaps
pm_request_suspend() can return error code if use_suspend_delay is not set,
so that it's clear the caller knows what he's doing?]
I'd also rename the above function to something like
pm_runtime_put_suspend() (frankly, I can't invent any good name right now).
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
