diff mbox

[v2,2/4] ARM: sunxi: Add ahci-sunxi driver for the Allwinner SUNXi SoCs sata

Message ID 1388826878-5602-3-git-send-email-hdegoede@redhat.com
State Superseded, archived
Headers show

Commit Message

Hans de Goede Jan. 4, 2014, 9:14 a.m. UTC
From: Oliver Schinagl <oliver@schinagl.nl>

This patch adds support for the ahci sata controler found on Allwinner A10
and A20 SoCs.

Orignally written by Olliver Schinagl using the approach of having a platform
device which probe method creates a new child platform device which gets
driven by ahci_platform.c, as done by ahci_imx.c .

Given that almost all functionality already is shared through libahci /
ata-core, and that ahci_platform.c cannot cleanly handle somewhat more complex
platform specific ahci cases, such as the sunxi case, it was refactored into
a stand-alone platform driver by Hans de Goede.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
 drivers/ata/Kconfig                                |   9 +
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci_sunxi.c                           | 349 +++++++++++++++++++++
 4 files changed, 383 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
 create mode 100644 drivers/ata/ahci_sunxi.c

Comments

Arnd Bergmann Jan. 4, 2014, 9:39 p.m. UTC | #1
On Saturday 04 January 2014 10:14:36 Hans de Goede wrote:
> diff --git a/Documentation/devicetree/bindings/ata/ahci-sunxi.txt b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
> new file mode 100644
> index 0000000..0792fa5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
> @@ -0,0 +1,24 @@
> +Allwinner SUNXI AHCI SATA Controller
> +
> +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 "allwinner,sun4i-a10-ahci"
> +- reg		: <registers mapping>
> +- interrupts	: <interrupt mapping for AHCI IRQ>
> +- clocks	: clocks for ACHI
> +- clock-names	: clock names for AHCI

The binding needs to specify the required names for the clocks.

> +Optional properties:
> +- pwr-supply	: regulator to control the power supply GPIO
> +
> +Example:
> +	ahci@01c18000 {
> +		compatible = "allwinner,sun4i-a10-ahci";
> +		reg = <0x01c18000 0x1000>;
> +		interrupts = <0 56 1>;
> +		clocks = <&ahb_gates 25>, <&pll6 0>;
> +		clock-names = "ahb_sata", "pll6_sata";

"pll6_sata" doesn't sound particularly generic. The name should
reflect what the clock is used for, not what drives it.

Also, please send the binding as a separate patch with Cc to
the devicetree-discuss mailing list.

> +
> +static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
> +{
> +	u32 reg_val;
> +	int timeout;
> +
> +	/* This magic is from the original code */
> +	writel(0, reg_base + AHCI_RWCR);
> +	mdelay(5);

This function should probably be in a separate phy driver. I would
very much hope that we can minimize the required code in an AHCI
driver and move code from this new file into the ahci-platform
driver. The clock, regulator and phy setup can all be optional
properties of the generic driver, and then there shouldn't
be much left that is sunxi specific.

> +static int sunxi_ahci_susp(struct device *dev)
> +{
> +	struct ata_host *host = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = host->private_data;
> +	struct sunxi_ahci *ahci = hpriv->plat_data;
> +	int ret;
> +
> +	/*
> +	 * AHCI spec rev1.1 section 8.3.3:
> +	 * Software must disable interrupts prior to requesting a
> +	 * transition of the HBA to D3 state.
> +	 */
> +	sunxi_clrbits(hpriv->mmio + HOST_CTL, HOST_IRQ_EN);
> +
> +	ret = ata_host_suspend(host, PMSG_SUSPEND);
> +	if (ret)
> +		return ret;
> +
> +	sunxi_ahci_disable_clks(ahci);
> +
> +	return 0;
> +}

The only thing in here that seems sunxi-specific is the irq disabling
part. Can't you do this instead by calling disable_irq() and make
the function completely generic?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Jan. 4, 2014, 9:47 p.m. UTC | #2
On Saturday 04 January 2014 22:39:50 Arnd Bergmann wrote:
> > +Required properties:
> > +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci"
> > +- reg                : <registers mapping>
> > +- interrupts : <interrupt mapping for AHCI IRQ>
> > +- clocks     : clocks for ACHI
> > +- clock-names        : clock names for AHCI
> 
> The binding needs to specify the required names for the clocks.

fwiw, the imx driver uses "ahb" and "sata_ref" as the clock names.
I would strongly suggest using the same names here, and documenting
them in the ahci binding as optional.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede Jan. 4, 2014, 11:44 p.m. UTC | #3
Hi,

On 01/04/2014 10:39 PM, Arnd Bergmann wrote:
> On Saturday 04 January 2014 10:14:36 Hans de Goede wrote:
>> diff --git a/Documentation/devicetree/bindings/ata/ahci-sunxi.txt b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
>> new file mode 100644
>> index 0000000..0792fa5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
>> @@ -0,0 +1,24 @@
>> +Allwinner SUNXI AHCI SATA Controller
>> +
>> +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 "allwinner,sun4i-a10-ahci"
>> +- reg		: <registers mapping>
>> +- interrupts	: <interrupt mapping for AHCI IRQ>
>> +- clocks	: clocks for ACHI
>> +- clock-names	: clock names for AHCI
>
> The binding needs to specify the required names for the clocks.

Will fix.

>
>> +Optional properties:
>> +- pwr-supply	: regulator to control the power supply GPIO
>> +
>> +Example:
>> +	ahci@01c18000 {
>> +		compatible = "allwinner,sun4i-a10-ahci";
>> +		reg = <0x01c18000 0x1000>;
>> +		interrupts = <0 56 1>;
>> +		clocks = <&ahb_gates 25>, <&pll6 0>;
>> +		clock-names = "ahb_sata", "pll6_sata";
>
> "pll6_sata" doesn't sound particularly generic. The name should
> reflect what the clock is used for, not what drives it.

Will change to ahb + sata_ref.

>
> Also, please send the binding as a separate patch with Cc to
> the devicetree-discuss mailing list.

Hmm, this contradicts what others are saying who have requested for
the binding docs to be part of the same commit as the driver (with
various other drivers). Note that I've cc-ed devicetree already.

>
>> +
>> +static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
>> +{
>> +	u32 reg_val;
>> +	int timeout;
>> +
>> +	/* This magic is from the original code */
>> +	writel(0, reg_base + AHCI_RWCR);
>> +	mdelay(5);
>
> This function should probably be in a separate phy driver.  I would
> very much hope that we can minimize the required code in an AHCI
> driver and move code from this new file into the ahci-platform
> driver. The clock, regulator and phy setup can all be optional
> properties of the generic driver, and then there shouldn't
> be much left that is sunxi specific.

Writing a phy driver, and extending ahci-platform to use that
was my original plan. But the phy really is part of the
ahci ip-block here, and not a separate ip-block. Its registers
are smack in the middle of the io-range for the ahci function.

Also note that sunxi_ahci_pre_start_engine is rather sunxi
specific. Needing to put that in a generic ahci_platform driver
and only activating it for sunxi socs would only serve to
prove my point that at some point it is simply easier and
better to write a non generic platform glue driver when dealing
with exotic ahci ip blocks.

If we end up putting all sort of if soca do foo else if socb
do bar, else do nothing magic in ahci_platform.c I think we're
over generalizing. If something nicely fits as a generic
platform dev, by all means we should use a generic platform
driver, but that just won't work cleanly here.

Actually I've just completed writing a phy driver for
the usbphy in the same soc, and a patch to extend ehci-platform
to take an optional clk and phy. So I completely agree with the
basic idea, it just does not seem doable cleanly in this case,
hence my choice to write a separate platform driver. For the
usb stuff (wip) see:
https://github.com/jwrdegoede/linux-sunxi/commit/4655dd01936f42d8a75da08a00af439e0a34eaf7
https://github.com/jwrdegoede/linux-sunxi/commit/bcb674859d015ff9e082829dbd5cf239b8b53d4a

As said this is exactly what your asking me to do for sunxi-ahci,
but the difference is that for sunxi-ehci it works nicely, and
for sunxi-ahci it just doesn't work cleanly.



>
>> +static int sunxi_ahci_susp(struct device *dev)
>> +{
>> +	struct ata_host *host = dev_get_drvdata(dev);
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +	struct sunxi_ahci *ahci = hpriv->plat_data;
>> +	int ret;
>> +
>> +	/*
>> +	 * AHCI spec rev1.1 section 8.3.3:
>> +	 * Software must disable interrupts prior to requesting a
>> +	 * transition of the HBA to D3 state.
>> +	 */
>> +	sunxi_clrbits(hpriv->mmio + HOST_CTL, HOST_IRQ_EN);
>> +
>> +	ret = ata_host_suspend(host, PMSG_SUSPEND);
>> +	if (ret)
>> +		return ret;
>> +
>> +	sunxi_ahci_disable_clks(ahci);
>> +
>> +	return 0;
>> +}
>
> The only thing in here that seems sunxi-specific is the irq disabling
> part. Can't you do this instead by calling disable_irq() and make
> the function completely generic?

The irq disabling part actually is not sunxi specific, ahci_platform.c
has it too.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Jan. 5, 2014, 11:35 a.m. UTC | #4
On Sunday 05 January 2014, Hans de Goede wrote:
> >
> > Also, please send the binding as a separate patch with Cc to
> > the devicetree-discuss mailing list.
> 
> Hmm, this contradicts what others are saying who have requested for
> the binding docs to be part of the same commit as the driver (with
> various other drivers). Note that I've cc-ed devicetree already.

Probably my fault then, I've been away for some time and may misremember
what we discussed. Maybe someone else can clarify.

> >> +static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
> >> +{
> >> +	u32 reg_val;
> >> +	int timeout;
> >> +
> >> +	/* This magic is from the original code */
> >> +	writel(0, reg_base + AHCI_RWCR);
> >> +	mdelay(5);
> >
> > This function should probably be in a separate phy driver.  I would
> > very much hope that we can minimize the required code in an AHCI
> > driver and move code from this new file into the ahci-platform
> > driver. The clock, regulator and phy setup can all be optional
> > properties of the generic driver, and then there shouldn't
> > be much left that is sunxi specific.
> 
> Writing a phy driver, and extending ahci-platform to use that
> was my original plan. But the phy really is part of the
> ahci ip-block here, and not a separate ip-block. Its registers
> are smack in the middle of the io-range for the ahci function.

I see. I wonder if the register layout is common with some other
implementation then. If it's part of the AHCI block, it's probably
not an Allwinner invention but comes from whoever implemented the
AHCI.

> Also note that sunxi_ahci_pre_start_engine is rather sunxi
> specific. Needing to put that in a generic ahci_platform driver
> and only activating it for sunxi socs would only serve to
> prove my point that at some point it is simply easier and
> better to write a non generic platform glue driver when dealing
> with exotic ahci ip blocks.
> 
> If we end up putting all sort of if soca do foo else if socb
> do bar, else do nothing magic in ahci_platform.c I think we're
> over generalizing. If something nicely fits as a generic
> platform dev, by all means we should use a generic platform
> driver, but that just won't work cleanly here.

Yes, but there may be some middle ground. I still think it would
be worthwhile to make the clock handling part of the common
ahci (or ahci-platform) driver and reuse that, since it seems
to be needed on a number of implementations. IIRC there is already
some inheritence model in libata that can be used to define
variations of drivers and have common parts done only once.

> Actually I've just completed writing a phy driver for
> the usbphy in the same soc, and a patch to extend ehci-platform
> to take an optional clk and phy. So I completely agree with the
> basic idea, it just does not seem doable cleanly in this case,
> hence my choice to write a separate platform driver. For the
> usb stuff (wip) see:
> https://github.com/jwrdegoede/linux-sunxi/commit/4655dd01936f42d8a75da08a00af439e0a34eaf7
> https://github.com/jwrdegoede/linux-sunxi/commit/bcb674859d015ff9e082829dbd5cf239b8b53d4a

Ah, very nice!

> >> +static int sunxi_ahci_susp(struct device *dev)
> >> +{
> >> +	struct ata_host *host = dev_get_drvdata(dev);
> >> +	struct ahci_host_priv *hpriv = host->private_data;
> >> +	struct sunxi_ahci *ahci = hpriv->plat_data;
> >> +	int ret;
> >> +
> >> +	/*
> >> +	 * AHCI spec rev1.1 section 8.3.3:
> >> +	 * Software must disable interrupts prior to requesting a
> >> +	 * transition of the HBA to D3 state.
> >> +	 */
> >> +	sunxi_clrbits(hpriv->mmio + HOST_CTL, HOST_IRQ_EN);
> >> +
> >> +	ret = ata_host_suspend(host, PMSG_SUSPEND);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	sunxi_ahci_disable_clks(ahci);
> >> +
> >> +	return 0;
> >> +}
> >
> > The only thing in here that seems sunxi-specific is the irq disabling
> > part. Can't you do this instead by calling disable_irq() and make
> > the function completely generic?
> 
> The irq disabling part actually is not sunxi specific, ahci_platform.c
> has it too.

Ok. Can this function be shared in some way then, e.g. by exporting the
ahci-platform implementation?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olliver Schinagl Jan. 5, 2014, 12:42 p.m. UTC | #5
On 01/04/14 22:47, Arnd Bergmann wrote:
> On Saturday 04 January 2014 22:39:50 Arnd Bergmann wrote:
>>> +Required properties:
>>> +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci"
>>> +- reg                : <registers mapping>
>>> +- interrupts : <interrupt mapping for AHCI IRQ>
>>> +- clocks     : clocks for ACHI
>>> +- clock-names        : clock names for AHCI
>> The binding needs to specify the required names for the clocks.
> fwiw, the imx driver uses "ahb" and "sata_ref" as the clock names.
> I would strongly suggest using the same names here, and documenting
> them in the ahci binding as optional.
This is my fault, and you just reminded me I should have fixed that from 
the previous comments. It just slipped my mind and I'm sorry for that!

Hans, I'll go over the original commit thread and pick up all changes 
and send them as a patch to you Monday.

Oliver
>
> 	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede Jan. 5, 2014, 1:06 p.m. UTC | #6
Hi,

On 01/05/2014 01:42 PM, Olliver Schinagl wrote:
> On 01/04/14 22:47, Arnd Bergmann wrote:
>> On Saturday 04 January 2014 22:39:50 Arnd Bergmann wrote:
>>>> +Required properties:
>>>> +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci"
>>>> +- reg                : <registers mapping>
>>>> +- interrupts : <interrupt mapping for AHCI IRQ>
>>>> +- clocks     : clocks for ACHI
>>>> +- clock-names        : clock names for AHCI
>>> The binding needs to specify the required names for the clocks.
>> fwiw, the imx driver uses "ahb" and "sata_ref" as the clock names.
>> I would strongly suggest using the same names here, and documenting
>> them in the ahci binding as optional.
> This is my fault, and you just reminded me I should have fixed that from the previous comments. It just slipped my mind and I'm sorry for that!
>
> Hans, I'll go over the original commit thread and pick up all changes and send them as a patch to you Monday.

Thanks, sounds good. Then I won't touch the code till I get your patch (way too busy with usb now anyways).

Regards,

Hans


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede Jan. 5, 2014, 1:29 p.m. UTC | #7
Hi,

On 01/05/2014 12:35 PM, Arnd Bergmann wrote:
> On Sunday 05 January 2014, Hans de Goede wrote:

<snip>

>>>> +static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
>>>> +{
>>>> +	u32 reg_val;
>>>> +	int timeout;
>>>> +
>>>> +	/* This magic is from the original code */
>>>> +	writel(0, reg_base + AHCI_RWCR);
>>>> +	mdelay(5);
>>>
>>> This function should probably be in a separate phy driver.  I would
>>> very much hope that we can minimize the required code in an AHCI
>>> driver and move code from this new file into the ahci-platform
>>> driver. The clock, regulator and phy setup can all be optional
>>> properties of the generic driver, and then there shouldn't
>>> be much left that is sunxi specific.
>>
>> Writing a phy driver, and extending ahci-platform to use that
>> was my original plan. But the phy really is part of the
>> ahci ip-block here, and not a separate ip-block. Its registers
>> are smack in the middle of the io-range for the ahci function.
>
> I see. I wonder if the register layout is common with some other
> implementation then. If it's part of the AHCI block, it's probably
> not an Allwinner invention but comes from whoever implemented the
> AHCI.

Right, but so far we've been unable to find anything quite like it.

>> Also note that sunxi_ahci_pre_start_engine is rather sunxi
>> specific. Needing to put that in a generic ahci_platform driver
>> and only activating it for sunxi socs would only serve to
>> prove my point that at some point it is simply easier and
>> better to write a non generic platform glue driver when dealing
>> with exotic ahci ip blocks.
>>
>> If we end up putting all sort of if soca do foo else if socb
>> do bar, else do nothing magic in ahci_platform.c I think we're
>> over generalizing. If something nicely fits as a generic
>> platform dev, by all means we should use a generic platform
>> driver, but that just won't work cleanly here.
>
> Yes, but there may be some middle ground. I still think it would
> be worthwhile to make the clock handling part of the common
> ahci (or ahci-platform) driver and reuse that, since it seems
> to be needed on a number of implementations. IIRC there is already
> some inheritence model in libata that can be used to define
> variations of drivers and have common parts done only once.

Ok, thinking more about this I think I have a solution which
should be acceptable. I'll do a v3 the coming days.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede Jan. 5, 2014, 1:32 p.m. UTC | #8
Hi,

On 01/05/2014 01:42 PM, Olliver Schinagl wrote:
> On 01/04/14 22:47, Arnd Bergmann wrote:
>> On Saturday 04 January 2014 22:39:50 Arnd Bergmann wrote:
>>>> +Required properties:
>>>> +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci"
>>>> +- reg                : <registers mapping>
>>>> +- interrupts : <interrupt mapping for AHCI IRQ>
>>>> +- clocks     : clocks for ACHI
>>>> +- clock-names        : clock names for AHCI
>>> The binding needs to specify the required names for the clocks.
>> fwiw, the imx driver uses "ahb" and "sata_ref" as the clock names.
>> I would strongly suggest using the same names here, and documenting
>> them in the ahci binding as optional.
> This is my fault, and you just reminded me I should have fixed that from the previous comments. It just slipped my mind and I'm sorry for that!
>
> Hans, I'll go over the original commit thread and pick up all changes and send them as a patch to you Monday.

As I just mentioned to Arnd I've a better idea to be able to re-use most of
the ahci_platform.c code without copy-pasting it. So I'm going to do a v3
soonish. Can you please just make a mail with summary of the requested
changes and send that to me, or do this after I've send v3 ?

Thanks,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olliver Schinagl Jan. 5, 2014, 2 p.m. UTC | #9
On 01/05/14 14:32, Hans de Goede wrote:
> Hi,
>
> On 01/05/2014 01:42 PM, Olliver Schinagl wrote:
>> On 01/04/14 22:47, Arnd Bergmann wrote:
>>> On Saturday 04 January 2014 22:39:50 Arnd Bergmann wrote:
>>>>> +Required properties:
>>>>> +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci"
>>>>> +- reg                : <registers mapping>
>>>>> +- interrupts : <interrupt mapping for AHCI IRQ>
>>>>> +- clocks     : clocks for ACHI
>>>>> +- clock-names        : clock names for AHCI
>>>> The binding needs to specify the required names for the clocks.
>>> fwiw, the imx driver uses "ahb" and "sata_ref" as the clock names.
>>> I would strongly suggest using the same names here, and documenting
>>> them in the ahci binding as optional.
>> This is my fault, and you just reminded me I should have fixed that from the previous comments. It just slipped my mind and I'm sorry for that!
>>
>> Hans, I'll go over the original commit thread and pick up all changes and send them as a patch to you Monday.
> As I just mentioned to Arnd I've a better idea to be able to re-use most of
> the ahci_platform.c code without copy-pasting it. So I'm going to do a v3
> soonish. Can you please just make a mail with summary of the requested
> changes and send that to me, or do this after I've send v3 ?
if memory serves, it was mostly about the dt documentation and binding 
names, i'll work on it first thing tomorrow morning ;)
>
> Thanks,
>
> Hans

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

Patch

diff --git a/Documentation/devicetree/bindings/ata/ahci-sunxi.txt b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
new file mode 100644
index 0000000..0792fa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt
@@ -0,0 +1,24 @@ 
+Allwinner SUNXI AHCI SATA Controller
+
+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 "allwinner,sun4i-a10-ahci"
+- reg		: <registers mapping>
+- interrupts	: <interrupt mapping for AHCI IRQ>
+- clocks	: clocks for ACHI
+- clock-names	: clock names for AHCI
+
+Optional properties:
+- pwr-supply	: regulator to control the power supply GPIO
+
+Example:
+	ahci@01c18000 {
+		compatible = "allwinner,sun4i-a10-ahci";
+		reg = <0x01c18000 0x1000>;
+		interrupts = <0 56 1>;
+		clocks = <&ahb_gates 25>, <&pll6 0>;
+		clock-names = "ahb_sata", "pll6_sata";
+		pwr-supply = <&reg_ahci_5v>;
+	};
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 4e73772..3c80bbe 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -106,6 +106,15 @@  config AHCI_IMX
 
 	  If unsure, say N.
 
+config AHCI_SUNXI
+	tristate "Allwinner sunxi AHCI SATA support"
+	depends on ARCH_SUNXI
+	help
+	  This option enables support for the Allwinner sunxi SoC's
+	  onboard AHCI SATA.
+
+	  If unsure, say N.
+
 config SATA_FSL
 	tristate "Freescale 3.0Gbps SATA support"
 	depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 46518c6..956abc3 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -11,6 +11,7 @@  obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
 obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
 obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o
+obj-$(CONFIG_AHCI_SUNXI)	+= ahci_sunxi.o libahci.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
new file mode 100644
index 0000000..22d3972
--- /dev/null
+++ b/drivers/ata/ahci_sunxi.c
@@ -0,0 +1,349 @@ 
+/*
+ * Allwinner sunxi AHCI SATA platform driver
+ * Copyright 2013 Olliver Schinagl <oliver@schinagl.nl>
+ * Copyright 2014 Hans de Goede <hdegoede@redhat.com>
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ * Based on code from Allwinner Technology Co., Ltd. <www.allwinnertech.com>,
+ * Daniel Wang <danielwang@allwinnertech.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include "ahci.h"
+
+#define AHCI_BISTAFR 0x00a0
+#define AHCI_BISTCR 0x00a4
+#define AHCI_BISTFCTR 0x00a8
+#define AHCI_BISTSR 0x00ac
+#define AHCI_BISTDECR 0x00b0
+#define AHCI_DIAGNR0 0x00b4
+#define AHCI_DIAGNR1 0x00b8
+#define AHCI_OOBR 0x00bc
+#define AHCI_PHYCS0R 0x00c0
+#define AHCI_PHYCS1R 0x00c4
+#define AHCI_PHYCS2R 0x00c8
+#define AHCI_TIMER1MS 0x00e0
+#define AHCI_GPARAM1R 0x00e8
+#define AHCI_GPARAM2R 0x00ec
+#define AHCI_PPARAMR 0x00f0
+#define AHCI_TESTR 0x00f4
+#define AHCI_VERSIONR 0x00f8
+#define AHCI_IDR 0x00fc
+#define AHCI_RWCR 0x00fc
+#define AHCI_P0DMACR 0x0170
+#define AHCI_P0PHYCR 0x0178
+#define AHCI_P0PHYSR 0x017c
+
+struct sunxi_ahci {
+	struct ahci_host_priv hpriv;
+	struct regulator *pwr;
+	struct clk *sata_clk;
+	struct clk *ahb_clk;
+};
+
+static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val &= ~(clr_val);
+	writel(reg_val, reg);
+}
+
+static void sunxi_setbits(void __iomem *reg, u32 set_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val |= set_val;
+	writel(reg_val, reg);
+}
+
+static void sunxi_clrsetbits(void __iomem *reg, u32 clr_val, u32 set_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val &= ~(clr_val);
+	reg_val |= set_val;
+	writel(reg_val, reg);
+}
+
+static u32 sunxi_getbits(void __iomem *reg, u8 mask, u8 shift)
+{
+	return (readl(reg) >> shift) & mask;
+}
+
+static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
+{
+	u32 reg_val;
+	int timeout;
+
+	/* This magic is from the original code */
+	writel(0, reg_base + AHCI_RWCR);
+	mdelay(5);
+
+	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(19));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
+			 (0x7 << 24),
+			 (0x5 << 24) | BIT(23) | BIT(18));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS1R,
+			 (0x3 << 16) | (0x1f << 8) | (0x3 << 6),
+			 (0x2 << 16) | (0x6 << 8) | (0x2 << 6));
+	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(28) | BIT(15));
+	sunxi_clrbits(reg_base + AHCI_PHYCS1R, BIT(19));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
+			 (0x7 << 20), (0x3 << 20));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS2R,
+			 (0x1f << 5), (0x19 << 5));
+	mdelay(5);
+
+	sunxi_setbits(reg_base + AHCI_PHYCS0R, (0x1 << 19));
+
+	timeout = 0x100000;
+	do {
+		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS0R, 0x7, 28);
+	} while (--timeout && (reg_val != 0x2));
+	if (!timeout) {
+		dev_err(dev, "PHY power up failed.\n");
+		return -EIO;
+	}
+
+	sunxi_setbits(reg_base + AHCI_PHYCS2R, (0x1 << 24));
+
+	timeout = 0x100000;
+	do {
+		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS2R, 0x1, 24);
+	} while (--timeout && reg_val);
+	if (!timeout) {
+		dev_err(dev, "PHY calibration failed.\n");
+		return -EIO;
+	}
+	mdelay(15);
+
+	writel(0x7, reg_base + AHCI_RWCR);
+
+	return 0;
+}
+
+void sunxi_ahci_pre_start_engine(struct ata_port *ap)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+
+	/* Setup DMA before DMA start */
+	sunxi_clrsetbits(hpriv->mmio + AHCI_P0DMACR, 0x0000ff00, 0x00004400);
+}
+
+static int sunxi_ahci_enable_clks(struct sunxi_ahci *ahci)
+{
+	int ret;
+
+	ret = clk_prepare_enable(ahci->sata_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(ahci->ahb_clk);
+	if (ret)
+		clk_disable_unprepare(ahci->sata_clk);
+
+	return ret;
+}
+
+static void sunxi_ahci_disable_clks(struct sunxi_ahci *ahci)
+{
+	clk_disable_unprepare(ahci->ahb_clk);
+	clk_disable_unprepare(ahci->sata_clk);
+}
+
+static void sunxi_ahci_host_stop(struct ata_host *host)
+{
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct sunxi_ahci *ahci = hpriv->plat_data;
+
+	if (!IS_ERR(ahci->pwr))
+		regulator_disable(ahci->pwr);
+
+	sunxi_ahci_disable_clks(ahci);
+}
+
+static struct ata_port_operations sunxi_ahci_platform_ops = {
+	.inherits	= &ahci_ops,
+	.host_stop	= sunxi_ahci_host_stop,
+};
+
+static const struct ata_port_info sunxiahci_port_info = {
+	AHCI_HFLAGS(AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
+			  AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
+	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &sunxi_ahci_platform_ops,
+};
+
+static struct scsi_host_template sunxi_ahci_platform_sht = {
+	AHCI_SHT("sunxi_ahci"),
+};
+
+static int sunxi_ahci_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct ata_port_info *ppi[] = { &sunxiahci_port_info, NULL };
+	struct sunxi_ahci *ahci;
+	struct ata_host *host;
+	int ret;
+
+	ahci = devm_kzalloc(&pdev->dev, sizeof(*ahci), GFP_KERNEL);
+	if (!ahci)
+		return -ENOMEM;
+
+	ahci->pwr = devm_regulator_get_optional(dev, "pwr");
+	if (IS_ERR(ahci->pwr) && PTR_ERR(ahci->pwr) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	host = ata_host_alloc_pinfo(dev, ppi, 1);
+	if (!host)
+		return -ENOMEM;
+
+	host->private_data = &ahci->hpriv;
+	host->flags |= ATA_HOST_PARALLEL_SCAN;
+
+	ahci->hpriv.flags = (unsigned long)ppi[0]->private_data;
+	ahci->hpriv.plat_data = ahci;
+	ahci->hpriv.pre_start_engine = sunxi_ahci_pre_start_engine;
+	ahci->hpriv.mmio = devm_ioremap_resource(dev,
+			      platform_get_resource(pdev, IORESOURCE_MEM, 0));
+	if (IS_ERR(ahci->hpriv.mmio))
+		return PTR_ERR(ahci->hpriv.mmio);
+
+	ahci->ahb_clk = devm_clk_get(&pdev->dev, "ahb_sata");
+	if (IS_ERR(ahci->ahb_clk))
+		return PTR_ERR(ahci->ahb_clk);
+
+	ahci->sata_clk = devm_clk_get(&pdev->dev, "pll6_sata");
+	if (IS_ERR(ahci->sata_clk))
+		return PTR_ERR(ahci->sata_clk);
+
+	ret = sunxi_ahci_enable_clks(ahci);
+	if (ret)
+		return ret;
+
+	if (!IS_ERR(ahci->pwr)) {
+		ret = regulator_enable(ahci->pwr);
+		if (ret) {
+			sunxi_ahci_disable_clks(ahci);
+			return ret;
+		}
+	}
+
+	ret = sunxi_ahci_phy_init(dev, ahci->hpriv.mmio);
+	if (ret) {
+		sunxi_ahci_host_stop(host);
+		return ret;
+	}
+
+	ahci_save_initial_config(dev, &ahci->hpriv, 0, 0);
+
+	ret = ahci_reset_controller(host);
+	if (ret) {
+		sunxi_ahci_host_stop(host);
+		return ret;
+	}
+
+	ahci_init_controller(host);
+	ahci_print_info(host, "sunxi");
+
+	ret = ata_host_activate(host, platform_get_irq(pdev, 0),
+				ahci_interrupt, 0, &sunxi_ahci_platform_sht);
+	if (ret)
+		sunxi_ahci_host_stop(host);
+
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sunxi_ahci_susp(struct device *dev)
+{
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct sunxi_ahci *ahci = hpriv->plat_data;
+	int ret;
+
+	/*
+	 * AHCI spec rev1.1 section 8.3.3:
+	 * Software must disable interrupts prior to requesting a
+	 * transition of the HBA to D3 state.
+	 */
+	sunxi_clrbits(hpriv->mmio + HOST_CTL, HOST_IRQ_EN);
+
+	ret = ata_host_suspend(host, PMSG_SUSPEND);
+	if (ret)
+		return ret;
+
+	sunxi_ahci_disable_clks(ahci);
+
+	return 0;
+}
+
+static int sunxi_ahci_resume(struct device *dev)
+{
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct sunxi_ahci *ahci = hpriv->plat_data;
+	int ret;
+
+	ret = sunxi_ahci_enable_clks(ahci);
+	if (ret)
+		return ret;
+
+	if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+		ret = ahci_reset_controller(host);
+		if (ret)
+			return ret;
+
+		ahci_init_controller(host);
+	}
+
+	ata_host_resume(host);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(sunxi_ahci_pmo, sunxi_ahci_susp, sunxi_ahci_resume);
+
+static const struct of_device_id sunxi_ahci_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-ahci" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sunxi_ahci_of_match);
+
+static struct platform_driver sunxi_ahci_driver = {
+	.probe = sunxi_ahci_probe,
+	.remove = ata_platform_remove_one,
+	.driver = {
+		.name = "sunxi-ahci",
+		.owner = THIS_MODULE,
+		.of_match_table = sunxi_ahci_of_match,
+		.pm = &sunxi_ahci_pmo,
+	},
+};
+module_platform_driver(sunxi_ahci_driver);
+
+MODULE_DESCRIPTION("Allwinner sunxi AHCI SATA platform driver");
+MODULE_AUTHOR("Olliver Schinagl <oliver@schinagl.nl>");
+MODULE_LICENSE("GPL");