Patchwork [RFC,08/15] ata: ahci_platform: Manage SATA PHY

login
register
mail settings
Submitter Roger Quadros
Date Sept. 19, 2013, 1:05 p.m.
Message ID <1379595943-14622-9-git-send-email-rogerq@ti.com>
Download mbox | patch
Permalink /patch/275952/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Roger Quadros - Sept. 19, 2013, 1:05 p.m.
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      |    2 +-
 drivers/ata/Kconfig                                |    1 +
 drivers/ata/ahci.h                                 |    2 +
 drivers/ata/ahci_platform.c                        |   31 +++++++++++++++++++-
 4 files changed, 34 insertions(+), 2 deletions(-)
Tejun Heo - Sept. 22, 2013, 4:58 p.m.
On Thu, Sep 19, 2013 at 04:05:36PM +0300, 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>

Looks okay to me although I don't know whether everyone using
ahci_platform would be happy with requiring phy.  Sergei, does this
look good to you?

Thanks.
Sergei Shtylyov - Sept. 22, 2013, 6:22 p.m.
Hello.

On 09/19/2013 05:05 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>
[...]

> 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>

struct phy;

should suffice.

> @@ -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..f812ffa 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/phy/phy.h>

    Why include it from both ahci.h and here?

>   #include "ahci.h"
>
>   static void ahci_host_stop(struct ata_host *host);
> @@ -141,16 +142,32 @@ static int ahci_probe(struct platform_device *pdev)
>   		}
>   	}
>
> +	hpriv->phy = devm_phy_get(dev, "sata-phy");
> +	if (IS_ERR(hpriv->phy)) {
> +		dev_err(dev, "can't get phy\n");

    Don't think it's a good idea to complain about missing PHY when the driver 
doesn't even use it.

> +		/* return only if -EPROBE_DEFER */
> +		if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
> +			rc = -EPROBE_DEFER;
> +			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
>   	 * region shouldn't be accessed before init (if provided) has
>   	 * returned successfully.
>   	 */
> +
> +	if (!(IS_ERR(hpriv->phy))) {

    () not needed around IS_ERR() invocation.

> +		phy_init(hpriv->phy);
> +		phy_power_on(hpriv->phy);
> +	}
> +

    I think this is misplaced, i.e. it should precede the comment.

>   	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,
[...]
> @@ -328,6 +356,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", },

    Looks like the binding documentation is incomplete -- it doesn't list 
"snps,exynos5440-ahci"...

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Sept. 22, 2013, 6:24 p.m.
Hello.

On 09/22/2013 08:58 PM, Tejun Heo 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>

> Looks okay to me although I don't know whether everyone using
> ahci_platform would be happy with requiring phy.  Sergei, does this
> look good to you?

    Not sure why you asked -- I'm not using this driver, neither I'm the 
author of it (former MV's Anton Vorontsov is IIRC). I've commented on the 
patch anyway though...

> Thanks.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 22, 2013, 9:48 p.m.
Hello,

On Sun, Sep 22, 2013 at 10:22:31PM +0400, Sergei Shtylyov wrote:
> >@@ -37,6 +37,7 @@
> >
> >  #include <linux/clk.h>
> >  #include <linux/libata.h>
> >+#include <linux/phy/phy.h>
> 
> struct phy;
> 
> should suffice.

Unless it's actually likely to cause inclusion loop, I think it's
better to include the header than adding explicit declarations.

Thanks.
Tejun Heo - Sept. 22, 2013, 9:51 p.m.
Hello,

On Sun, Sep 22, 2013 at 10:24:36PM +0400, Sergei Shtylyov wrote:
>    Not sure why you asked -- I'm not using this driver, neither I'm

Well, you have better grip of what's going on in the embedded world
than me.  I'm mostly curious whether adding dependency on PHY is okay.

Thanks.
Roger Quadros - Sept. 23, 2013, 7:37 a.m.
Hi Tejun,

On 09/23/2013 12:51 AM, Tejun Heo wrote:
> Hello,
> 
> On Sun, Sep 22, 2013 at 10:24:36PM +0400, Sergei Shtylyov wrote:
>>    Not sure why you asked -- I'm not using this driver, neither I'm
> 
> Well, you have better grip of what's going on in the embedded world
> than me.  I'm mostly curious whether adding dependency on PHY is okay.
> 

There is no hard dependency on presence of PHY. The driver will continue
as usual if devm_phy_get() fails.
I hope selecting GENERIC_PHY in Kconfig is not an issue.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros - Sept. 23, 2013, 7:42 a.m.
Hi,

On 09/22/2013 09:22 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/19/2013 05:05 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>
> [...]
> 
>> 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>
> 
> struct phy;
> 
> should suffice.
> 
>> @@ -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..f812ffa 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/phy/phy.h>
> 
>    Why include it from both ahci.h and here?
> 
OK. will move it to just .c file.

>>   #include "ahci.h"
>>
>>   static void ahci_host_stop(struct ata_host *host);
>> @@ -141,16 +142,32 @@ static int ahci_probe(struct platform_device *pdev)
>>           }
>>       }
>>
>> +    hpriv->phy = devm_phy_get(dev, "sata-phy");
>> +    if (IS_ERR(hpriv->phy)) {
>> +        dev_err(dev, "can't get phy\n");
> 
>    Don't think it's a good idea to complain about missing PHY when the driver doesn't even use it.

OK. will change it to dev_dbg() instead.
> 
>> +        /* return only if -EPROBE_DEFER */
>> +        if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
>> +            rc = -EPROBE_DEFER;
>> +            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
>>        * region shouldn't be accessed before init (if provided) has
>>        * returned successfully.
>>        */
>> +
>> +    if (!(IS_ERR(hpriv->phy))) {
> 
>    () not needed around IS_ERR() invocation.

OK.

> 
>> +        phy_init(hpriv->phy);
>> +        phy_power_on(hpriv->phy);
>> +    }
>> +
> 
>    I think this is misplaced, i.e. it should precede the comment.
> 

OK.

>>       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,
> [...]
>> @@ -328,6 +356,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", },
> 
>    Looks like the binding documentation is incomplete -- it doesn't list "snps,exynos5440-ahci"...

OK, I'll update it. Thanks for review.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Sept. 23, 2013, 12:53 p.m.
Hello.

On 23-09-2013 1:51, Tejun Heo wrote:

>>     Not sure why you asked -- I'm not using this driver, neither I'm

> Well, you have better grip of what's going on in the embedded world
> than me.  I'm mostly curious whether adding dependency on PHY is okay.

    This driver already supports optional clock, the optional PHY support
seems analogous.

> Thanks.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Sept. 23, 2013, 12:59 p.m.
Hello.

On 23-09-2013 11:37, Roger Quadros wrote:

>>>     Not sure why you asked -- I'm not using this driver, neither I'm

>> Well, you have better grip of what's going on in the embedded world
>> than me.  I'm mostly curious whether adding dependency on PHY is okay.

> There is no hard dependency on presence of PHY. The driver will continue
> as usual if devm_phy_get() fails.
> I hope selecting GENERIC_PHY in Kconfig is not an issue.

    Selecting in the AHCI_PLATFORM section? It seems I have overlooked it. No, 
I don't think it's a good idea. The generic PHY functions seem to be stubbed 
when GENERIC_PHY=n.

> cheers,
> -roger

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros - Sept. 23, 2013, 1:59 p.m.
On 09/23/2013 03:59 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 23-09-2013 11:37, Roger Quadros wrote:
> 
>>>>     Not sure why you asked -- I'm not using this driver, neither I'm
> 
>>> Well, you have better grip of what's going on in the embedded world
>>> than me.  I'm mostly curious whether adding dependency on PHY is okay.
> 
>> There is no hard dependency on presence of PHY. The driver will continue
>> as usual if devm_phy_get() fails.
>> I hope selecting GENERIC_PHY in Kconfig is not an issue.
> 
>    Selecting in the AHCI_PLATFORM section? It seems I have overlooked it. No, I don't think it's a good idea. The generic PHY functions seem to be stubbed when GENERIC_PHY=n.
> 
OK I will remove the select then.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Sept. 23, 2013, 2:10 p.m.
Hello.

On 09/23/2013 01:48 AM, Tejun Heo wrote:

>>> @@ -37,6 +37,7 @@

>>>   #include <linux/clk.h>
>>>   #include <linux/libata.h>
>>> +#include <linux/phy/phy.h>

>> struct phy;

>> should suffice.

> Unless it's actually likely to cause inclusion loop, I think it's
> better to include the header than adding explicit declarations.

    Apparently, tastes differ here. E.g. Greg KH would have also told Roger to 
use forward declaration in such case. :-)

> Thanks.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 23, 2013, 2:12 p.m.
On Mon, Sep 23, 2013 at 06:10:30PM +0400, Sergei Shtylyov wrote:
> >Unless it's actually likely to cause inclusion loop, I think it's
> >better to include the header than adding explicit declarations.
> 
>    Apparently, tastes differ here. E.g. Greg KH would have also told
> Roger to use forward declaration in such case. :-)

Yes, it's a matter of taste, but, of course mine is better than
Greg's. :P
Bartlomiej Zolnierkiewicz - Sept. 25, 2013, 12:16 p.m.
Hi,

On Monday, September 23, 2013 04:53:52 PM Sergei Shtylyov wrote:
> Hello.
> 
> On 23-09-2013 1:51, Tejun Heo wrote:
> 
> >>     Not sure why you asked -- I'm not using this driver, neither I'm
> 
> > Well, you have better grip of what's going on in the embedded world
> > than me.  I'm mostly curious whether adding dependency on PHY is okay.
> 
>     This driver already supports optional clock, the optional PHY support
> seems analogous.

Right, this reminds me that PHY support should probably also be added to
ahci_suspend() and ahci_resume().

Also please note the generic PHY framework is not yet merged in Linus'
tree so this patch should probably be merged only after the generic PHY
framework is in.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros - Feb. 7, 2014, 10:33 a.m.
Hi,

On 09/23/2013 04:59 PM, Roger Quadros wrote:
> On 09/23/2013 03:59 PM, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 23-09-2013 11:37, Roger Quadros wrote:
>>
>>>>>     Not sure why you asked -- I'm not using this driver, neither I'm
>>
>>>> Well, you have better grip of what's going on in the embedded world
>>>> than me.  I'm mostly curious whether adding dependency on PHY is okay.
>>
>>> There is no hard dependency on presence of PHY. The driver will continue
>>> as usual if devm_phy_get() fails.
>>> I hope selecting GENERIC_PHY in Kconfig is not an issue.
>>
>>    Selecting in the AHCI_PLATFORM section? It seems I have overlooked it. No, I don't think it's a good idea. The generic PHY functions seem to be stubbed when GENERIC_PHY=n.
>>
> OK I will remove the select then.

If I remove the select then build fails like so if we set CONFIG_GENERIC_PHY to 'm' and CONFIG_SATA_AHCI_PLATFORM to 'y'

drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x162647): undefined reference to `phy_init'
drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x16267c): undefined reference to `phy_power_on'
drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x162694): undefined reference to `phy_exit'
drivers/built-in.o: In function `ahci_platform_disable_resources':
(.text+0x1626af): undefined reference to `phy_power_off'
drivers/built-in.o: In function `ahci_platform_disable_resources':
(.text+0x1626b7): undefined reference to `phy_exit'
drivers/built-in.o: In function `ahci_platform_get_resources':
(.text+0x162768): undefined reference to `devm_phy_get'
make: *** [vmlinux] Error 1

This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
select it.

OR

Generic PHY layer must be fixed so that the API's are always built in.

What is the better option? I believe making the PHY API's always built in is the better option.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann - Feb. 7, 2014, 10:39 a.m.
On Friday 07 February 2014 12:33:38 Roger Quadros wrote:
> 
> This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
> select it.
> 
> OR
> 
> Generic PHY layer must be fixed so that the API's are always built in.
> 
> What is the better option? I believe making the PHY API's always built in is the better option.
> 

CONFIG_SATA_AHCI_PLATFORM should do

	"depends on CONFIG_GENERIC_PHY || !CONFIG_GENERIC_PHY"

which is the Kconfig way of saying that if CONFIG_GENERIC_PHY is a module,
CONFIG_SATA_AHCI_PLATFORM needs to be a module as well.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros - Feb. 7, 2014, 10:44 a.m.
On 02/07/2014 12:39 PM, Arnd Bergmann wrote:
> On Friday 07 February 2014 12:33:38 Roger Quadros wrote:
>>
>> This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
>> select it.
>>
>> OR
>>
>> Generic PHY layer must be fixed so that the API's are always built in.
>>
>> What is the better option? I believe making the PHY API's always built in is the better option.
>>
> 
> CONFIG_SATA_AHCI_PLATFORM should do
> 
> 	"depends on CONFIG_GENERIC_PHY || !CONFIG_GENERIC_PHY"
> 
> which is the Kconfig way of saying that if CONFIG_GENERIC_PHY is a module,
> CONFIG_SATA_AHCI_PLATFORM needs to be a module as well.
> 

Ah, that's neat. Thanks :).

cheers,
-roger

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 89de156..c6c549a 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -4,7 +4,7 @@  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" or "snps,dwc-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 4e73772..a53ef27 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -91,6 +91,7 @@  config SATA_AHCI
 
 config SATA_AHCI_PLATFORM
 	tristate "Platform AHCI SATA support"
+	select GENERIC_PHY
 	help
 	  This option enables support for Platform AHCI Serial ATA
 	  controllers.
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..f812ffa 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/phy/phy.h>
 #include "ahci.h"
 
 static void ahci_host_stop(struct ata_host *host);
@@ -141,16 +142,32 @@  static int ahci_probe(struct platform_device *pdev)
 		}
 	}
 
+	hpriv->phy = devm_phy_get(dev, "sata-phy");
+	if (IS_ERR(hpriv->phy)) {
+		dev_err(dev, "can't get phy\n");
+		/* return only if -EPROBE_DEFER */
+		if (PTR_ERR(hpriv->phy) == -EPROBE_DEFER) {
+			rc = -EPROBE_DEFER;
+			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
 	 * region shouldn't be accessed before init (if provided) has
 	 * returned successfully.
 	 */
+
+	if (!(IS_ERR(hpriv->phy))) {
+		phy_init(hpriv->phy);
+		phy_power_on(hpriv->phy);
+	}
+
 	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 +237,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 +261,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 +356,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);