On Wed, Jun 20, 2018 at 11:27 AM, Felipe Balbi <balbi@xxxxxxxxxx> wrote: > > Hi, > > Tony Lindgren <tony@xxxxxxxxxxx> writes: >> * Felipe Balbi <balbi@xxxxxxxxxx> [180619 01:23]: >>> This is a direct consequence of not paying attention to the order of >>> things. If driver were to assume that pm_domain->activate() would do the >>> right thing for the device -- meaning that probe would run with an >>> active device --, then we wouldn't need that pm_runtime_get() call on >>> probe at all. Rather we would follow the sequence: >>> >>> pm_runtime_forbid() Why do you need the _forbid() thing? Does the default user space control setting need to be changed? >>> pm_runtime_set_active() >>> pm_runtime_enable() >>> >>> /* do your probe routine */ >>> >>> pm_runtime_put_noidle() >>> >>> Then you remove you would need to call pm_runtime_get_noresume() to >>> balance out the pm_runtime_put_noidle() there. >> >> How about let's create some prettier interface for the above runtime PM >> trickery? >> >> How about something like pm_runtime_init_enabled() for the above >> sequence? And then have a separate helper for every other use case? Come on. >> It might be then able to do the trick even if activate is not >> implemented.. >> >> Right now it has the feeling of "oh well we can't get runtime PM to >> work so let's bypass it with activate call and then trick runtime PM >> to start in enabled mode" :) > > no strong feelings about that either way. It's only 3 lines of code > anyway. I feel like that's a minor cosmetic change. > >>> (If you need to know why the pm_runtime_put_noidle(), remember that >>> pm_runtime_set_active() increments the usage counter, so >>> pm_runtime_put_noidle is basically allowing pm_runtime to happen as soon >>> as userspace writes "auto" to /sys/..../power/control) >> >> I wonder if we could also remove the need for drivers to call >> pm_runtime_putnoidle() at the end of the probe? If we had >> pm_runtime_init_enabled() implemented. > > probably not. We want to block runtime pm during probe, until the device > is fully initialized, the only way to do that is to increment rpm usage > counter. That or enable it only at the end. But I guess you want to runtime-resume it earlier, don't you? -- 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
