[PATCH v2 0/2] ata: ahci_platform: Add PHY support and OMAP support

[PATCH v2 0/2] ata: ahci_platform: Add PHY support and OMAP support

[Roger Quadros]

Hi,

Some platforms have a PHY hooked up to the SATA controller.
The PHY needs to be initialized and powered up for SATA to work.
We do that using the Generic PHY framework. [1]. This is done
in PATCH 1

In order to support SATA on the OMAP platforms we need to runtime
resume the device before use. PATCH 2 takes care of that.

Changelog:
v2:
- got rid of Texas Instruments SATA wrapper driver
- adressed review comments

cheers,
-roger

[1] - The Generic PHY framework is available in Greg KH's usb-next branch.
  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git  usb-next

---
Balaji T K (1):
  ata: ahci_platform: Manage SATA PHY

Roger Quadros (1):
  ata: ahci_platform: runtime resume the device before use

 .../devicetree/bindings/ata/ahci-platform.txt      |    3 +-
 drivers/ata/ahci.h                                 |    2 +
 drivers/ata/ahci_platform.c                        |   41 +++++++++++++++++++-
 3 files changed, 44 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/2] ata: ahci_platform: Manage SATA PHY

[Roger Quadros]

From: Balaji T K <balajitk@ti.com>

Some platforms have a PHY hooked up to the
SATA controller. The PHY needs to be initialized
and powered up for SATA to work. We do that
using the PHY framework.

[Roger Q] Cleaned up.

CC: Tejun Heo <tj@kernel.org>
Signed-off-by: Balaji T K <balajitk@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/ata/ahci-platform.txt      |    3 +-
 drivers/ata/ahci.h                                 |    2 ++
 drivers/ata/ahci_platform.c                        |   29 +++++++++++++++++++-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 89de156..c8a6cea 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -4,7 +4,8 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "snps,spear-ahci"
+- compatible        : compatible list, contains "snps,spear-ahci",
+			snps,exynos5440-ahci or "snps,dwc-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 1145637..94484cb 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -37,6 +37,7 @@
 
 #include <linux/clk.h>
 #include <linux/libata.h>
+#include <linux/phy/phy.h>
 
 /* Enclosure Management Control */
 #define EM_CTRL_MSG_TYPE              0x000f0000
@@ -322,6 +323,7 @@ struct ahci_host_priv {
 	u32			em_buf_sz;	/* EM buffer size in byte */
 	u32			em_msg_type;	/* EM message type */
 	struct clk		*clk;		/* Only for platforms supporting clk */
+	struct phy		*phy;		/* If platforms use phy */
 	void			*plat_data;	/* Other platform data */
 };
 
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 2daaee0..5a0f1418 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -141,6 +141,21 @@ static int ahci_probe(struct platform_device *pdev)
 		}
 	}
 
+	hpriv->phy = devm_phy_get(dev, "sata-phy");
+	if (IS_ERR(hpriv->phy)) {
+		dev_dbg(dev, "can't get sata-phy\n");
+		/* return only if -EPROBE_DEFER */
+		if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
+			rc = -EPROBE_DEFER;
+			goto disable_unprepare_clk;
+		}
+	}
+
+	if (!IS_ERR(hpriv->phy)) {
+		phy_init(hpriv->phy);
+		phy_power_on(hpriv->phy);
+	}
+
 	/*
 	 * Some platforms might need to prepare for mmio region access,
 	 * which could be done in the following init call. So, the mmio
@@ -150,7 +165,7 @@ static int ahci_probe(struct platform_device *pdev)
 	if (pdata && pdata->init) {
 		rc = pdata->init(dev, hpriv->mmio);
 		if (rc)
-			goto disable_unprepare_clk;
+			goto disable_phy;
 	}
 
 	ahci_save_initial_config(dev, hpriv,
@@ -220,6 +235,12 @@ static int ahci_probe(struct platform_device *pdev)
 pdata_exit:
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
+disable_phy:
+	if (!IS_ERR(hpriv->phy)) {
+		phy_power_off(hpriv->phy);
+		phy_exit(hpriv->phy);
+	}
+
 disable_unprepare_clk:
 	if (!IS_ERR(hpriv->clk))
 		clk_disable_unprepare(hpriv->clk);
@@ -238,6 +259,11 @@ static void ahci_host_stop(struct ata_host *host)
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
 
+	if (!IS_ERR(hpriv->phy)) {
+		phy_power_off(hpriv->phy);
+		phy_exit(hpriv->phy);
+	}
+
 	if (!IS_ERR(hpriv->clk)) {
 		clk_disable_unprepare(hpriv->clk);
 		clk_put(hpriv->clk);
@@ -328,6 +354,7 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_suspend, ahci_resume);
 static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "snps,spear-ahci", },
 	{ .compatible = "snps,exynos5440-ahci", },
+	{ .compatible = "snps,dwc-ahci", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ahci_of_match);
-- 
1.7.9.5

Re: [PATCH v2 1/2] ata: ahci_platform: Manage SATA PHY

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, October 16, 2013 02:42:52 PM Roger Quadros wrote:
> From: Balaji T K <balajitk@ti.com>
> 
> Some platforms have a PHY hooked up to the
> SATA controller. The PHY needs to be initialized
> and powered up for SATA to work. We do that
> using the PHY framework.
> 
> [Roger Q] Cleaned up.
> 
> CC: Tejun Heo <tj@kernel.org>
> Signed-off-by: Balaji T K <balajitk@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/ata/ahci-platform.txt      |    3 +-
>  drivers/ata/ahci.h                                 |    2 ++
>  drivers/ata/ahci_platform.c                        |   29 +++++++++++++++++++-
>  3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 89de156..c8a6cea 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -4,7 +4,8 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
>  Each SATA controller should have its own node.
>  
>  Required properties:
> -- compatible        : compatible list, contains "snps,spear-ahci"
> +- compatible        : compatible list, contains "snps,spear-ahci",
> +			snps,exynos5440-ahci or "snps,dwc-ahci"

minor nit:
s/snps,exynos5440-ahci/"snps,exynos5440-ahci"/

>  - interrupts        : <interrupt mapping for SATA IRQ>
>  - reg               : <registers mapping>
>  
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 1145637..94484cb 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -37,6 +37,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/libata.h>
> +#include <linux/phy/phy.h>
>  
>  /* Enclosure Management Control */
>  #define EM_CTRL_MSG_TYPE              0x000f0000
> @@ -322,6 +323,7 @@ struct ahci_host_priv {
>  	u32			em_buf_sz;	/* EM buffer size in byte */
>  	u32			em_msg_type;	/* EM message type */
>  	struct clk		*clk;		/* Only for platforms supporting clk */
> +	struct phy		*phy;		/* If platforms use phy */

minor nit:
s/If platforms use phy/If platform uses PHY/

>  	void			*plat_data;	/* Other platform data */
>  };
>  
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 2daaee0..5a0f1418 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -141,6 +141,21 @@ static int ahci_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	hpriv->phy = devm_phy_get(dev, "sata-phy");
> +	if (IS_ERR(hpriv->phy)) {
> +		dev_dbg(dev, "can't get sata-phy\n");
> +		/* return only if -EPROBE_DEFER */
> +		if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
> +			rc = -EPROBE_DEFER;
> +			goto disable_unprepare_clk;
> +		}
> +	}
> +
> +	if (!IS_ERR(hpriv->phy)) {
> +		phy_init(hpriv->phy);
> +		phy_power_on(hpriv->phy);
> +	}
> +
>  	/*
>  	 * Some platforms might need to prepare for mmio region access,
>  	 * which could be done in the following init call. So, the mmio
> @@ -150,7 +165,7 @@ static int ahci_probe(struct platform_device *pdev)
>  	if (pdata && pdata->init) {
>  		rc = pdata->init(dev, hpriv->mmio);
>  		if (rc)
> -			goto disable_unprepare_clk;
> +			goto disable_phy;
>  	}
>  
>  	ahci_save_initial_config(dev, hpriv,
> @@ -220,6 +235,12 @@ static int ahci_probe(struct platform_device *pdev)
>  pdata_exit:
>  	if (pdata && pdata->exit)
>  		pdata->exit(dev);
> +disable_phy:
> +	if (!IS_ERR(hpriv->phy)) {
> +		phy_power_off(hpriv->phy);
> +		phy_exit(hpriv->phy);
> +	}
> +
>  disable_unprepare_clk:
>  	if (!IS_ERR(hpriv->clk))
>  		clk_disable_unprepare(hpriv->clk);
> @@ -238,6 +259,11 @@ static void ahci_host_stop(struct ata_host *host)
>  	if (pdata && pdata->exit)
>  		pdata->exit(dev);
>  
> +	if (!IS_ERR(hpriv->phy)) {
> +		phy_power_off(hpriv->phy);
> +		phy_exit(hpriv->phy);
> +	}
> +
>  	if (!IS_ERR(hpriv->clk)) {
>  		clk_disable_unprepare(hpriv->clk);
>  		clk_put(hpriv->clk);
> @@ -328,6 +354,7 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_suspend, ahci_resume);

Shouldn't phy_power_off/on also be used in ahci_suspend() and ahci_resume()?

>  static const struct of_device_id ahci_of_match[] = {
>  	{ .compatible = "snps,spear-ahci", },
>  	{ .compatible = "snps,exynos5440-ahci", },
> +	{ .compatible = "snps,dwc-ahci", },

This change together with ahci-platform.txt one should be in a separate patch.

>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, ahci_of_match);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v2 1/2] ata: ahci_platform: Manage SATA PHY

[Roger Quadros]

On 10/17/2013 04:57 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, October 16, 2013 02:42:52 PM Roger Quadros wrote:
>> From: Balaji T K <balajitk@ti.com>
>>
>> Some platforms have a PHY hooked up to the
>> SATA controller. The PHY needs to be initialized
>> and powered up for SATA to work. We do that
>> using the PHY framework.
>>
>> [Roger Q] Cleaned up.
>>
>> CC: Tejun Heo <tj@kernel.org>
>> Signed-off-by: Balaji T K <balajitk@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/ata/ahci-platform.txt      |    3 +-
>>  drivers/ata/ahci.h                                 |    2 ++
>>  drivers/ata/ahci_platform.c                        |   29 +++++++++++++++++++-
>>  3 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> index 89de156..c8a6cea 100644
>> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> @@ -4,7 +4,8 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
>>  Each SATA controller should have its own node.
>>  
>>  Required properties:
>> -- compatible        : compatible list, contains "snps,spear-ahci"
>> +- compatible        : compatible list, contains "snps,spear-ahci",
>> +			snps,exynos5440-ahci or "snps,dwc-ahci"
> 
> minor nit:
> s/snps,exynos5440-ahci/"snps,exynos5440-ahci"/

OK.
> 
>>  - interrupts        : <interrupt mapping for SATA IRQ>
>>  - reg               : <registers mapping>
>>  
>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
>> index 1145637..94484cb 100644
>> --- a/drivers/ata/ahci.h
>> +++ b/drivers/ata/ahci.h
>> @@ -37,6 +37,7 @@
>>  
>>  #include <linux/clk.h>
>>  #include <linux/libata.h>
>> +#include <linux/phy/phy.h>
>>  
>>  /* Enclosure Management Control */
>>  #define EM_CTRL_MSG_TYPE              0x000f0000
>> @@ -322,6 +323,7 @@ struct ahci_host_priv {
>>  	u32			em_buf_sz;	/* EM buffer size in byte */
>>  	u32			em_msg_type;	/* EM message type */
>>  	struct clk		*clk;		/* Only for platforms supporting clk */
>> +	struct phy		*phy;		/* If platforms use phy */
> 
> minor nit:
> s/If platforms use phy/If platform uses PHY/

Right.

> 
>>  	void			*plat_data;	/* Other platform data */
>>  };
>>  
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> index 2daaee0..5a0f1418 100644
>> --- a/drivers/ata/ahci_platform.c
>> +++ b/drivers/ata/ahci_platform.c
>> @@ -141,6 +141,21 @@ static int ahci_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> +	hpriv->phy = devm_phy_get(dev, "sata-phy");
>> +	if (IS_ERR(hpriv->phy)) {
>> +		dev_dbg(dev, "can't get sata-phy\n");
>> +		/* return only if -EPROBE_DEFER */
>> +		if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
>> +			rc = -EPROBE_DEFER;
>> +			goto disable_unprepare_clk;
>> +		}
>> +	}
>> +
>> +	if (!IS_ERR(hpriv->phy)) {
>> +		phy_init(hpriv->phy);
>> +		phy_power_on(hpriv->phy);
>> +	}
>> +
>>  	/*
>>  	 * Some platforms might need to prepare for mmio region access,
>>  	 * which could be done in the following init call. So, the mmio
>> @@ -150,7 +165,7 @@ static int ahci_probe(struct platform_device *pdev)
>>  	if (pdata && pdata->init) {
>>  		rc = pdata->init(dev, hpriv->mmio);
>>  		if (rc)
>> -			goto disable_unprepare_clk;
>> +			goto disable_phy;
>>  	}
>>  
>>  	ahci_save_initial_config(dev, hpriv,
>> @@ -220,6 +235,12 @@ static int ahci_probe(struct platform_device *pdev)
>>  pdata_exit:
>>  	if (pdata && pdata->exit)
>>  		pdata->exit(dev);
>> +disable_phy:
>> +	if (!IS_ERR(hpriv->phy)) {
>> +		phy_power_off(hpriv->phy);
>> +		phy_exit(hpriv->phy);
>> +	}
>> +
>>  disable_unprepare_clk:
>>  	if (!IS_ERR(hpriv->clk))
>>  		clk_disable_unprepare(hpriv->clk);
>> @@ -238,6 +259,11 @@ static void ahci_host_stop(struct ata_host *host)
>>  	if (pdata && pdata->exit)
>>  		pdata->exit(dev);
>>  
>> +	if (!IS_ERR(hpriv->phy)) {
>> +		phy_power_off(hpriv->phy);
>> +		phy_exit(hpriv->phy);
>> +	}
>> +
>>  	if (!IS_ERR(hpriv->clk)) {
>>  		clk_disable_unprepare(hpriv->clk);
>>  		clk_put(hpriv->clk);
>> @@ -328,6 +354,7 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_suspend, ahci_resume);
> 
> Shouldn't phy_power_off/on also be used in ahci_suspend() and ahci_resume()?

I think so. Maybe even phy_exit and phy_init?

> 
>>  static const struct of_device_id ahci_of_match[] = {
>>  	{ .compatible = "snps,spear-ahci", },
>>  	{ .compatible = "snps,exynos5440-ahci", },
>> +	{ .compatible = "snps,dwc-ahci", },
> 
> This change together with ahci-platform.txt one should be in a separate patch.

Agreed.

> 
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, ahci_of_match);

cheers,
-roger

[PATCH v2 2/2] ata: ahci_platform: runtime resume the device before use

[Roger Quadros]

On OMAP platforms the device needs to be runtime resumed before
it can be accessed. The OMAP HWMOD framework takes care of
enabling the module and its resources based on the
device's runtime PM state.

In this patch we runtime resume during .probe() and runtime suspend
during .remove() (i.e. ahci_host_stop()).

We also update the runtime PM state during .resume().

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Balaji T K <balajitk@ti.com>
---
 drivers/ata/ahci_platform.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 5a0f1418..0da3b95 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
+#include <linux/pm_runtime.h>
 #include "ahci.h"
 
 static void ahci_host_stop(struct ata_host *host);
@@ -141,6 +142,9 @@ static int ahci_probe(struct platform_device *pdev)
 		}
 	}
 
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
 	hpriv->phy = devm_phy_get(dev, "sata-phy");
 	if (IS_ERR(hpriv->phy)) {
 		dev_dbg(dev, "can't get sata-phy\n");
@@ -268,6 +272,9 @@ static void ahci_host_stop(struct ata_host *host)
 		clk_disable_unprepare(hpriv->clk);
 		clk_put(hpriv->clk);
 	}
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -339,6 +346,11 @@ static int ahci_resume(struct device *dev)
 
 	ata_host_resume(host);
 
+	/* We resumed so update PM runtime state */
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	return 0;
 
 disable_unprepare_clk:
-- 
1.7.9.5

Re: [PATCH v2 2/2] ata: ahci_platform: runtime resume the device before use

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, October 16, 2013 02:42:53 PM Roger Quadros wrote:
> On OMAP platforms the device needs to be runtime resumed before
> it can be accessed. The OMAP HWMOD framework takes care of
> enabling the module and its resources based on the
> device's runtime PM state.
> 
> In this patch we runtime resume during .probe() and runtime suspend
> during .remove() (i.e. ahci_host_stop()).
> 
> We also update the runtime PM state during .resume().
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Balaji T K <balajitk@ti.com>
> ---
>  drivers/ata/ahci_platform.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 5a0f1418..0da3b95 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -23,6 +23,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/libata.h>
>  #include <linux/ahci_platform.h>
> +#include <linux/pm_runtime.h>
>  #include "ahci.h"
>  
>  static void ahci_host_stop(struct ata_host *host);
> @@ -141,6 +142,9 @@ static int ahci_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +
>  	hpriv->phy = devm_phy_get(dev, "sata-phy");
>  	if (IS_ERR(hpriv->phy)) {
>  		dev_dbg(dev, "can't get sata-phy\n");
> @@ -268,6 +272,9 @@ static void ahci_host_stop(struct ata_host *host)
>  		clk_disable_unprepare(hpriv->clk);
>  		clk_put(hpriv->clk);
>  	}
> +
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);

It would be better to add proper .remove callback (i.e. named
ahci_remove_one) and put this code there so it matches .probe
callback.

>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -339,6 +346,11 @@ static int ahci_resume(struct device *dev)
>  
>  	ata_host_resume(host);
>  
> +	/* We resumed so update PM runtime state */
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
>  	return 0;
>  
>  disable_unprepare_clk:

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v2 2/2] ata: ahci_platform: runtime resume the device before use

[Roger Quadros]

Hi,

On 10/17/2013 05:15 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, October 16, 2013 02:42:53 PM Roger Quadros wrote:
>> On OMAP platforms the device needs to be runtime resumed before
>> it can be accessed. The OMAP HWMOD framework takes care of
>> enabling the module and its resources based on the
>> device's runtime PM state.
>>
>> In this patch we runtime resume during .probe() and runtime suspend
>> during .remove() (i.e. ahci_host_stop()).
>>
>> We also update the runtime PM state during .resume().
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Balaji T K <balajitk@ti.com>
>> ---
>>  drivers/ata/ahci_platform.c |   12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> index 5a0f1418..0da3b95 100644
>> --- a/drivers/ata/ahci_platform.c
>> +++ b/drivers/ata/ahci_platform.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/libata.h>
>>  #include <linux/ahci_platform.h>
>> +#include <linux/pm_runtime.h>
>>  #include "ahci.h"
>>  
>>  static void ahci_host_stop(struct ata_host *host);
>> @@ -141,6 +142,9 @@ static int ahci_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> +	pm_runtime_enable(dev);
>> +	pm_runtime_get_sync(dev);
>> +
>>  	hpriv->phy = devm_phy_get(dev, "sata-phy");
>>  	if (IS_ERR(hpriv->phy)) {
>>  		dev_dbg(dev, "can't get sata-phy\n");
>> @@ -268,6 +272,9 @@ static void ahci_host_stop(struct ata_host *host)
>>  		clk_disable_unprepare(hpriv->clk);
>>  		clk_put(hpriv->clk);
>>  	}
>> +
>> +	pm_runtime_put_sync(dev);
>> +	pm_runtime_disable(dev);
> 
> It would be better to add proper .remove callback (i.e. named
> ahci_remove_one) and put this code there so it matches .probe
> callback.

After ahci host is started (i.e. probe succeeded), ahci_host_stop() is called
when the device resources are freed via devres management. So, we don't really
need to do this again in the .remove callback.

If we want to do all that in .remove then everything that is done in ahci_host_stop()
must be done in .remove() and ahci_host_stop() becomes a nop.

cheers,
-roger

> 
>>  }
>>  
>>  #ifdef CONFIG_PM_SLEEP
>> @@ -339,6 +346,11 @@ static int ahci_resume(struct device *dev)
>>  
>>  	ata_host_resume(host);
>>  
>> +	/* We resumed so update PM runtime state */
>> +	pm_runtime_disable(dev);
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +
>>  	return 0;
>>  
>>  disable_unprepare_clk:
> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>