[PATCH v4 0/3] ata: ahci_platform: Add PHY support and OMAP support

[PATCH v4 0/3] 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 in PATCH 2.

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

PATCH 1 just updates the compatible list in the DT binding document.

Changelog:
v4:
- Tackle error conditions while using PHY framework APIs.

v3:
- Addressed review comments and rebased on 3.13-rc7
- DT binding update moved to new patch.

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

cheers,
-roger

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

Roger Quadros (2):
  ata: ahci_platform: Update DT compatible list
  ata: ahci_platform: runtime resume the device before use

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

-- 
1.8.3.2

[PATCH v4 1/3] ata: ahci_platform: Update DT compatible list

[Roger Quadros]

The ahci_platform driver supports "snps,exynos5440-ahci" and
"ibm,476gtr-ahci". Add this to the DT binding information.

Also add compatible string "snps,dwc-ahci", which should be used
for Synopsis Designware SATA cores. e.g. on TI OMAP5 and DRA7 platforms.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 4 +++-
 drivers/ata/ahci_platform.c                             | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 89de156..08ba647 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -4,7 +4,9 @@ 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", "ibm,476gtr-ahci",
+		      or "snps,dwc-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 4b231ba..d5ced13 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -330,6 +330,7 @@ static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "snps,spear-ahci", },
 	{ .compatible = "snps,exynos5440-ahci", },
 	{ .compatible = "ibm,476gtr-ahci", },
+	{ .compatible = "snps,dwc-ahci", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ahci_of_match);
-- 
1.8.3.2

[PATCH v4 2/3] 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>
---
 drivers/ata/ahci.h          |  2 ++
 drivers/ata/ahci_platform.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 2289efd..f9bbada 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 platform uses phy */
 	void			*plat_data;	/* Other platform data */
 };
 
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index d5ced13..b476642 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -142,6 +142,39 @@ static int ahci_probe(struct platform_device *pdev)
 		}
 	}
 
+	hpriv->phy = devm_phy_get(dev, "sata-phy");
+	if (IS_ERR(hpriv->phy)) {
+		rc = PTR_ERR(hpriv->phy);
+		switch (rc) {
+		case -ENODEV:
+		case -ENOSYS:
+			/* continue normally */
+			hpriv->phy = NULL;
+			break;
+
+		case -EPROBE_DEFER:
+			goto disable_unprepare_clk;
+			break;
+
+		default:
+			dev_err(dev, "couldn't get sata-phy\n");
+			goto disable_unprepare_clk;
+			break;
+		}
+	}
+
+	if (hpriv->phy) {
+		rc = phy_init(hpriv->phy);
+		if (rc)
+			goto disable_unprepare_clk;
+
+		rc = phy_power_on(hpriv->phy);
+		if (rc) {
+			phy_exit(hpriv->phy);
+			goto disable_unprepare_clk;
+		}
+	}
+
 	/*
 	 * Some platforms might need to prepare for mmio region access,
 	 * which could be done in the following init call. So, the mmio
@@ -151,7 +184,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,
@@ -221,6 +254,12 @@ static int ahci_probe(struct platform_device *pdev)
 pdata_exit:
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
+disable_phy:
+	if (hpriv->phy) {
+		phy_power_off(hpriv->phy);
+		phy_exit(hpriv->phy);
+	}
+
 disable_unprepare_clk:
 	if (!IS_ERR(hpriv->clk))
 		clk_disable_unprepare(hpriv->clk);
@@ -239,6 +278,11 @@ static void ahci_host_stop(struct ata_host *host)
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
 
+	if (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);
-- 
1.8.3.2

[PATCH v4 3/3] 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 b476642..8a70e5b 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);
@@ -142,6 +143,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)) {
 		rc = PTR_ERR(hpriv->phy);
@@ -287,6 +291,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
@@ -358,6 +365,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.8.3.2

Re: [PATCH v4 0/3] ata: ahci_platform: Add PHY support and OMAP support

[Arnd Bergmann]

On Thursday 09 January 2014, Roger Quadros wrote:
> 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 in PATCH 2.
> 
> In order to support SATA on the OMAP platforms we need to runtime
> resume the device before use. PATCH 3 takes care of that.
> 
> PATCH 1 just updates the compatible list in the DT binding document.
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v4 2/3] ata: ahci_platform: Manage SATA PHY

[Sergei Shtylyov]

On 09-01-2014 14:56, 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>
[...]

> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index d5ced13..b476642 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -142,6 +142,39 @@ static int ahci_probe(struct platform_device *pdev)
>   		}
>   	}
>
> +	hpriv->phy = devm_phy_get(dev, "sata-phy");
> +	if (IS_ERR(hpriv->phy)) {
> +		rc = PTR_ERR(hpriv->phy);
> +		switch (rc) {
> +		case -ENODEV:
> +		case -ENOSYS:
> +			/* continue normally */
> +			hpriv->phy = NULL;
> +			break;
> +
> +		case -EPROBE_DEFER:
> +			goto disable_unprepare_clk;
> +			break;

    Well, well, *break* after *goto*? :-)

> +
> +		default:
> +			dev_err(dev, "couldn't get sata-phy\n");
> +			goto disable_unprepare_clk;
> +			break;

    Again?

WBR, Sergei

Re: [PATCH v4 0/3] ata: ahci_platform: Add PHY support and OMAP support

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday, January 09, 2014 04:25:59 PM Roger Quadros wrote:
> 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 in PATCH 2.
> 
> In order to support SATA on the OMAP platforms we need to runtime
> resume the device before use. PATCH 3 takes care of that.
> 
> PATCH 1 just updates the compatible list in the DT binding document.
> 
> Changelog:
> v4:
> - Tackle error conditions while using PHY framework APIs.
> 
> v3:
> - Addressed review comments and rebased on 3.13-rc7
> - DT binding update moved to new patch.
> 
> v2:
> - got rid of Texas Instruments SATA wrapper driver
> - adressed review comments

Thanks for fixing it.

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

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

Re: [PATCH v4 2/3] ata: ahci_platform: Manage SATA PHY

[Roger Quadros]

On 01/09/2014 04:50 PM, Sergei Shtylyov wrote:
> On 09-01-2014 14:56, 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>
> [...]
> 
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> index d5ced13..b476642 100644
>> --- a/drivers/ata/ahci_platform.c
>> +++ b/drivers/ata/ahci_platform.c
>> @@ -142,6 +142,39 @@ static int ahci_probe(struct platform_device *pdev)
>>           }
>>       }
>>
>> +    hpriv->phy = devm_phy_get(dev, "sata-phy");
>> +    if (IS_ERR(hpriv->phy)) {
>> +        rc = PTR_ERR(hpriv->phy);
>> +        switch (rc) {
>> +        case -ENODEV:
>> +        case -ENOSYS:
>> +            /* continue normally */
>> +            hpriv->phy = NULL;
>> +            break;
>> +
>> +        case -EPROBE_DEFER:
>> +            goto disable_unprepare_clk;
>> +            break;
> 
>    Well, well, *break* after *goto*? :-)
> 
I know it is pointless functionally, but it is just my preference
aesthetically.

Is it much of an issue if left as is?

cheers,
-roger

Re: [PATCH v4 2/3] ata: ahci_platform: Manage SATA PHY

[Tejun Heo]

On Thu, Jan 09, 2014 at 05:07:29PM +0530, Roger Quadros wrote:
> I know it is pointless functionally, but it is just my preference
> aesthetically.
> 
> Is it much of an issue if left as is?

Yes.

-- 
tejun

Re: [PATCH v4 2/3] ata: ahci_platform: Manage SATA PHY

[Kishon Vijay Abraham I]

On Thursday 09 January 2014 04:26 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>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/ata/ahci.h          |  2 ++
>  drivers/ata/ahci_platform.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 2289efd..f9bbada 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 platform uses phy */
>  	void			*plat_data;	/* Other platform data */
>  };
>  
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index d5ced13..b476642 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -142,6 +142,39 @@ static int ahci_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	hpriv->phy = devm_phy_get(dev, "sata-phy");
> +	if (IS_ERR(hpriv->phy)) {
> +		rc = PTR_ERR(hpriv->phy);
> +		switch (rc) {
> +		case -ENODEV:
> +		case -ENOSYS:
> +			/* continue normally */
> +			hpriv->phy = NULL;
> +			break;
> +
> +		case -EPROBE_DEFER:
> +			goto disable_unprepare_clk;
> +			break;
> +
> +		default:
> +			dev_err(dev, "couldn't get sata-phy\n");
> +			goto disable_unprepare_clk;
> +			break;
> +		}
> +	}
> +
> +	if (hpriv->phy) {
> +		rc = phy_init(hpriv->phy);
> +		if (rc)
> +			goto disable_unprepare_clk;
> +
> +		rc = phy_power_on(hpriv->phy);
> +		if (rc) {
> +			phy_exit(hpriv->phy);
> +			goto disable_unprepare_clk;
> +		}
> +	}
> +
>  	/*
>  	 * Some platforms might need to prepare for mmio region access,
>  	 * which could be done in the following init call. So, the mmio
> @@ -151,7 +184,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,
> @@ -221,6 +254,12 @@ static int ahci_probe(struct platform_device *pdev)
>  pdata_exit:
>  	if (pdata && pdata->exit)
>  		pdata->exit(dev);
> +disable_phy:
> +	if (hpriv->phy) {
> +		phy_power_off(hpriv->phy);
> +		phy_exit(hpriv->phy);
> +	}
> +
>  disable_unprepare_clk:
>  	if (!IS_ERR(hpriv->clk))
>  		clk_disable_unprepare(hpriv->clk);
> @@ -239,6 +278,11 @@ static void ahci_host_stop(struct ata_host *host)
>  	if (pdata && pdata->exit)
>  		pdata->exit(dev);
>  
> +	if (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);
> 

Re: [PATCH v4 2/3] ata: ahci_platform: Manage SATA PHY

[Roger Quadros]

On 01/09/2014 05:00 PM, Tejun Heo wrote:
> On Thu, Jan 09, 2014 at 05:07:29PM +0530, Roger Quadros wrote:
>> I know it is pointless functionally, but it is just my preference
>> aesthetically.
>>
>> Is it much of an issue if left as is?
> 
> Yes.
> 
OK. I'll send a v5.

cheers,
-roger