Hi,
On 26-04-18 21:15, Tejun Heo wrote:
(cc'ing Hans. Can you please take a look at the patchset?)
On Fri, Apr 20, 2018 at 10:18:33AM +0000, 0v3rdr0n3@xxxxxxxxx wrote:
From: Samuel Morris <samorris@xxxxxxxxxxx>
A number of resources remain powered to support hotplug. On platforms
I've worked with, allowing the ahci_platform to suspend saves about
150mW. This patch allows the device to be auto suspended if the
config parameter is set.
The idea looks good to me but I really wish it were something which
can be turned on/off runtime rather than baked into a CONFIG option
(we can add a CONFIG option to select the default behavior).
I agree with your assessment, the idea looks good, but the
implementation really needs to change.
Here is how I think this should work:
-Always have runtime_pm callbacks, no #ifdef-ery for these
-The third patch calls runtime_pm_allow() from a weird place, the
proper way with a device being attached to a port or not is to
have the scan_host code do a runtime_pm_get_sync() before scanning
and do a put again when no device is found or keep the reference
when a device is found, this can be done always even for any ata
drivers which do not support runtime_pm, the calls will be nops
there, this also removes the weird #ifdef from the 3th patch
-Then on unplug the ref should be released by calling runtime_pm_put(),
this way on a hot unplug runtime pm will start working after the
unplug
-Never call runtime_pm_allow() directly from the code, instead
users who want this can echo "auto" to the power/control sysfs
attribute, this will also give more fine grained (per device)
control over this. There are a number of examples in the kernel
of drivers implementing runtime-pm but needing such an echo to
enable it. A good example is almost all USB device drivers.
So how does this sound?
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
