Hi, On 18-07-18 19:54, Samuel Morris wrote:
Adding Hans de Goede...
Thanks, patch 2/3 looks good to me. I've some remarks inline in this one, note I'm not an expert on runtime pm support, it might be good if someone more knowledgeable on this subject replies.
On Mon, Jul 16, 2018 at 10:41 PM, Sam Morris <0v3rdr0n3@xxxxxxxxx> wrote:From: Samuel Morris <samorris@xxxxxxxxxxx> This prevents the first resume of the ahci_platform device, while still allowing parent devices to be resumed. Signed-off-by: Samuel Morris <samorris@xxxxxxxxxxx> --- drivers/ata/ahci.h | 1 + drivers/ata/libahci_platform.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 1609ebab4e23..8ec39f69fae4 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -349,6 +349,7 @@ struct ahci_host_priv { u32 em_buf_sz; /* EM buffer size in byte */ u32 em_msg_type; /* EM message type */ bool got_runtime_pm; /* Did we do pm_runtime_get? */ + bool initialized; struct clk *clks[AHCI_MAX_CLKS]; /* Optional */ struct regulator **target_pwrs; /* Optional */ /* diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index feee2e11fb33..baad85adf90c 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -476,11 +476,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) goto err_out; } - pm_runtime_set_active(dev);
The set_active before enable you are removing here should already avoid resume being called on init and thus enable_resources being called twice (which I assume is the problem, resume should never be called twice itself).
- pm_runtime_enable(dev); - pm_runtime_forbid(dev); - hpriv->got_runtime_pm = true; - devres_remove_group(dev, NULL); return hpriv; @@ -594,7 +589,16 @@ int ahci_platform_init_host(struct platform_device *pdev, ahci_init_controller(host); ahci_print_info(host, "platform"); - return ahci_host_activate(host, sht); + rc = ahci_host_activate(host, sht); + if (rc) + return rc; + + pm_runtime_enable(dev); + pm_runtime_forbid(dev); + hpriv->got_runtime_pm = true; + hpriv->initialized = true; + + return 0; } EXPORT_SYMBOL_GPL(ahci_platform_init_host);
Hmm, moving thus from get_resources to platform_init_host while keeping the counter parts in put_resources makes get_resources and put_resources asymmetrical. As such I'm not a fan of this change. Regards, Hans
@@ -760,6 +764,9 @@ static int _ahci_platform_resume(struct device *dev) struct ahci_host_priv *hpriv = host->private_data; int rc; + if (!hpriv->initialized) + return 0; + rc = ahci_platform_enable_resources(hpriv); if (rc) return rc; -- 2.17.0
-- 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
