[PATCH 0/3] AHCI: sunxi: Add sunxi AHCI driver

[PATCH 0/3] AHCI: sunxi: Add sunxi AHCI driver

[oliver]

From: Olliver Schinagl <oliver@schinagl.nl>

This patch series adds Allwinner sunxi support to platform_ahci.
There are a few quirks here however that will need discussing.

Allwinner added something to setup the DMA engine prior to enabling DMA in
libahci.c. I'm not familiar at all with the AHCI spec and not sure what
the proper way to implement this would have been. Don't other platforms
optionally require the same thing?

Also the way ahci_platform works seems a little hack-ish, it seems like
something along the lines of ahci_platform_register(ahci-data) would be more
appropriate, but not sure yet on those details (I am just a beginner).

Furthermore this driver should, in theory, work on the A10 but on my
cubieboard 1 it does not work yet (just hangs somewhere in libahci or after,
but not the DMA call that was altered). I've also tested it the cubietruck,
cubieboard 2 and olinuxino and it worked fine there.

Thanks,

Oliver

Oliver Schinagl (3):
  RFC: AHCI: libahci is missing DMA
  ARM: sunxi: Add an ahci-platform compatible AHCI driver for the    
    Allwinner SUNXi series of SoCs
  ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

 .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
 arch/arm/boot/dts/sun4i-a10-cubieboard.dts         |  26 ++
 arch/arm/boot/dts/sun4i-a10.dtsi                   |   9 +
 arch/arm/boot/dts/sun7i-a20-cubieboard2.dts        |  26 ++
 arch/arm/boot/dts/sun7i-a20-cubietruck.dts         |  26 ++
 arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts    |  26 ++
 arch/arm/boot/dts/sun7i-a20.dtsi                   |   9 +
 drivers/ata/Kconfig                                |   9 +
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci.h                                 |   6 +
 drivers/ata/ahci_platform.c                        |  12 +
 drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
 drivers/ata/libahci.c                              |   8 +
 13 files changed, 487 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
 create mode 100644 drivers/ata/ahci_sunxi.c

-- 
1.8.3.2

[PATCH 1/3] RFC: AHCI: libahci is missing DMA

[oliver]

From: Oliver Schinagl <oliver@schinagl.nl>

The Allwinner sunxi platforms have patched in the following to enable
DMA. This patch enables DMA controllers for the SUNXI Architecture.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/ata/ahci.h    | 6 ++++++
 drivers/ata/libahci.c | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 2289efdf..2bf2423 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -138,6 +138,7 @@ enum {
 	PORT_SCR_NTF		= 0x3c, /* SATA phy register: SNotification */
 	PORT_FBS		= 0x40, /* FIS-based Switching */
 	PORT_DEVSLP		= 0x44, /* device sleep */
+	PORT_DMA		= 0x70, /* direct memory access */
 
 	/* PORT_IRQ_{STAT,MASK} bits */
 	PORT_IRQ_COLD_PRES	= (1 << 31), /* cold presence detect */
@@ -209,6 +210,11 @@ enum {
 	PORT_DEVSLP_DSP		= (1 << 1),       /* DevSlp present */
 	PORT_DEVSLP_ADSE	= (1 << 0),       /* Aggressive DevSlp enable */
 
+	/* PORT_DMA bits */
+	PORT_DMA_SETUP_OFFSET	= 8, /* dma setup offset */
+	PORT_DMA_SETUP_MASK	= (0xff << PORT_DMA_SETUP_OFFSET),/* dma mask */
+	PORT_DMA_SETUP_INIT	= (0x44 << 0),
+
 	/* hpriv->flags bits */
 
 #define AHCI_HFLAGS(flags)		.private_data	= (void *)(flags)
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index c482f8c..d697a74 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -570,6 +570,14 @@ void ahci_start_engine(struct ata_port *ap)
 	void __iomem *port_mmio = ahci_port_base(ap);
 	u32 tmp;
 
+#ifdef CONFIG_ARCH_SUNXI
+	/* Setup DMA before DMA start */
+	tmp = readl(port_mmio + PORT_DMA);
+	tmp &= ~PORT_DMA_SETUP_MASK;
+	tmp |= PORT_DMA_SETUP_INIT << PORT_DMA_SETUP_OFFSET;
+	writel(tmp, port_mmio + PORT_DMA);
+#endif
+
 	/* start DMA */
 	tmp = readl(port_mmio + PORT_CMD);
 	tmp |= PORT_CMD_START;
-- 
1.8.3.2

Re: [PATCH 1/3] RFC: AHCI: libahci is missing DMA

[Tejun Heo]

On Wed, Dec 04, 2013 at 01:10:53PM +0100, oliver@schinagl.nl wrote:
> From: Oliver Schinagl <oliver@schinagl.nl>
> 
> The Allwinner sunxi platforms have patched in the following to enable
> DMA. This patch enables DMA controllers for the SUNXI Architecture.
> 
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/ata/ahci.h    | 6 ++++++
>  drivers/ata/libahci.c | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 2289efdf..2bf2423 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -138,6 +138,7 @@ enum {
>  	PORT_SCR_NTF		= 0x3c, /* SATA phy register: SNotification */
>  	PORT_FBS		= 0x40, /* FIS-based Switching */
>  	PORT_DEVSLP		= 0x44, /* device sleep */
> +	PORT_DMA		= 0x70, /* direct memory access */
>  
>  	/* PORT_IRQ_{STAT,MASK} bits */
>  	PORT_IRQ_COLD_PRES	= (1 << 31), /* cold presence detect */
> @@ -209,6 +210,11 @@ enum {
>  	PORT_DEVSLP_DSP		= (1 << 1),       /* DevSlp present */
>  	PORT_DEVSLP_ADSE	= (1 << 0),       /* Aggressive DevSlp enable */
>  
> +	/* PORT_DMA bits */
> +	PORT_DMA_SETUP_OFFSET	= 8, /* dma setup offset */
> +	PORT_DMA_SETUP_MASK	= (0xff << PORT_DMA_SETUP_OFFSET),/* dma mask */
> +	PORT_DMA_SETUP_INIT	= (0x44 << 0),

Ummm... this doesn't belong to ahci proper, right?

> +
>  	/* hpriv->flags bits */
>  
>  #define AHCI_HFLAGS(flags)		.private_data	= (void *)(flags)
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index c482f8c..d697a74 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -570,6 +570,14 @@ void ahci_start_engine(struct ata_port *ap)
>  	void __iomem *port_mmio = ahci_port_base(ap);
>  	u32 tmp;
>  
> +#ifdef CONFIG_ARCH_SUNXI
> +	/* Setup DMA before DMA start */
> +	tmp = readl(port_mmio + PORT_DMA);
> +	tmp &= ~PORT_DMA_SETUP_MASK;
> +	tmp |= PORT_DMA_SETUP_INIT << PORT_DMA_SETUP_OFFSET;
> +	writel(tmp, port_mmio + PORT_DMA);
> +#endif

If this is something platform device specific, wouldn't overriding
->port_start() which wraps around ahci_port_start() make more sense?

Thanks.

-- 
tejun

Re: [PATCH 1/3] RFC: AHCI: libahci is missing DMA

[Oliver Schinagl]

Hey Tejun Heo,

On 04-12-13 13:32, Tejun Heo wrote:
> On Wed, Dec 04, 2013 at 01:10:53PM +0100, oliver@schinagl.nl wrote:
>> From: Oliver Schinagl <oliver@schinagl.nl>
>>
>> The Allwinner sunxi platforms have patched in the following to enable
>> DMA. This patch enables DMA controllers for the SUNXI Architecture.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>   drivers/ata/ahci.h    | 6 ++++++
>>   drivers/ata/libahci.c | 8 ++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
>> index 2289efdf..2bf2423 100644
>> --- a/drivers/ata/ahci.h
>> +++ b/drivers/ata/ahci.h
>> @@ -138,6 +138,7 @@ enum {
>>   	PORT_SCR_NTF		= 0x3c, /* SATA phy register: SNotification */
>>   	PORT_FBS		= 0x40, /* FIS-based Switching */
>>   	PORT_DEVSLP		= 0x44, /* device sleep */
>> +	PORT_DMA		= 0x70, /* direct memory access */
>>   
>>   	/* PORT_IRQ_{STAT,MASK} bits */
>>   	PORT_IRQ_COLD_PRES	= (1 << 31), /* cold presence detect */
>> @@ -209,6 +210,11 @@ enum {
>>   	PORT_DEVSLP_DSP		= (1 << 1),       /* DevSlp present */
>>   	PORT_DEVSLP_ADSE	= (1 << 0),       /* Aggressive DevSlp enable */
>>   
>> +	/* PORT_DMA bits */
>> +	PORT_DMA_SETUP_OFFSET	= 8, /* dma setup offset */
>> +	PORT_DMA_SETUP_MASK	= (0xff << PORT_DMA_SETUP_OFFSET),/* dma mask */
>> +	PORT_DMA_SETUP_INIT	= (0x44 << 0),
> Ummm... this doesn't belong to ahci proper, right?
I have no idea why Allwinner added that and what it really does. We have 
no documentation, only code drops.

I had high hopes someone around here knows what it could mean and where 
it does belong.
>
>> +
>>   	/* hpriv->flags bits */
>>   
>>   #define AHCI_HFLAGS(flags)		.private_data	= (void *)(flags)
>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>> index c482f8c..d697a74 100644
>> --- a/drivers/ata/libahci.c
>> +++ b/drivers/ata/libahci.c
>> @@ -570,6 +570,14 @@ void ahci_start_engine(struct ata_port *ap)
>>   	void __iomem *port_mmio = ahci_port_base(ap);
>>   	u32 tmp;
>>   
>> +#ifdef CONFIG_ARCH_SUNXI
>> +	/* Setup DMA before DMA start */
>> +	tmp = readl(port_mmio + PORT_DMA);
>> +	tmp &= ~PORT_DMA_SETUP_MASK;
>> +	tmp |= PORT_DMA_SETUP_INIT << PORT_DMA_SETUP_OFFSET;
>> +	writel(tmp, port_mmio + PORT_DMA);
>> +#endif
> If this is something platform device specific, wouldn't overriding
> ->port_start() which wraps around ahci_port_start() make more sense?
Again, I don't know, this is where Allwinner had put it. We don't even 
know who's IP they use. I'm happy to start experimenting moving this 
around a bit and will take your clue to figure out what you mean and if 
it could work.

Oliver
>
> Thanks.
>

Re: [PATCH 1/3] RFC: AHCI: libahci is missing DMA

[Sergei Shtylyov]

Hello.

On 04-12-2013 16:10, oliver@schinagl.nl wrote:

> From: Oliver Schinagl <oliver@schinagl.nl>

> The Allwinner sunxi platforms have patched in the following to enable
> DMA. This patch enables DMA controllers for the SUNXI Architecture.

> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>   drivers/ata/ahci.h    | 6 ++++++
>   drivers/ata/libahci.c | 8 ++++++++
>   2 files changed, 14 insertions(+)

> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 2289efdf..2bf2423 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
[...]
> @@ -209,6 +210,11 @@ enum {
>   	PORT_DEVSLP_DSP		= (1 << 1),       /* DevSlp present */
>   	PORT_DEVSLP_ADSE	= (1 << 0),       /* Aggressive DevSlp enable */
>
> +	/* PORT_DMA bits */
> +	PORT_DMA_SETUP_OFFSET	= 8, /* dma setup offset */
> +	PORT_DMA_SETUP_MASK	= (0xff << PORT_DMA_SETUP_OFFSET),/* dma mask */
> +	PORT_DMA_SETUP_INIT	= (0x44 << 0),

    Why not shift it right by PORT_DMA_SETUP_OFFSET if you do it in the next 
file anyway?

[...]
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index c482f8c..d697a74 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -570,6 +570,14 @@ void ahci_start_engine(struct ata_port *ap)
>   	void __iomem *port_mmio = ahci_port_base(ap);
>   	u32 tmp;
>
> +#ifdef CONFIG_ARCH_SUNXI
> +	/* Setup DMA before DMA start */
> +	tmp = readl(port_mmio + PORT_DMA);
> +	tmp &= ~PORT_DMA_SETUP_MASK;
> +	tmp |= PORT_DMA_SETUP_INIT << PORT_DMA_SETUP_OFFSET;
> +	writel(tmp, port_mmio + PORT_DMA);
> +#endif
> +

    I don't think #ifdef is appropriate here.

WBR, Sergei

Re: [PATCH 1/3] RFC: AHCI: libahci is missing DMA

[Oliver Schinagl]

On 04-12-13 13:47, Sergei Shtylyov wrote:
> Hello.
>
> On 04-12-2013 16:10, oliver@schinagl.nl wrote:
>
>> From: Oliver Schinagl <oliver@schinagl.nl>
>
>> The Allwinner sunxi platforms have patched in the following to enable
>> DMA. This patch enables DMA controllers for the SUNXI Architecture.
>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>   drivers/ata/ahci.h    | 6 ++++++
>>   drivers/ata/libahci.c | 8 ++++++++
>>   2 files changed, 14 insertions(+)
>
>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
>> index 2289efdf..2bf2423 100644
>> --- a/drivers/ata/ahci.h
>> +++ b/drivers/ata/ahci.h
> [...]
>> @@ -209,6 +210,11 @@ enum {
>>       PORT_DEVSLP_DSP        = (1 << 1),       /* DevSlp present */
>>       PORT_DEVSLP_ADSE    = (1 << 0),       /* Aggressive DevSlp 
>> enable */
>>
>> +    /* PORT_DMA bits */
>> +    PORT_DMA_SETUP_OFFSET    = 8, /* dma setup offset */
>> +    PORT_DMA_SETUP_MASK    = (0xff << PORT_DMA_SETUP_OFFSET),/* dma 
>> mask */
>> +    PORT_DMA_SETUP_INIT    = (0x44 << 0),
>
>    Why not shift it right by PORT_DMA_SETUP_OFFSET if you do it in the 
> next file anyway?
Verbosity and clarity I suppose, try to reduce the number of magic 
values. I don't know what the preferred way is as we know so little 
about the IP.
>
> [...]
>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>> index c482f8c..d697a74 100644
>> --- a/drivers/ata/libahci.c
>> +++ b/drivers/ata/libahci.c
>> @@ -570,6 +570,14 @@ void ahci_start_engine(struct ata_port *ap)
>>       void __iomem *port_mmio = ahci_port_base(ap);
>>       u32 tmp;
>>
>> +#ifdef CONFIG_ARCH_SUNXI
>> +    /* Setup DMA before DMA start */
>> +    tmp = readl(port_mmio + PORT_DMA);
>> +    tmp &= ~PORT_DMA_SETUP_MASK;
>> +    tmp |= PORT_DMA_SETUP_INIT << PORT_DMA_SETUP_OFFSET;
>> +    writel(tmp, port_mmio + PORT_DMA);
>> +#endif
>> +
>
>    I don't think #ifdef is appropriate here.
It's not, but I was hoping Smarter devs then me would point me the right 
way ;)

Oliver
>
> WBR, Sergei
>

[PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[oliver]

From: Oliver Schinagl <oliver@schinagl.nl>

This patch adds support for the sunxi series of SoC's by allwinner. It
plugs into the ahci-platform framework.

Note: Currently it uses a somewhat hackish approach that probably needs
a lot more work, but does the same as the IMX SoC's.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
 drivers/ata/Kconfig                                |   9 +
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci_platform.c                        |  12 +
 drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
 5 files changed, 351 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
 create mode 100644 drivers/ata/ahci_sunxi.c

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..b87e2ba 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 SATA_AHCI_PLATFORM && 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..246050b 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
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index f955431..28ff1eb 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -31,6 +31,7 @@ enum ahci_type {
 	AHCI,		/* standard platform ahci */
 	IMX53_AHCI,	/* ahci on i.mx53 */
 	STRICT_AHCI,	/* delayed DMA engine start */
+	SUNXI_AHCI,	/* ahci on sunxi */
 };
 
 static struct platform_device_id ahci_devtype[] = {
@@ -44,6 +45,9 @@ static struct platform_device_id ahci_devtype[] = {
 		.name = "strict-ahci",
 		.driver_data = STRICT_AHCI,
 	}, {
+		.name = "sunxi-ahci",
+		.driver_data = SUNXI_AHCI,
+	}, {
 		/* sentinel */
 	}
 };
@@ -81,6 +85,14 @@ static const struct ata_port_info ahci_port_info[] = {
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &ahci_platform_ops,
 	},
+	[SUNXI_AHCI] = {
+		AHCI_HFLAGS	(AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
+				 AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
+		.flags		= AHCI_FLAG_COMMON,
+		.pio_mask	= ATA_PIO4,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &ahci_platform_ops,
+	},
 };
 
 static struct scsi_host_template ahci_platform_sht = {
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
new file mode 100644
index 0000000..982641f
--- /dev/null
+++ b/drivers/ata/ahci_sunxi.c
@@ -0,0 +1,305 @@
+/*
+ * Allwinner sunxi AHCI SATA platform driver
+ * Copyright 2013 Olliver Schinagl <oliver@schinagl.nl>
+ *
+ * Based on the AHCI SATA platform driver by Freescale and Allwinner
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/regulator/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/gfp.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/errno.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+#define DRV_NAME "sunxi-sata"
+
+#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_data {
+	struct platform_device *ahci_pdev;
+	struct regulator *regulator;
+	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");
+
+	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");
+	mdelay(15);
+
+	writel(0x7, reg_base + AHCI_RWCR);
+
+	return 0;
+}
+
+static int sunxi_ahci_init(struct device *dev, void __iomem *reg_base)
+{
+	struct sunxi_ahci_data *ahci_data;
+	int ret;
+
+	ahci_data = dev_get_drvdata(dev->parent);
+
+	ret = clk_prepare_enable(ahci_data->sata_clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_prepare_enable(ahci_data->ahb_clk);
+	if (ret < 0)
+		return ret;
+
+	ret = regulator_enable(ahci_data->regulator);
+	if (ret)
+		return ret;
+
+	return sunxi_ahci_phy_init(dev, reg_base);
+}
+
+static void sunxi_ahci_exit(struct device *dev)
+{
+	struct sunxi_ahci_data *ahci_data;
+
+	ahci_data = dev_get_drvdata(dev->parent);
+
+	regulator_disable(ahci_data->regulator);
+
+	clk_disable_unprepare(ahci_data->ahb_clk);
+	clk_disable_unprepare(ahci_data->sata_clk);
+}
+
+static struct ahci_platform_data sunxi_ahci_pdata = {
+	.init = sunxi_ahci_init,
+	.exit = sunxi_ahci_exit,
+};
+
+static int sunxi_ahci_remove(struct platform_device *pdev)
+{
+	struct sunxi_ahci_data *ahci_data;
+
+	ahci_data = platform_get_drvdata(pdev);
+	platform_device_unregister(ahci_data->ahci_pdev);
+
+	dev_dbg(&pdev->dev, "driver unloaded\n");
+
+	return 0;
+}
+
+static const struct of_device_id sunxi_ahci_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-ahci", .data = &sunxi_ahci_pdata},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, sunxi_ahci_of_match);
+
+static int sunxi_ahci_probe(struct platform_device *pdev)
+{
+	const struct ahci_platform_data *pdata;
+	const struct of_device_id *of_dev_id;
+	struct resource *mem, *irq, res[2];
+	struct platform_device *ahci_pdev;
+	struct sunxi_ahci_data *ahci_data;
+	struct regulator *regulator;
+	int ret;
+
+	regulator = devm_regulator_get(&pdev->dev, "pwr");
+	if (IS_ERR(regulator)) {
+		ret = PTR_ERR(regulator);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "no regulator found (%d)\n", ret);
+		return ret;
+	}
+
+	ahci_data = devm_kzalloc(&pdev->dev, sizeof(*ahci_data), GFP_KERNEL);
+	if (!ahci_data)
+		return -ENOMEM;
+
+	ahci_pdev = platform_device_alloc("sunxi-ahci", -1);
+	if (!ahci_pdev)
+		return -ENODEV;
+
+	ahci_pdev->dev.parent = &pdev->dev;
+
+	ahci_data->regulator = regulator;
+	ahci_data->ahb_clk = devm_clk_get(&pdev->dev, "ahb_sata");
+	if (IS_ERR(ahci_data->ahb_clk)) {
+		ret = PTR_ERR(ahci_data->ahb_clk);
+		goto err_out;
+	}
+
+	ahci_data->sata_clk = devm_clk_get(&pdev->dev, "pll6_sata");
+	if (IS_ERR(ahci_data->sata_clk)) {
+		ret = PTR_ERR(ahci_data->sata_clk);
+		goto err_out;
+	}
+
+	ahci_data->ahci_pdev = ahci_pdev;
+	platform_set_drvdata(pdev, ahci_data);
+
+	ahci_pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	ahci_pdev->dev.dma_mask = &ahci_pdev->dev.coherent_dma_mask;
+	ahci_pdev->dev.of_node = pdev->dev.of_node;
+
+	of_dev_id = of_match_device(sunxi_ahci_of_match, &pdev->dev);
+	if (of_dev_id) {
+		pdata = of_dev_id->data;
+	} else {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!mem || !irq) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	res[0] = *mem;
+	res[1] = *irq;
+	ret = platform_device_add_resources(ahci_pdev, res, 2);
+	if (ret)
+		goto err_out;
+
+	ret = platform_device_add_data(ahci_pdev, pdata, sizeof(*pdata));
+	if (ret)
+		goto err_out;
+
+	ret = platform_device_add(ahci_pdev);
+	if (ret)
+		goto err_out;
+
+	return 0;
+
+err_out:
+	platform_device_put(ahci_pdev);
+	return ret;
+}
+
+static struct platform_driver sunxi_ahci_driver = {
+	.probe = sunxi_ahci_probe,
+	.remove = sunxi_ahci_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = sunxi_ahci_of_match,
+	},
+};
+module_platform_driver(sunxi_ahci_driver);
+
+MODULE_DESCRIPTION("Allwinner sunxi AHCI SATA platform driver");
+MODULE_AUTHOR("Olliver Schinagl <oliver@schinagl.nl>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ahci:sunxi");
-- 
1.8.3.2

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Mark Rutland]

On Wed, Dec 04, 2013 at 12:10:54PM +0000, oliver@schinagl.nl wrote:
> From: Oliver Schinagl <oliver@schinagl.nl>
> 
> This patch adds support for the sunxi series of SoC's by allwinner. It
> plugs into the ahci-platform framework.
> 
> Note: Currently it uses a somewhat hackish approach that probably needs
> a lot more work, but does the same as the IMX SoC's.
> 
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
>  drivers/ata/Kconfig                                |   9 +
>  drivers/ata/Makefile                               |   1 +
>  drivers/ata/ahci_platform.c                        |  12 +
>  drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
>  5 files changed, 351 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
>  create mode 100644 drivers/ata/ahci_sunxi.c
> 
> 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"

- compatible: Should contain "allwinner,sun4i-a10-ahci"

> +- reg          : <registers mapping>

- reg: The offset and length of the MMIO registers.

> +- interrupts   : <interrupt mapping for AHCI IRQ>

- interrupts: An interrupt-specifier for the ACHI interrupt

> +- clocks       : clocks for ACHI
> +- clock-names  : clock names for AHCI

Please _define_ the set of clock-names you expect. This binding is
meaningless without it. If you require clock-names, define the clocks
property in terms of it.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Oliver Schinagl]

On 04-12-13 13:26, Mark Rutland wrote:
> On Wed, Dec 04, 2013 at 12:10:54PM +0000, oliver@schinagl.nl wrote:
>> From: Oliver Schinagl <oliver@schinagl.nl>
>>
>> This patch adds support for the sunxi series of SoC's by allwinner. It
>> plugs into the ahci-platform framework.
>>
>> Note: Currently it uses a somewhat hackish approach that probably needs
>> a lot more work, but does the same as the IMX SoC's.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>   .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
>>   drivers/ata/Kconfig                                |   9 +
>>   drivers/ata/Makefile                               |   1 +
>>   drivers/ata/ahci_platform.c                        |  12 +
>>   drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
>>   5 files changed, 351 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
>>   create mode 100644 drivers/ata/ahci_sunxi.c
>>
>> 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"
> - compatible: Should contain "allwinner,sun4i-a10-ahci"
>
>> +- reg          : <registers mapping>
> - reg: The offset and length of the MMIO registers.
>
>> +- interrupts   : <interrupt mapping for AHCI IRQ>
> - interrupts: An interrupt-specifier for the ACHI interrupt
>
>> +- clocks       : clocks for ACHI
>> +- clock-names  : clock names for AHCI
> Please _define_ the set of clock-names you expect. This binding is
> meaningless without it. If you require clock-names, define the clocks
> property in terms of it.
I copied ahci_platform.txt and filled in the missing bits, I will 
improve all the above. Appologies!
>
> Thanks,
> Mark.

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Tejun Heo]

On Wed, Dec 04, 2013 at 01:10:54PM +0100, oliver@schinagl.nl wrote:
> From: Oliver Schinagl <oliver@schinagl.nl>
> 
> This patch adds support for the sunxi series of SoC's by allwinner. It
> plugs into the ahci-platform framework.
> 
> Note: Currently it uses a somewhat hackish approach that probably needs
> a lot more work, but does the same as the IMX SoC's.
> 
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
>  drivers/ata/Kconfig                                |   9 +
>  drivers/ata/Makefile                               |   1 +
>  drivers/ata/ahci_platform.c                        |  12 +
>  drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++

I'm not really liking the way things are going.  Do we really need
separate drivers for each platform ahci implementation.  Are they
really that different?  Would it be impossible to make ahci_platform
generic enough so that we don't eventually end up with a gazillion
ahci_XXX drivers?

-- 
tejun

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Oliver Schinagl]

On 04-12-13 13:37, Tejun Heo wrote:
> On Wed, Dec 04, 2013 at 01:10:54PM +0100, oliver@schinagl.nl wrote:
>> From: Oliver Schinagl <oliver@schinagl.nl>
>>
>> This patch adds support for the sunxi series of SoC's by allwinner. It
>> plugs into the ahci-platform framework.
>>
>> Note: Currently it uses a somewhat hackish approach that probably needs
>> a lot more work, but does the same as the IMX SoC's.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>   .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
>>   drivers/ata/Kconfig                                |   9 +
>>   drivers/ata/Makefile                               |   1 +
>>   drivers/ata/ahci_platform.c                        |  12 +
>>   drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
> I'm not really liking the way things are going.  Do we really need
> separate drivers for each platform ahci implementation.  Are they
> really that different?  Would it be impossible to make ahci_platform
> generic enough so that we don't eventually end up with a gazillion
> ahci_XXX drivers?
I took the imx driver as example, as I wasn't sure on where to start. 
But I don't think it's possible yet without improving ahci_platform as I 
suggested in the cover letter. So if ahci_platform needs to be improved, 
I guess a separate patch series would be more appropriate?

So would it be acceptable to have this as the 2nd (and last?) 
ahci_platform driver and go from there? Or do you want to block new 
ahci_XXX drivers until ahci_platform has been improved?

Oliver
>

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Tejun Heo]

Hello,

On Wed, Dec 04, 2013 at 01:56:23PM +0100, Oliver Schinagl wrote:
> I took the imx driver as example, as I wasn't sure on where to
> start. But I don't think it's possible yet without improving
> ahci_platform as I suggested in the cover letter. So if
> ahci_platform needs to be improved, I guess a separate patch series
> would be more appropriate?
> 
> So would it be acceptable to have this as the 2nd (and last?)
> ahci_platform driver and go from there? Or do you want to block new
> ahci_XXX drivers until ahci_platform has been improved?

I don't want to block new drivers unconditionally but at least I want
to know which direction we're headed in the longer term.  Right now it
feels like we could be at the beginning of an uncoordinated explosion
of these drivers which will take a hell lot mpore effort to clean up
after the fact.  I could be wrong and these could actually be
different enough to justify separate drivers and there isn't gonna be
an avalanche of these but again I at least want to know the general
direction things are headed before making any decisions.

Thanks.

-- 
tejun

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Olliver Schinagl]

On 04-12-13 14:14, Tejun Heo wrote:
> Hello,
>
> On Wed, Dec 04, 2013 at 01:56:23PM +0100, Oliver Schinagl wrote:
>> I took the imx driver as example, as I wasn't sure on where to
>> start. But I don't think it's possible yet without improving
>> ahci_platform as I suggested in the cover letter. So if
>> ahci_platform needs to be improved, I guess a separate patch series
>> would be more appropriate?
>>
>> So would it be acceptable to have this as the 2nd (and last?)
>> ahci_platform driver and go from there? Or do you want to block new
>> ahci_XXX drivers until ahci_platform has been improved?
>
> I don't want to block new drivers unconditionally but at least I want
> to know which direction we're headed in the longer term.  Right now it
> feels like we could be at the beginning of an uncoordinated explosion
> of these drivers which will take a hell lot mpore effort to clean up
> after the fact.  I could be wrong and these could actually be
> different enough to justify separate drivers and there isn't gonna be
> an avalanche of these but again I at least want to know the general
> direction things are headed before making any decisions.
I'd be happy to pour it in any form that's needed. I even do the 
modification/rewrite of ahci_platform if I get enough help as it might 
be a little over my head initially ;)

That said, I don't think it's much different at all and I do think it 
could be much simpler. In my mind, the sunxi_ahci driver wouldn't need 
to be much bigger then a few lines that are specific to the SoC 
(hardware init) and registerd to the ahci_platform framework via 
platform_ahci_register() instead of platform_device_register().

But again, point me (for dummies ;) in the right direction and I'll work 
on it with some help.

Oliver
>
> Thanks.
>

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Tejun Heo]

Hello,

(cc'ing Richard and Shawn, hi!)

On Wed, Dec 04, 2013 at 02:16:49PM +0100, Olliver Schinagl wrote:
> On 04-12-13 14:14, Tejun Heo wrote:
> >Hello,
> >
> >On Wed, Dec 04, 2013 at 01:56:23PM +0100, Oliver Schinagl wrote:
> >>I took the imx driver as example, as I wasn't sure on where to
> >>start. But I don't think it's possible yet without improving
> >>ahci_platform as I suggested in the cover letter. So if
> >>ahci_platform needs to be improved, I guess a separate patch series
> >>would be more appropriate?
> >>
> >>So would it be acceptable to have this as the 2nd (and last?)
> >>ahci_platform driver and go from there? Or do you want to block new
> >>ahci_XXX drivers until ahci_platform has been improved?
> >
> >I don't want to block new drivers unconditionally but at least I want
> >to know which direction we're headed in the longer term.  Right now it
> >feels like we could be at the beginning of an uncoordinated explosion
> >of these drivers which will take a hell lot mpore effort to clean up
> >after the fact.  I could be wrong and these could actually be
> >different enough to justify separate drivers and there isn't gonna be
> >an avalanche of these but again I at least want to know the general
> >direction things are headed before making any decisions.
> I'd be happy to pour it in any form that's needed. I even do the
> modification/rewrite of ahci_platform if I get enough help as it
> might be a little over my head initially ;)
> 
> That said, I don't think it's much different at all and I do think
> it could be much simpler. In my mind, the sunxi_ahci driver wouldn't
> need to be much bigger then a few lines that are specific to the SoC
> (hardware init) and registerd to the ahci_platform framework via
> platform_ahci_register() instead of platform_device_register().
> 
> But again, point me (for dummies ;) in the right direction and I'll
> work on it with some help.

Richard and Shawn recently worked on ahci_imx.  Can you guys please
talk with each other and figure out what can be done to share as much
as possible among these new platform-specific drivers?  I'd really
like to see the common things factored out as much as possible with
only the actual hardware differences described for each device.

Thanks a lot!

-- 
tejun

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Thomas Petazzoni]

Dear Tejun Heo,

On Wed, 4 Dec 2013 08:23:12 -0500, Tejun Heo wrote:

> > But again, point me (for dummies ;) in the right direction and I'll
> > work on it with some help.
> 
> Richard and Shawn recently worked on ahci_imx.  Can you guys please
> talk with each other and figure out what can be done to share as much
> as possible among these new platform-specific drivers?  I'd really
> like to see the common things factored out as much as possible with
> only the actual hardware differences described for each device.

Also, please Cc me on such discussions. I have a pending AHCI platform
driver for another ARM SoC family. It is very similar to ahci_platform,
but needs to do a few more things that are SoC specific (map an
additional register area, and do some SoC-specific stuff with them).

For the moment, we're left with two approaches:

 * Do what Oliver did, where the ahci_<foo> driver will do its own
   SoC-specific stuff, and then will register an additional
   platform_device to trigger the ->probe() of the generic
   ahci_platform driver. I must say I don't really like this solution,
   since it involves having two platform_device registered for the same
   piece of hardware (one platform_device to trigger the ->probe of
   ahci_<foo>, and another one to trigger the ->probe of ahci_platform).

 * Duplicate in ahci_<foo> the (relatively small) amount of code that
   is present in ahci_platform.

>From my point of view, ahci_platform should be turned into a small
"library", that provides an API for ahci_<foo> drivers to 1/ do their
own custom stuff and 2/ do the common ahci_platform stuff.

This way we avoid the registration of two platform_device for the same
piece of hardware, and we avoid the duplication of code.

Want me to propose a RFC for this idea?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Oliver Schinagl]

On 06-12-13 10:01, Thomas Petazzoni wrote:
> Dear Tejun Heo,
>
> On Wed, 4 Dec 2013 08:23:12 -0500, Tejun Heo wrote:
>
>>> But again, point me (for dummies ;) in the right direction and I'll
>>> work on it with some help.
>> Richard and Shawn recently worked on ahci_imx.  Can you guys please
>> talk with each other and figure out what can be done to share as much
>> as possible among these new platform-specific drivers?  I'd really
>> like to see the common things factored out as much as possible with
>> only the actual hardware differences described for each device.
> Also, please Cc me on such discussions. I have a pending AHCI platform
> driver for another ARM SoC family. It is very similar to ahci_platform,
> but needs to do a few more things that are SoC specific (map an
> additional register area, and do some SoC-specific stuff with them).
>
> For the moment, we're left with two approaches:
>
>   * Do what Oliver did, where the ahci_<foo> driver will do its own
>     SoC-specific stuff, and then will register an additional
>     platform_device to trigger the ->probe() of the generic
>     ahci_platform driver. I must say I don't really like this solution,
>     since it involves having two platform_device registered for the same
>     piece of hardware (one platform_device to trigger the ->probe of
>     ahci_<foo>, and another one to trigger the ->probe of ahci_platform).
>
>   * Duplicate in ahci_<foo> the (relatively small) amount of code that
>     is present in ahci_platform.
>
>  From my point of view, ahci_platform should be turned into a small
> "library", that provides an API for ahci_<foo> drivers to 1/ do their
> own custom stuff and 2/ do the common ahci_platform stuff.
>
> This way we avoid the registration of two platform_device for the same
> piece of hardware, and we avoid the duplication of code.
>
> Want me to propose a RFC for this idea?
I've started to do what sdhci does with their pltfrm driver, assuming 
that's the right approach. Since i'm only dabbling and not always 100% 
sure what should or shouldn't be done, it may take a little while, but 
looks promising from my end ;)

So is the sdhci-pltfrm approach the correct one? We still have ahci_* 
drivers, but ahci_platform.c won't be a driver in the sense that it is 
now anymore.

Oliver
>
> Best regards,
>
> Thomas

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Thomas Petazzoni]

Dear Oliver Schinagl,

On Fri, 06 Dec 2013 10:12:34 +0100, Oliver Schinagl wrote:

> >  From my point of view, ahci_platform should be turned into a small
> > "library", that provides an API for ahci_<foo> drivers to 1/ do their
> > own custom stuff and 2/ do the common ahci_platform stuff.
> >
> > This way we avoid the registration of two platform_device for the same
> > piece of hardware, and we avoid the duplication of code.
> >
> > Want me to propose a RFC for this idea?
> I've started to do what sdhci does with their pltfrm driver, assuming 
> that's the right approach. Since i'm only dabbling and not always 100% 
> sure what should or shouldn't be done, it may take a little while, but 
> looks promising from my end ;)

Yes, the approach of shdci_pltfrm is exactly the one I was proposing
here, so it definitely looks like the right direction to me (though my
opinion is not authoritative at all in this area, obviously).

> So is the sdhci-pltfrm approach the correct one? We still have ahci_* 
> drivers, but ahci_platform.c won't be a driver in the sense that it is 
> now anymore.

Yes, seems good. We will still need ahci_<foo> for the various SoC
families, because depending on the SoC, you have different things to do
(take various clocks, or do other SoC-specific configuration). As an
example, the main thing I have to do on the SoC I'm working with is
configuring some memory windows that allow the SATA controller to do
DMA to the DRAM. This configuration is inherently completely specific
to this SoC family, and it wouldn't make sense to have that in an
ahci_platform driver shared by all platforms.

Oliver, can you Cc me on your future patches about this topic, so that
I can test them in the context of the SoC I'm working on?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Hans de Goede]

Hi,

On 12/06/2013 10:12 AM, Oliver Schinagl wrote:
>
> On 06-12-13 10:01, Thomas Petazzoni wrote:
>> Dear Tejun Heo,
>>
>> On Wed, 4 Dec 2013 08:23:12 -0500, Tejun Heo wrote:
>>
>>>> But again, point me (for dummies ;) in the right direction and I'll
>>>> work on it with some help.
>>> Richard and Shawn recently worked on ahci_imx.  Can you guys please
>>> talk with each other and figure out what can be done to share as much
>>> as possible among these new platform-specific drivers?  I'd really
>>> like to see the common things factored out as much as possible with
>>> only the actual hardware differences described for each device.
>> Also, please Cc me on such discussions. I have a pending AHCI platform
>> driver for another ARM SoC family. It is very similar to ahci_platform,
>> but needs to do a few more things that are SoC specific (map an
>> additional register area, and do some SoC-specific stuff with them).
>>
>> For the moment, we're left with two approaches:
>>
>>   * Do what Oliver did, where the ahci_<foo> driver will do its own
>>     SoC-specific stuff, and then will register an additional
>>     platform_device to trigger the ->probe() of the generic
>>     ahci_platform driver. I must say I don't really like this solution,
>>     since it involves having two platform_device registered for the same
>>     piece of hardware (one platform_device to trigger the ->probe of
>>     ahci_<foo>, and another one to trigger the ->probe of ahci_platform).
>>
>>   * Duplicate in ahci_<foo> the (relatively small) amount of code that
>>     is present in ahci_platform.
>>
>>  From my point of view, ahci_platform should be turned into a small
>> "library", that provides an API for ahci_<foo> drivers to 1/ do their
>> own custom stuff and 2/ do the common ahci_platform stuff.
>>
>> This way we avoid the registration of two platform_device for the same
>> piece of hardware, and we avoid the duplication of code.
>>
>> Want me to propose a RFC for this idea?
> I've started to do what sdhci does with their pltfrm driver, assuming that's the right approach. Since i'm only dabbling and not always 100% sure what should or shouldn't be done, it may take a little while, but looks promising from my end ;)
>
> So is the sdhci-pltfrm approach the correct one? We still have ahci_* drivers, but ahci_platform.c won't be a driver in the sense that it is now anymore.

Sounds good to me. May I suggest simply adding a new ahci_pltfrm driver for
this and leaving the existing ahci_platform alone? Of course in the end
we want the old ahci_platform to go away, but it is probably best to
introduce the new one in parallel and then port things over 1 by 1
by people who can actually test the port :)

Regards,

Hans

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Hans de Goede]

Hi Tejun,

I think it would be a good idea to merge ahci upstream using the
ahci_imx.c method for now. You already indicated that you were not against
doing that for now.

Oliver is working on getting a cleaner solution for this, but doing this
properly takes tinme, and we would like to move forward with getting
sunxi ahci support upstream in the mean time.

So if it is ok with you we would like to move forward with the version
initially posted. Therefor I would like to request you to review it
(glossing over the platform device instantiating a platform device
approach for now), and provide us with feedback so that we can do a v2
and start moving this towards the mainline kernel.

Regards,

Hans

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Tejun Heo]

Hello, Hans.

On Sun, Dec 15, 2013 at 08:00:20PM +0100, Hans de Goede wrote:
> I think it would be a good idea to merge ahci upstream using the
> ahci_imx.c method for now. You already indicated that you were not against
> doing that for now.

Well, the thing is nothing actually happened since ahci_imx got
merged, so I'm wondering maybe there isn't enough pressure.

> Oliver is working on getting a cleaner solution for this, but doing this
> properly takes tinme, and we would like to move forward with getting
> sunxi ahci support upstream in the mean time.
> 
> So if it is ok with you we would like to move forward with the version
> initially posted. Therefor I would like to request you to review it
> (glossing over the platform device instantiating a platform device
> approach for now), and provide us with feedback so that we can do a v2
> and start moving this towards the mainline kernel.

We still have some time left before the next merge window and if it's
really necessary, I can submit drivers post -rc1 too, so I'd still
*much* prefer doing it properly rather than creating more drivers
which would need to be cleaned up later.

Thanks.

-- 
tejun

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Oliver Schinagl]

On 15-12-13 20:04, Tejun Heo wrote:
> Hello, Hans.
>
> On Sun, Dec 15, 2013 at 08:00:20PM +0100, Hans de Goede wrote:
>> I think it would be a good idea to merge ahci upstream using the
>> ahci_imx.c method for now. You already indicated that you were not against
>> doing that for now.
> Well, the thing is nothing actually happened since ahci_imx got
> merged, so I'm wondering maybe there isn't enough pressure.
I a slowly progressing on the improved platform driver, but still 
waiting on an answer about my clock question that I asked a few days ago.
>
>> Oliver is working on getting a cleaner solution for this, but doing this
>> properly takes tinme, and we would like to move forward with getting
>> sunxi ahci support upstream in the mean time.
>>
>> So if it is ok with you we would like to move forward with the version
>> initially posted. Therefor I would like to request you to review it
>> (glossing over the platform device instantiating a platform device
>> approach for now), and provide us with feedback so that we can do a v2
>> and start moving this towards the mainline kernel.
> We still have some time left before the next merge window and if it's
> really necessary, I can submit drivers post -rc1 too, so I'd still
> *much* prefer doing it properly rather than creating more drivers
> which would need to be cleaned up later.
Since I do this in my spare time and my technical skill isn't as 
advanced as some, I don't think I have something reviewed and merge-able 
before that time frame. I do enjoy the challenge and do like working on 
the new ahci_platform framework. and do wish to continue to work on it 
however.

Oliver
>
> Thanks.
>

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Olliver Schinagl]

Hey all,

On 04-12-13 14:23, Tejun Heo wrote:
> Hello,
>
> (cc'ing Richard and Shawn, hi!)
>
> On Wed, Dec 04, 2013 at 02:16:49PM +0100, Olliver Schinagl wrote:
>> On 04-12-13 14:14, Tejun Heo wrote:
>>> Hello,
>>>
>>> On Wed, Dec 04, 2013 at 01:56:23PM +0100, Oliver Schinagl wrote:
>>>> I took the imx driver as example, as I wasn't sure on where to
>>>> start. But I don't think it's possible yet without improving
>>>> ahci_platform as I suggested in the cover letter. So if
>>>> ahci_platform needs to be improved, I guess a separate patch series
>>>> would be more appropriate?
>>>>
>>>> So would it be acceptable to have this as the 2nd (and last?)
>>>> ahci_platform driver and go from there? Or do you want to block new
>>>> ahci_XXX drivers until ahci_platform has been improved?
>>> I don't want to block new drivers unconditionally but at least I want
>>> to know which direction we're headed in the longer term.  Right now it
>>> feels like we could be at the beginning of an uncoordinated explosion
>>> of these drivers which will take a hell lot mpore effort to clean up
>>> after the fact.  I could be wrong and these could actually be
>>> different enough to justify separate drivers and there isn't gonna be
>>> an avalanche of these but again I at least want to know the general
>>> direction things are headed before making any decisions.
>> I'd be happy to pour it in any form that's needed. I even do the
>> modification/rewrite of ahci_platform if I get enough help as it
>> might be a little over my head initially ;)
>>
>> That said, I don't think it's much different at all and I do think
>> it could be much simpler. In my mind, the sunxi_ahci driver wouldn't
>> need to be much bigger then a few lines that are specific to the SoC
>> (hardware init) and registerd to the ahci_platform framework via
>> platform_ahci_register() instead of platform_device_register().
>>
>> But again, point me (for dummies ;) in the right direction and I'll
>> work on it with some help.
> Richard and Shawn recently worked on ahci_imx.  Can you guys please
> talk with each other and figure out what can be done to share as much
> as possible among these new platform-specific drivers?  I'd really
> like to see the common things factored out as much as possible with
> only the actual hardware differences described for each device.
Working on this and studying the existing ahci_platform/shci_platform 
drivers the last few days and was figuring out why ahci_platform only 
supports 1 clock. IMX handles this by having 3 clocks defined in the DT, 
the first one gets enabled by default via ahci_platform, the other 2 get 
enabled in IMX's probe function.

Is it an idea to extend this to support all clocks that would be 
required (via a callback)? Or do we prefer having the clocks separated 
for other technical reasons? Or do we want to handle the clocks via the 
ahci_platform framework and extend hpriv->clk to an array of clocks?

Oliver

>
> Thanks a lot!
>

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Shawn Guo]

On Wed, Dec 11, 2013 at 03:51:51PM +0100, Olliver Schinagl wrote:
> Working on this and studying the existing
> ahci_platform/shci_platform drivers the last few days and was
> figuring out why ahci_platform only supports 1 clock. IMX handles
> this by having 3 clocks defined in the DT, the first one gets
> enabled by default via ahci_platform, the other 2 get enabled in
> IMX's probe function.
> 
> Is it an idea to extend this to support all clocks that would be
> required (via a callback)?

Not really.  We did this for ahci_imx driver only because we do not want
to churn generic ahci_platform driver with those imx specific setup
code.  Note, beside the additional two clocks, we have some PHY
parameters to set up in IMX IOMUXC general purpose registers, and vendor
specific register HOST_TIMER1MS to be set up as well.

> Or do we prefer having the clocks
> separated for other technical reasons? Or do we want to handle the
> clocks via the ahci_platform framework and extend hpriv->clk to an
> array of clocks?

The direction of the generic ahci platform driver will be having it be
a library providing helper functions, as discussed as below.

https://lkml.org/lkml/2013/12/6/153

We can ask the helper function to handle the common clocks and leave the
platform specific ones to platform driver.

Shawn

Re: [PATCH 2/3] ARM: sunxi: Add an ahci-platform compatible AHCI driver for the Allwinner SUNXi series of SoCs

[Olliver Schinagl]

On 12-12-13 07:40, Shawn Guo wrote:
> On Wed, Dec 11, 2013 at 03:51:51PM +0100, Olliver Schinagl wrote:
>> Working on this and studying the existing
>> ahci_platform/shci_platform drivers the last few days and was
>> figuring out why ahci_platform only supports 1 clock. IMX handles
>> this by having 3 clocks defined in the DT, the first one gets
>> enabled by default via ahci_platform, the other 2 get enabled in
>> IMX's probe function.
>>
>> Is it an idea to extend this to support all clocks that would be
>> required (via a callback)?
> Not really.  We did this for ahci_imx driver only because we do not want
> to churn generic ahci_platform driver with those imx specific setup
> code.  Note, beside the additional two clocks, we have some PHY
> parameters to set up in IMX IOMUXC general purpose registers, and vendor
> specific register HOST_TIMER1MS to be set up as well.
Well many SoC's have these specific things so we need a place to provide 
for this.

The the question then becomes, what do we define as being the common 
clocks? Only the clock for the SATA IP? Or also the clock to which it 
connects to the bus? Where is the line drawn etc.

E.g. a rewritten new ahci-pltfrm.c will support all clocks defined in 
the DT for the ahci node.
>
>> Or do we prefer having the clocks
>> separated for other technical reasons? Or do we want to handle the
>> clocks via the ahci_platform framework and extend hpriv->clk to an
>> array of clocks?
> The direction of the generic ahci platform driver will be having it be
> a library providing helper functions, as discussed as below.
>
> https://lkml.org/lkml/2013/12/6/153
I know, I started this entire thread :p hence my question :)

Oliver
>
> We can ask the helper function to handle the common clocks and leave the
> platform specific ones to platform driver.
>
> Shawn
>

[PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

[oliver]

From: Oliver Schinagl <oliver@schinagl.nl>

This patch adds sunxi sata support to A10 and A20 boards that have such
a connector. Some boards also feature a regulator via a GPIO and support
for this is also added.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 arch/arm/boot/dts/sun4i-a10-cubieboard.dts      | 26 +++++++++++++++++++++++++
 arch/arm/boot/dts/sun4i-a10.dtsi                |  9 +++++++++
 arch/arm/boot/dts/sun7i-a20-cubieboard2.dts     | 26 +++++++++++++++++++++++++
 arch/arm/boot/dts/sun7i-a20-cubietruck.dts      | 26 +++++++++++++++++++++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 26 +++++++++++++++++++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi                |  9 +++++++++
 6 files changed, 122 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
index 425a7db..b620084 100644
--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
@@ -42,7 +42,18 @@
 			};
 		};
 
+		sata: ahci@01c18000 {
+			pwr-supply = <&reg_ahci_5v>;
+			status = "okay";
+		};
+
 		pinctrl@01c20800 {
+			ahci_pwr_pin: ahci_pwr_pin@0 {
+				allwinner,pins = "PB8";
+				allwinner,function = "gpio_out";
+				allwinner,driver = <0>;
+				allwinner,pull = <0>;
+			};
 			led_pins_cubieboard: led_pins@0 {
 				allwinner,pins = "PH20", "PH21";
 				allwinner,function = "gpio_out";
@@ -86,4 +97,19 @@
 			linux,default-trigger = "heartbeat";
 		};
 	};
+
+	regulators {
+		compatible = "simple-bus";
+		pinctrl-names = "default";
+
+		reg_ahci_5v: ahci-5v {
+			compatible = "regulator-fixed";
+			regulator-name = "ahci-5v";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			pinctrl-0 = <&ahci_pwr_pin>;
+			gpio = <&pio 1 8 0>;
+			enable-active-high;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 4dccdb0..53c6cdb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -306,6 +306,15 @@
 			#size-cells = <0>;
 		};
 
+		sata: 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";
+			status = "disabled";
+		};
+
 		intc: interrupt-controller@01c20400 {
 			compatible = "allwinner,sun4i-ic";
 			reg = <0x01c20400 0x400>;
diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
index 5c51cb8..99c5e78 100644
--- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
@@ -34,7 +34,18 @@
 			};
 		};
 
+		sata: ahci@01c18000 {
+			pwr-supply = <&reg_ahci_5v>;
+			status = "okay";
+		};
+
 		pinctrl@01c20800 {
+			ahci_pwr_pin: ahci_pwr_pin@0 {
+				allwinner,pins = "PB8";
+				allwinner,function = "gpio_out";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
 			led_pins_cubieboard2: led_pins@0 {
 				allwinner,pins = "PH20", "PH21";
 				allwinner,function = "gpio_out";
@@ -77,4 +88,19 @@
 			gpios = <&pio 7 20 0>;
 		};
 	};
+
+	regulators {
+		compatible = "simple-bus";
+		pinctrl-names = "default";
+
+		reg_ahci_5v: ahci-5v {
+			compatible = "regulator-fixed";
+			regulator-name = "ahci-5v";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			pinctrl-0 = <&ahci_pwr_pin>;
+			gpio = <&pio 1 8 0>;
+			enable-active-high;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
index 8a1009d..19af23e 100644
--- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
+++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
@@ -19,7 +19,18 @@
 	compatible = "cubietech,cubietruck", "allwinner,sun7i-a20";
 
 	soc@01c00000 {
+		sata: ahci@01c18000 {
+			pwr-supply = <&reg_ahci_5v>;
+			status = "okay";
+		};
+
 		pinctrl@01c20800 {
+			ahci_pwr_pin: ahci_pwr_pin@0 {
+				allwinner,pins = "PH12";
+				allwinner,function = "gpio_out";
+				allwinner,driver = <0>;
+				allwinner,pull = <0>;
+			};
 			led_pins_cubietruck: led_pins@0 {
 				allwinner,pins = "PH7", "PH11", "PH20", "PH21";
 				allwinner,function = "gpio_out";
@@ -60,4 +71,19 @@
 			gpios = <&pio 7 7 0>;
 		};
 	};
+
+	regulators {
+		compatible = "simple-bus";
+		pinctrl-names = "default";
+
+		reg_ahci_5v: ahci-5v {
+			compatible = "regulator-fixed";
+			regulator-name = "ahci-5v";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			pinctrl-0 = <&ahci_pwr_pin>;
+			gpio = <&pio 7 12 0>;
+			enable-active-high;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
index ead3013..23ed708 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
@@ -34,7 +34,19 @@
 			};
 		};
 
+		sata: ahci@01c18000 {
+			pwr-supply = <&reg_ahci_5v>;
+			status = "okay";
+		};
+
 		pinctrl@01c20800 {
+			ahci_pwr_pin: ahci_pwr_pin@0 {
+				allwinner,pins = "PB8";
+				allwinner,function = "gpio_out";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
+
 			led_pins_olinuxino: led_pins@0 {
 				allwinner,pins = "PH2";
 				allwinner,function = "gpio_out";
@@ -91,4 +103,18 @@
 			default-state = "on";
 		};
 	};
+
+	regulators {
+		compatible = "simple-bus";
+
+		reg_ahci_5v: ahci-5v {
+			compatible = "regulator-fixed";
+			regulator-name = "ahci-5v";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			pinctrl-0 = <&ahci_pwr_pin>;
+			gpio = <&pio 1 8 0>;
+			enable-active-high;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 0552a64..b72c69e 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -368,6 +368,15 @@
 			};
 		};
 
+		sata: 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";
+			status = "disabled";
+		};
+
 		timer@01c20c00 {
 			compatible = "allwinner,sun4i-timer";
 			reg = <0x01c20c00 0x90>;
-- 
1.8.3.2

Re: [PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7581 bytes --]

Hi Oliver,

On Wed, Dec 04, 2013 at 01:10:55PM +0100, oliver@schinagl.nl wrote:
> From: Oliver Schinagl <oliver@schinagl.nl>
> 
> This patch adds sunxi sata support to A10 and A20 boards that have such
> a connector. Some boards also feature a regulator via a GPIO and support
> for this is also added.
> 
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Your git setup seems to be pretty uncertain about how your first name is spelled :)

> ---
>  arch/arm/boot/dts/sun4i-a10-cubieboard.dts      | 26 +++++++++++++++++++++++++
>  arch/arm/boot/dts/sun4i-a10.dtsi                |  9 +++++++++
>  arch/arm/boot/dts/sun7i-a20-cubieboard2.dts     | 26 +++++++++++++++++++++++++
>  arch/arm/boot/dts/sun7i-a20-cubietruck.dts      | 26 +++++++++++++++++++++++++
>  arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 26 +++++++++++++++++++++++++
>  arch/arm/boot/dts/sun7i-a20.dtsi                |  9 +++++++++
>  6 files changed, 122 insertions(+)

Could you split this into several patches please?

At least one per SoC.

> diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> index 425a7db..b620084 100644
> --- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> @@ -42,7 +42,18 @@
>  			};
>  		};
>  
> +		sata: ahci@01c18000 {
> +			pwr-supply = <&reg_ahci_5v>;
> +			status = "okay";
> +		};
> +
>  		pinctrl@01c20800 {
> +			ahci_pwr_pin: ahci_pwr_pin@0 {

Please prefix it with name of the board.

> +				allwinner,pins = "PB8";
> +				allwinner,function = "gpio_out";
> +				allwinner,driver = <0>;
> +				allwinner,pull = <0>;
> +			};

Please add a newline here.

>  			led_pins_cubieboard: led_pins@0 {
>  				allwinner,pins = "PH20", "PH21";
>  				allwinner,function = "gpio_out";
> @@ -86,4 +97,19 @@
>  			linux,default-trigger = "heartbeat";
>  		};
>  	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		pinctrl-names = "default";
> +
> +		reg_ahci_5v: ahci-5v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "ahci-5v";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			pinctrl-0 = <&ahci_pwr_pin>;
> +			gpio = <&pio 1 8 0>;
> +			enable-active-high;
> +		};
> +	};
>  };
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 4dccdb0..53c6cdb 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -306,6 +306,15 @@
>  			#size-cells = <0>;
>  		};
>  
> +		sata: ahci@01c18000 {
> +			compatible = "allwinner,sun4i-a10-ahci";

Please use sun4i-ahci for consistency.

> +			reg = <0x01c18000 0x1000>;
> +			interrupts = <0 56 1>;

The interrupt here doesn't seem right. Is it actually working at all?

> +			clocks = <&ahb_gates 25>, <&pll6 0>;
> +			clock-names = "ahb_sata", "pll6_sata";
> +			status = "disabled";
> +		};
> +
>  		intc: interrupt-controller@01c20400 {
>  			compatible = "allwinner,sun4i-ic";
>  			reg = <0x01c20400 0x400>;
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> index 5c51cb8..99c5e78 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -34,7 +34,18 @@
>  			};
>  		};
>  
> +		sata: ahci@01c18000 {
> +			pwr-supply = <&reg_ahci_5v>;
> +			status = "okay";
> +		};
> +
>  		pinctrl@01c20800 {
> +			ahci_pwr_pin: ahci_pwr_pin@0 {
> +				allwinner,pins = "PB8";
> +				allwinner,function = "gpio_out";
> +				allwinner,drive = <0>;
> +				allwinner,pull = <0>;
> +			};

Please add a newline here.

>  			led_pins_cubieboard2: led_pins@0 {
>  				allwinner,pins = "PH20", "PH21";
>  				allwinner,function = "gpio_out";
> @@ -77,4 +88,19 @@
>  			gpios = <&pio 7 20 0>;
>  		};
>  	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		pinctrl-names = "default";
> +
> +		reg_ahci_5v: ahci-5v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "ahci-5v";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			pinctrl-0 = <&ahci_pwr_pin>;
> +			gpio = <&pio 1 8 0>;
> +			enable-active-high;
> +		};
> +	};
>  };
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> index 8a1009d..19af23e 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> @@ -19,7 +19,18 @@
>  	compatible = "cubietech,cubietruck", "allwinner,sun7i-a20";
>  
>  	soc@01c00000 {
> +		sata: ahci@01c18000 {
> +			pwr-supply = <&reg_ahci_5v>;
> +			status = "okay";
> +		};
> +
>  		pinctrl@01c20800 {
> +			ahci_pwr_pin: ahci_pwr_pin@0 {
> +				allwinner,pins = "PH12";
> +				allwinner,function = "gpio_out";
> +				allwinner,driver = <0>;
> +				allwinner,pull = <0>;
> +			};

Please add a newline here.

>  			led_pins_cubietruck: led_pins@0 {
>  				allwinner,pins = "PH7", "PH11", "PH20", "PH21";
>  				allwinner,function = "gpio_out";
> @@ -60,4 +71,19 @@
>  			gpios = <&pio 7 7 0>;
>  		};
>  	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		pinctrl-names = "default";
> +
> +		reg_ahci_5v: ahci-5v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "ahci-5v";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			pinctrl-0 = <&ahci_pwr_pin>;
> +			gpio = <&pio 7 12 0>;
> +			enable-active-high;
> +		};
> +	};
>  };
> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> index ead3013..23ed708 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> @@ -34,7 +34,19 @@
>  			};
>  		};
>  
> +		sata: ahci@01c18000 {
> +			pwr-supply = <&reg_ahci_5v>;
> +			status = "okay";
> +		};
> +
>  		pinctrl@01c20800 {
> +			ahci_pwr_pin: ahci_pwr_pin@0 {
> +				allwinner,pins = "PB8";
> +				allwinner,function = "gpio_out";
> +				allwinner,drive = <0>;
> +				allwinner,pull = <0>;
> +			};
> +
>  			led_pins_olinuxino: led_pins@0 {
>  				allwinner,pins = "PH2";
>  				allwinner,function = "gpio_out";
> @@ -91,4 +103,18 @@
>  			default-state = "on";
>  		};
>  	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +
> +		reg_ahci_5v: ahci-5v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "ahci-5v";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			pinctrl-0 = <&ahci_pwr_pin>;
> +			gpio = <&pio 1 8 0>;
> +			enable-active-high;
> +		};
> +	};
>  };
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 0552a64..b72c69e 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -368,6 +368,15 @@
>  			};
>  		};
>  
> +		sata: 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";
> +			status = "disabled";
> +		};
> +
>  		timer@01c20c00 {
>  			compatible = "allwinner,sun4i-timer";
>  			reg = <0x01c20c00 0x90>;
> -- 
> 1.8.3.2
> 

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

[Olliver Schinagl]

Hey maxime,
On 06-12-13 19:33, Maxime Ripard wrote:
> Hi Oliver,
>
> On Wed, Dec 04, 2013 at 01:10:55PM +0100, oliver@schinagl.nl wrote:
>> From: Oliver Schinagl <oliver@schinagl.nl>
>>
>> This patch adds sunxi sata support to A10 and A20 boards that have such
>> a connector. Some boards also feature a regulator via a GPIO and support
>> for this is also added.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>
> Your git setup seems to be pretty uncertain about how your first name is spelled :)
I should have formally mention it to confuse less people,

This is how officially my name is spelled (I left out any 'middle' 
letters. I never really used it as such, as it confuses people and they 
always write it wrong anyway. After years I decided that at least on 
these patches, I should write it down properly (googleability etc in the 
future). So formally it's Olliver 'oliver' M. Schinagl.

And no, I won't share my middle name :p

There! :)
>
>> ---
>>   arch/arm/boot/dts/sun4i-a10-cubieboard.dts      | 26 +++++++++++++++++++++++++
>>   arch/arm/boot/dts/sun4i-a10.dtsi                |  9 +++++++++
>>   arch/arm/boot/dts/sun7i-a20-cubieboard2.dts     | 26 +++++++++++++++++++++++++
>>   arch/arm/boot/dts/sun7i-a20-cubietruck.dts      | 26 +++++++++++++++++++++++++
>>   arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 26 +++++++++++++++++++++++++
>>   arch/arm/boot/dts/sun7i-a20.dtsi                |  9 +++++++++
>>   6 files changed, 122 insertions(+)
>
> Could you split this into several patches please?
Yes, appologies, will take care of this! Sorry,

Oliver
>
> At least one per SoC.
>
>> diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> index 425a7db..b620084 100644
>> --- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> +++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> @@ -42,7 +42,18 @@
>>   			};
>>   		};
>>
>> +		sata: ahci@01c18000 {
>> +			pwr-supply = <&reg_ahci_5v>;
>> +			status = "okay";
>> +		};
>> +
>>   		pinctrl@01c20800 {
>> +			ahci_pwr_pin: ahci_pwr_pin@0 {
>
> Please prefix it with name of the board.
>
>> +				allwinner,pins = "PB8";
>> +				allwinner,function = "gpio_out";
>> +				allwinner,driver = <0>;
>> +				allwinner,pull = <0>;
>> +			};
>
> Please add a newline here.
>
>>   			led_pins_cubieboard: led_pins@0 {
>>   				allwinner,pins = "PH20", "PH21";
>>   				allwinner,function = "gpio_out";
>> @@ -86,4 +97,19 @@
>>   			linux,default-trigger = "heartbeat";
>>   		};
>>   	};
>> +
>> +	regulators {
>> +		compatible = "simple-bus";
>> +		pinctrl-names = "default";
>> +
>> +		reg_ahci_5v: ahci-5v {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "ahci-5v";
>> +			regulator-min-microvolt = <5000000>;
>> +			regulator-max-microvolt = <5000000>;
>> +			pinctrl-0 = <&ahci_pwr_pin>;
>> +			gpio = <&pio 1 8 0>;
>> +			enable-active-high;
>> +		};
>> +	};
>>   };
>> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
>> index 4dccdb0..53c6cdb 100644
>> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
>> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
>> @@ -306,6 +306,15 @@
>>   			#size-cells = <0>;
>>   		};
>>
>> +		sata: ahci@01c18000 {
>> +			compatible = "allwinner,sun4i-a10-ahci";
>
> Please use sun4i-ahci for consistency.
>
>> +			reg = <0x01c18000 0x1000>;
>> +			interrupts = <0 56 1>;
>
> The interrupt here doesn't seem right. Is it actually working at all?
>
>> +			clocks = <&ahb_gates 25>, <&pll6 0>;
>> +			clock-names = "ahb_sata", "pll6_sata";
>> +			status = "disabled";
>> +		};
>> +
>>   		intc: interrupt-controller@01c20400 {
>>   			compatible = "allwinner,sun4i-ic";
>>   			reg = <0x01c20400 0x400>;
>> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>> index 5c51cb8..99c5e78 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>> @@ -34,7 +34,18 @@
>>   			};
>>   		};
>>
>> +		sata: ahci@01c18000 {
>> +			pwr-supply = <&reg_ahci_5v>;
>> +			status = "okay";
>> +		};
>> +
>>   		pinctrl@01c20800 {
>> +			ahci_pwr_pin: ahci_pwr_pin@0 {
>> +				allwinner,pins = "PB8";
>> +				allwinner,function = "gpio_out";
>> +				allwinner,drive = <0>;
>> +				allwinner,pull = <0>;
>> +			};
>
> Please add a newline here.
>
>>   			led_pins_cubieboard2: led_pins@0 {
>>   				allwinner,pins = "PH20", "PH21";
>>   				allwinner,function = "gpio_out";
>> @@ -77,4 +88,19 @@
>>   			gpios = <&pio 7 20 0>;
>>   		};
>>   	};
>> +
>> +	regulators {
>> +		compatible = "simple-bus";
>> +		pinctrl-names = "default";
>> +
>> +		reg_ahci_5v: ahci-5v {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "ahci-5v";
>> +			regulator-min-microvolt = <5000000>;
>> +			regulator-max-microvolt = <5000000>;
>> +			pinctrl-0 = <&ahci_pwr_pin>;
>> +			gpio = <&pio 1 8 0>;
>> +			enable-active-high;
>> +		};
>> +	};
>>   };
>> diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> index 8a1009d..19af23e 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> @@ -19,7 +19,18 @@
>>   	compatible = "cubietech,cubietruck", "allwinner,sun7i-a20";
>>
>>   	soc@01c00000 {
>> +		sata: ahci@01c18000 {
>> +			pwr-supply = <&reg_ahci_5v>;
>> +			status = "okay";
>> +		};
>> +
>>   		pinctrl@01c20800 {
>> +			ahci_pwr_pin: ahci_pwr_pin@0 {
>> +				allwinner,pins = "PH12";
>> +				allwinner,function = "gpio_out";
>> +				allwinner,driver = <0>;
>> +				allwinner,pull = <0>;
>> +			};
>
> Please add a newline here.
>
>>   			led_pins_cubietruck: led_pins@0 {
>>   				allwinner,pins = "PH7", "PH11", "PH20", "PH21";
>>   				allwinner,function = "gpio_out";
>> @@ -60,4 +71,19 @@
>>   			gpios = <&pio 7 7 0>;
>>   		};
>>   	};
>> +
>> +	regulators {
>> +		compatible = "simple-bus";
>> +		pinctrl-names = "default";
>> +
>> +		reg_ahci_5v: ahci-5v {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "ahci-5v";
>> +			regulator-min-microvolt = <5000000>;
>> +			regulator-max-microvolt = <5000000>;
>> +			pinctrl-0 = <&ahci_pwr_pin>;
>> +			gpio = <&pio 7 12 0>;
>> +			enable-active-high;
>> +		};
>> +	};
>>   };
>> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>> index ead3013..23ed708 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>> @@ -34,7 +34,19 @@
>>   			};
>>   		};
>>
>> +		sata: ahci@01c18000 {
>> +			pwr-supply = <&reg_ahci_5v>;
>> +			status = "okay";
>> +		};
>> +
>>   		pinctrl@01c20800 {
>> +			ahci_pwr_pin: ahci_pwr_pin@0 {
>> +				allwinner,pins = "PB8";
>> +				allwinner,function = "gpio_out";
>> +				allwinner,drive = <0>;
>> +				allwinner,pull = <0>;
>> +			};
>> +
>>   			led_pins_olinuxino: led_pins@0 {
>>   				allwinner,pins = "PH2";
>>   				allwinner,function = "gpio_out";
>> @@ -91,4 +103,18 @@
>>   			default-state = "on";
>>   		};
>>   	};
>> +
>> +	regulators {
>> +		compatible = "simple-bus";
>> +
>> +		reg_ahci_5v: ahci-5v {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "ahci-5v";
>> +			regulator-min-microvolt = <5000000>;
>> +			regulator-max-microvolt = <5000000>;
>> +			pinctrl-0 = <&ahci_pwr_pin>;
>> +			gpio = <&pio 1 8 0>;
>> +			enable-active-high;
>> +		};
>> +	};
>>   };
>> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
>> index 0552a64..b72c69e 100644
>> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
>> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
>> @@ -368,6 +368,15 @@
>>   			};
>>   		};
>>
>> +		sata: ahci@01c18000 {
>> +			compatible = "allwinner,sun4i-a10-ahci";
>> +			reg = <0x01c18000 0x1000>;
>> +			interrupts = <0 56 1>;
Will always fix this to <0 56 4> if i'm not mistaken.

oliver
>> +			clocks = <&ahb_gates 25>, <&pll6 0>;
>> +			clock-names = "ahb_sata", "pll6_sata";
>> +			status = "disabled";
>> +		};
>> +
>>   		timer@01c20c00 {
>>   			compatible = "allwinner,sun4i-timer";
>>   			reg = <0x01c20c00 0x90>;
>> --
>> 1.8.3.2
>>
>
> Thanks,
> Maxime
>

Re: [linux-sunxi] Re: [PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

[Michal Suchanek]

On 7 December 2013 12:47, Olliver Schinagl <oliver+list@schinagl.nl> wrote:
> Hey maxime,
>
> On 06-12-13 19:33, Maxime Ripard wrote:
>>
>> Hi Oliver,
>>
>> On Wed, Dec 04, 2013 at 01:10:55PM +0100, oliver@schinagl.nl wrote:
>>>
>>> From: Oliver Schinagl <oliver@schinagl.nl>
>>>
>>> This patch adds sunxi sata support to A10 and A20 boards that have such
>>> a connector. Some boards also feature a regulator via a GPIO and support
>>> for this is also added.
>>>
>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>
>>
>> Your git setup seems to be pretty uncertain about how your first name is
>> spelled :)
>
> I should have formally mention it to confuse less people,
>
> This is how officially my name is spelled (I left out any 'middle' letters.
> I never really used it as such, as it confuses people and they always write
> it wrong anyway. After years I decided that at least on these patches, I
> should write it down properly (googleability etc in the future). So formally
> it's Olliver 'oliver' M. Schinagl.
>
> And no, I won't share my middle name :p
>
> There! :)
>
>>
>>> ---
>>>   arch/arm/boot/dts/sun4i-a10-cubieboard.dts      | 26
>>> +++++++++++++++++++++++++
>>>   arch/arm/boot/dts/sun4i-a10.dtsi                |  9 +++++++++
>>>   arch/arm/boot/dts/sun7i-a20-cubieboard2.dts     | 26
>>> +++++++++++++++++++++++++
>>>   arch/arm/boot/dts/sun7i-a20-cubietruck.dts      | 26
>>> +++++++++++++++++++++++++
>>>   arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 26
>>> +++++++++++++++++++++++++
>>>   arch/arm/boot/dts/sun7i-a20.dtsi                |  9 +++++++++
>>>   6 files changed, 122 insertions(+)
>>
>>
>> Could you split this into several patches please?
>
> Yes, appologies, will take care of this! Sorry,
>
> Oliver
>
>>
>> At least one per SoC.
>>
>>> diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>>> b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>>> index 425a7db..b620084 100644
>>> --- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>>> +++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>>> @@ -42,7 +42,18 @@
>>>                         };
>>>                 };
>>>
>>> +               sata: ahci@01c18000 {
>>> +                       pwr-supply = <&reg_ahci_5v>;
>>> +                       status = "okay";
>>> +               };
>>> +
>>>                 pinctrl@01c20800 {
>>> +                       ahci_pwr_pin: ahci_pwr_pin@0 {
>>
>>
>> Please prefix it with name of the board.
>>
>>> +                               allwinner,pins = "PB8";
>>> +                               allwinner,function = "gpio_out";
>>> +                               allwinner,driver = <0>;
>>> +                               allwinner,pull = <0>;
>>> +                       };
>>
>>
>> Please add a newline here.
>>
>>>                         led_pins_cubieboard: led_pins@0 {
>>>                                 allwinner,pins = "PH20", "PH21";
>>>                                 allwinner,function = "gpio_out";
>>> @@ -86,4 +97,19 @@
>>>                         linux,default-trigger = "heartbeat";
>>>                 };
>>>         };
>>> +
>>> +       regulators {
>>> +               compatible = "simple-bus";
>>> +               pinctrl-names = "default";
>>> +
>>> +               reg_ahci_5v: ahci-5v {
>>> +                       compatible = "regulator-fixed";
>>> +                       regulator-name = "ahci-5v";
>>> +                       regulator-min-microvolt = <5000000>;
>>> +                       regulator-max-microvolt = <5000000>;
>>> +                       pinctrl-0 = <&ahci_pwr_pin>;
>>> +                       gpio = <&pio 1 8 0>;
>>> +                       enable-active-high;
>>> +               };
>>> +       };
>>>   };
>>> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
>>> b/arch/arm/boot/dts/sun4i-a10.dtsi
>>> index 4dccdb0..53c6cdb 100644
>>> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
>>> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
>>> @@ -306,6 +306,15 @@
>>>                         #size-cells = <0>;
>>>                 };
>>>
>>> +               sata: ahci@01c18000 {
>>> +                       compatible = "allwinner,sun4i-a10-ahci";
>>
>>
>> Please use sun4i-ahci for consistency.
>>
>>> +                       reg = <0x01c18000 0x1000>;
>>> +                       interrupts = <0 56 1>;
>>
>>
>> The interrupt here doesn't seem right. Is it actually working at all?
>>
>>> +                       clocks = <&ahb_gates 25>, <&pll6 0>;
>>> +                       clock-names = "ahb_sata", "pll6_sata";
>>> +                       status = "disabled";
>>> +               };
>>> +
>>>                 intc: interrupt-controller@01c20400 {
>>>                         compatible = "allwinner,sun4i-ic";
>>>                         reg = <0x01c20400 0x400>;
>>> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>>> b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>>> index 5c51cb8..99c5e78 100644
>>> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>>> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>>> @@ -34,7 +34,18 @@
>>>                         };
>>>                 };
>>>
>>> +               sata: ahci@01c18000 {
>>> +                       pwr-supply = <&reg_ahci_5v>;
>>> +                       status = "okay";
>>> +               };
>>> +
>>>                 pinctrl@01c20800 {
>>> +                       ahci_pwr_pin: ahci_pwr_pin@0 {
>>> +                               allwinner,pins = "PB8";
>>> +                               allwinner,function = "gpio_out";
>>> +                               allwinner,drive = <0>;
>>> +                               allwinner,pull = <0>;
>>> +                       };
>>
>>
>> Please add a newline here.
>>
>>>                         led_pins_cubieboard2: led_pins@0 {
>>>                                 allwinner,pins = "PH20", "PH21";
>>>                                 allwinner,function = "gpio_out";
>>> @@ -77,4 +88,19 @@
>>>                         gpios = <&pio 7 20 0>;
>>>                 };
>>>         };
>>> +
>>> +       regulators {
>>> +               compatible = "simple-bus";
>>> +               pinctrl-names = "default";
>>> +
>>> +               reg_ahci_5v: ahci-5v {
>>> +                       compatible = "regulator-fixed";
>>> +                       regulator-name = "ahci-5v";
>>> +                       regulator-min-microvolt = <5000000>;
>>> +                       regulator-max-microvolt = <5000000>;
>>> +                       pinctrl-0 = <&ahci_pwr_pin>;
>>> +                       gpio = <&pio 1 8 0>;
>>> +                       enable-active-high;
>>> +               };
>>> +       };
>>>   };
>>> diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>>> b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>>> index 8a1009d..19af23e 100644
>>> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>>> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>>> @@ -19,7 +19,18 @@
>>>         compatible = "cubietech,cubietruck", "allwinner,sun7i-a20";
>>>
>>>         soc@01c00000 {
>>> +               sata: ahci@01c18000 {
>>> +                       pwr-supply = <&reg_ahci_5v>;
>>> +                       status = "okay";
>>> +               };
>>> +
>>>                 pinctrl@01c20800 {
>>> +                       ahci_pwr_pin: ahci_pwr_pin@0 {
>>> +                               allwinner,pins = "PH12";
>>> +                               allwinner,function = "gpio_out";
>>> +                               allwinner,driver = <0>;
>>> +                               allwinner,pull = <0>;
>>> +                       };
>>
>>
>> Please add a newline here.
>>
>>>                         led_pins_cubietruck: led_pins@0 {
>>>                                 allwinner,pins = "PH7", "PH11", "PH20",
>>> "PH21";
>>>                                 allwinner,function = "gpio_out";
>>> @@ -60,4 +71,19 @@
>>>                         gpios = <&pio 7 7 0>;
>>>                 };
>>>         };
>>> +
>>> +       regulators {
>>> +               compatible = "simple-bus";
>>> +               pinctrl-names = "default";
>>> +
>>> +               reg_ahci_5v: ahci-5v {
>>> +                       compatible = "regulator-fixed";
>>> +                       regulator-name = "ahci-5v";
>>> +                       regulator-min-microvolt = <5000000>;
>>> +                       regulator-max-microvolt = <5000000>;
>>> +                       pinctrl-0 = <&ahci_pwr_pin>;
>>> +                       gpio = <&pio 7 12 0>;
>>> +                       enable-active-high;
>>> +               };
>>> +       };
>>>   };
>>> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>>> b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>>> index ead3013..23ed708 100644
>>> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>>> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
>>> @@ -34,7 +34,19 @@
>>>                         };
>>>                 };
>>>
>>> +               sata: ahci@01c18000 {
>>> +                       pwr-supply = <&reg_ahci_5v>;
>>> +                       status = "okay";
>>> +               };
>>> +
>>>                 pinctrl@01c20800 {
>>> +                       ahci_pwr_pin: ahci_pwr_pin@0 {
>>> +                               allwinner,pins = "PB8";
>>> +                               allwinner,function = "gpio_out";
>>> +                               allwinner,drive = <0>;
>>> +                               allwinner,pull = <0>;
>>> +                       };
>>> +
>>>                         led_pins_olinuxino: led_pins@0 {
>>>                                 allwinner,pins = "PH2";
>>>                                 allwinner,function = "gpio_out";
>>> @@ -91,4 +103,18 @@
>>>                         default-state = "on";
>>>                 };
>>>         };
>>> +
>>> +       regulators {
>>> +               compatible = "simple-bus";
>>> +
>>> +               reg_ahci_5v: ahci-5v {
>>> +                       compatible = "regulator-fixed";
>>> +                       regulator-name = "ahci-5v";
>>> +                       regulator-min-microvolt = <5000000>;
>>> +                       regulator-max-microvolt = <5000000>;
>>> +                       pinctrl-0 = <&ahci_pwr_pin>;
>>> +                       gpio = <&pio 1 8 0>;
>>> +                       enable-active-high;
>>> +               };
>>> +       };
>>>   };
>>> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
>>> b/arch/arm/boot/dts/sun7i-a20.dtsi
>>> index 0552a64..b72c69e 100644
>>> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
>>> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
>>> @@ -368,6 +368,15 @@
>>>                         };
>>>                 };
>>>
>>> +               sata: ahci@01c18000 {
>>> +                       compatible = "allwinner,sun4i-a10-ahci";
>>> +                       reg = <0x01c18000 0x1000>;
>>> +                       interrupts = <0 56 1>;
>
> Will always fix this to <0 56 4> if i'm not mistaken.

<0 56 1> appears to work on sun7i but sun4i has <56> in the working
DT. The DT you sent has <0 56 1> on sun4i which somehow compiles and
loads as DT but the driver fails.

Thanks

Michal

Re: [PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Sat, Dec 07, 2013 at 12:47:39PM +0100, Olliver Schinagl wrote:
> Hey maxime,
> On 06-12-13 19:33, Maxime Ripard wrote:
> >Hi Oliver,
> >
> >On Wed, Dec 04, 2013 at 01:10:55PM +0100, oliver@schinagl.nl wrote:
> >>From: Oliver Schinagl <oliver@schinagl.nl>
> >>
> >>This patch adds sunxi sata support to A10 and A20 boards that have such
> >>a connector. Some boards also feature a regulator via a GPIO and support
> >>for this is also added.
> >>
> >>Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> >
> >Your git setup seems to be pretty uncertain about how your first name is spelled :)
> I should have formally mention it to confuse less people,
> 
> This is how officially my name is spelled (I left out any 'middle'
> letters. I never really used it as such, as it confuses people and
> they always write it wrong anyway. After years I decided that at
> least on these patches, I should write it down properly
> (googleability etc in the future). So formally it's Olliver 'oliver'
> M. Schinagl.
> 
> And no, I won't share my middle name :p
> 
> There! :)

My point was more that the spelling in your From is different from the
spelling in your SoB.

> >>diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> >>index 0552a64..b72c69e 100644
> >>--- a/arch/arm/boot/dts/sun7i-a20.dtsi
> >>+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> >>@@ -368,6 +368,15 @@
> >>  			};
> >>  		};
> >>
> >>+		sata: ahci@01c18000 {
> >>+			compatible = "allwinner,sun4i-a10-ahci";
> >>+			reg = <0x01c18000 0x1000>;
> >>+			interrupts = <0 56 1>;
> Will always fix this to <0 56 4> if i'm not mistaken.

Only the GIC has a three-cells interrupts. So only the A20 and A31 are
supposed to have this value.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/3] AHCI: sunxi: Add sunxi AHCI driver

[Olliver Schinagl]

And I've spotted my first mistake (one of many I'm sure) I typo'ed Rob's 
e-mail address. Sorry Rob.

Oliver

On 04-12-13 13:10, oliver@schinagl.nl wrote:
> From: Olliver Schinagl <oliver@schinagl.nl>
>
> This patch series adds Allwinner sunxi support to platform_ahci.
> There are a few quirks here however that will need discussing.
>
> Allwinner added something to setup the DMA engine prior to enabling DMA in
> libahci.c. I'm not familiar at all with the AHCI spec and not sure what
> the proper way to implement this would have been. Don't other platforms
> optionally require the same thing?
>
> Also the way ahci_platform works seems a little hack-ish, it seems like
> something along the lines of ahci_platform_register(ahci-data) would be more
> appropriate, but not sure yet on those details (I am just a beginner).
>
> Furthermore this driver should, in theory, work on the A10 but on my
> cubieboard 1 it does not work yet (just hangs somewhere in libahci or after,
> but not the DMA call that was altered). I've also tested it the cubietruck,
> cubieboard 2 and olinuxino and it worked fine there.
>
> Thanks,
>
> Oliver
>
> Oliver Schinagl (3):
>    RFC: AHCI: libahci is missing DMA
>    ARM: sunxi: Add an ahci-platform compatible AHCI driver for the
>      Allwinner SUNXi series of SoCs
>    ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards
>
>   .../devicetree/bindings/ata/ahci-sunxi.txt         |  24 ++
>   arch/arm/boot/dts/sun4i-a10-cubieboard.dts         |  26 ++
>   arch/arm/boot/dts/sun4i-a10.dtsi                   |   9 +
>   arch/arm/boot/dts/sun7i-a20-cubieboard2.dts        |  26 ++
>   arch/arm/boot/dts/sun7i-a20-cubietruck.dts         |  26 ++
>   arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts    |  26 ++
>   arch/arm/boot/dts/sun7i-a20.dtsi                   |   9 +
>   drivers/ata/Kconfig                                |   9 +
>   drivers/ata/Makefile                               |   1 +
>   drivers/ata/ahci.h                                 |   6 +
>   drivers/ata/ahci_platform.c                        |  12 +
>   drivers/ata/ahci_sunxi.c                           | 305 +++++++++++++++++++++
>   drivers/ata/libahci.c                              |   8 +
>   13 files changed, 487 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/ata/ahci-sunxi.txt
>   create mode 100644 drivers/ata/ahci_sunxi.c
>