[PATCH v2 0/7] Serial ATA support for NVIDIA Tegra124

[PATCH v2 0/7] Serial ATA support for NVIDIA Tegra124

[Mikko Perttunen]

Hi,

This series adds support for the onboard AHCI-compliant Serial ATA
controller found on Tegra124 systems-on-chip. The series depends on
Thierry's XUSB pinctrl driver.

A branch containing the series is located at 
	git://github.com/cyndis/linux.git
branch ahci-rel-v2.

Mikko Perttunen (7):
  of: Add NVIDIA Tegra SATA controller binding
  ARM: tegra: Add SATA controller to Tegra124 device tree
  ARM: tegra: Add SATA and SATA power to Jetson TK1 device tree
  clk: tegra: Enable hardware control of SATA PLL
  clk: tegra: Add SATA clocks to Tegra124 initialization table
  ata: Add support for the Tegra124 SATA controller
  ARM: tegra: Add options for Tegra AHCI support to tegra_defconfig

 .../devicetree/bindings/ata/tegra-sata.txt         |  30 ++
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |  35 ++
 arch/arm/boot/dts/tegra124.dtsi                    |  25 ++
 arch/arm/configs/tegra_defconfig                   |   3 +
 drivers/ata/Kconfig                                |   9 +
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci_tegra.c                           | 448 +++++++++++++++++++++
 drivers/clk/tegra/clk-pll.c                        |   8 +
 drivers/clk/tegra/clk-tegra124.c                   |   2 +
 9 files changed, 561 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/tegra-sata.txt
 create mode 100644 drivers/ata/ahci_tegra.c

-- 
1.8.1.5

[PATCH v2 1/7] of: Add NVIDIA Tegra SATA controller binding

[Mikko Perttunen]

This patch adds device tree binding documentation for the SATA
controller found on NVIDIA Tegra SoCs.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v2:
- added target-5v-supply and target-12v-supply
- changed wording to be similar with other Tegra drivers

 .../devicetree/bindings/ata/tegra-sata.txt         | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/tegra-sata.txt

diff --git a/Documentation/devicetree/bindings/ata/tegra-sata.txt b/Documentation/devicetree/bindings/ata/tegra-sata.txt
new file mode 100644
index 0000000..946f207
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/tegra-sata.txt
@@ -0,0 +1,30 @@
+Tegra124 SoC SATA AHCI controller
+
+Required properties :
+- compatible : "nvidia,tegra124-ahci".
+- reg : Should contain 2 entries:
+  - AHCI register set (SATA BAR5)
+  - SATA register set
+- interrupts : Defines the interrupt used by SATA
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+  - sata
+  - sata-oob
+  - cml1
+  - pll_e
+- resets : Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names : Must include the following entries:
+  - sata
+  - sata-oob
+  - sata-cold
+- phys : Must contain an entry for each entry in phy-names.
+  See ../phy/phy-bindings.txt for details.
+- phy-names : Must include the following entries:
+  - sata-phy : XUSB PADCTL SATA PHY
+- hvdd-supply : Defines the SATA HVDD regulator
+- vddio-supply : Defines the SATA VDDIO regulator
+- avdd-supply : Defines the SATA AVDD regulator
+- target-5v-supply : Defines the SATA 5V power regulator
+- target-12v-supply : Defines the SATA 12V power regulator
-- 
1.8.1.5

[PATCH v2 2/7] ARM: tegra: Add SATA controller to Tegra124 device tree

[Mikko Perttunen]

This adds the integrated AHCI-compliant Serial ATA controller present
in Tegra124 systems-on-chip to the Tegra124 device tree.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v2:
- added clock-names property
- changed 0 -> GIC_SPI

 arch/arm/boot/dts/tegra124.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 6fa06d2..63a2bfa 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -734,6 +734,31 @@
 		status = "disabled";
 	};
 
+	sata at 0,70020000 {
+		compatible = "nvidia,tegra124-ahci";
+
+		reg = <0x0 0x70027000 0x0 0x2000>, /* AHCI */
+			<0x0 0x70020000 0x0 0x7000>; /* SATA */
+
+		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
+
+		clocks = <&tegra_car TEGRA124_CLK_SATA>,
+			<&tegra_car TEGRA124_CLK_SATA_OOB>,
+			<&tegra_car TEGRA124_CLK_CML1>,
+			<&tegra_car TEGRA124_CLK_PLL_E>;
+		clock-names = "sata", "sata-oob", "cml1", "pll_e";
+
+		resets = <&tegra_car 124>,
+			<&tegra_car 123>,
+			<&tegra_car 129>;
+		reset-names = "sata", "sata-oob", "sata-cold";
+
+		phys = <&padctl TEGRA_XUSB_PADCTL_SATA>;
+		phy-names = "sata-phy";
+
+		status = "disabled";
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
1.8.1.5

[PATCH v2 3/7] ARM: tegra: Add SATA and SATA power to Jetson TK1 device tree

[Mikko Perttunen]

This enables the integrated SATA controller on the Tegra124 system-on-chip
on the Jetson TK1 board and adds regulators for the onboard Molex connector
commonly used to power SATA devices. The regulators are marked always-on
since they can be used for other purposes than powering SATA devices.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v2:
- added target-v5-supply and target-12v-supply
- made sata power not always-on

 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 35 +++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 11c1650..00ba373 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -1687,6 +1687,18 @@
 		vbus-supply = <&vdd_usb3_vbus>;
 	};
 
+	/* Serial ATA */
+	sata at 0,70020000 {
+		status = "okay";
+
+		hvdd-supply = <&vdd_3v3_lp0>;
+		vddio-supply = <&vdd_1v05_run>;
+		avdd-supply = <&vdd_1v05_run>;
+
+		target-5v-supply = <&vdd_5v0_sata>;
+		target-12v-supply = <&vdd_12v0_sata>;
+	};
+
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -1828,6 +1840,29 @@
 			enable-active-high;
 			vin-supply = <&vdd_5v0_sys>;
 		};
+
+		/* Molex power connector */
+		vdd_5v0_sata: regulator at 13 {
+			compatible = "regulator-fixed";
+			reg = <13>;
+			regulator-name = "+5V_SATA";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio TEGRA_GPIO(EE, 2) GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+			vin-supply = <&vdd_5v0_sys>;
+		};
+
+		vdd_12v0_sata: regulator at 14 {
+			compatible = "regulator-fixed";
+			reg = <14>;
+			regulator-name = "+12V_SATA";
+			regulator-min-microvolt = <12000000>;
+			regulator-max-microvolt = <12000000>;
+			gpio = <&gpio TEGRA_GPIO(EE, 2) GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+			vin-supply = <&vdd_mux>;
+		};
 	};
 
 	sound {
-- 
1.8.1.5

[PATCH v2 4/7] clk: tegra: Enable hardware control of SATA PLL

[Mikko Perttunen]

This makes the SATA PLL be controlled by hardware instead of software.
This is required for working SATA support.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/clk/tegra/clk-pll.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 637b62c..f070c36 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -110,6 +110,9 @@
 #define XUSBIO_PLL_CFG0_SEQ_ENABLE		BIT(24)
 #define XUSBIO_PLL_CFG0_SEQ_START_STATE		BIT(25)
 
+#define SATA_PLL_CFG0		0x490
+#define SATA_PLL_CFG0_PADPLL_RESET_SWCTL	BIT(0)
+
 #define PLLE_MISC_PLLE_PTS	BIT(8)
 #define PLLE_MISC_IDDQ_SW_VALUE	BIT(13)
 #define PLLE_MISC_IDDQ_SW_CTRL	BIT(14)
@@ -1361,6 +1364,11 @@ static int clk_plle_tegra114_enable(struct clk_hw *hw)
 	val |= XUSBIO_PLL_CFG0_SEQ_ENABLE;
 	pll_writel(val, XUSBIO_PLL_CFG0, pll);
 
+	/* Enable hw control of SATA pll */
+	val = pll_readl(SATA_PLL_CFG0, pll);
+	val &= ~SATA_PLL_CFG0_PADPLL_RESET_SWCTL;
+	pll_writel(val, SATA_PLL_CFG0, pll);
+
 out:
 	if (pll->lock)
 		spin_unlock_irqrestore(pll->lock, flags);
-- 
1.8.1.5

[PATCH v2 4/7] clk: tegra: Enable hardware control of SATA PLL

[Andrew Bresticker]

On Wed, Jun 18, 2014 at 7:23 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
> This makes the SATA PLL be controlled by hardware instead of software.
> This is required for working SATA support.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> Acked-by: Stephen Warren <swarren@nvidia.com>

I know Peter sent a pull request including this patch already, but I
don't see it yet in Mike's tree, so perhaps it's possible to address
my comment below (or else I'll include it in the next spin of my XUSB
series.

> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c

> @@ -1361,6 +1364,11 @@ static int clk_plle_tegra114_enable(struct clk_hw *hw)
>         val |= XUSBIO_PLL_CFG0_SEQ_ENABLE;
>         pll_writel(val, XUSBIO_PLL_CFG0, pll);
>
> +       /* Enable hw control of SATA pll */
> +       val = pll_readl(SATA_PLL_CFG0, pll);
> +       val &= ~SATA_PLL_CFG0_PADPLL_RESET_SWCTL;
> +       pll_writel(val, SATA_PLL_CFG0, pll);
> +

Apparently the procedure for enabling the SATA PLL for XUSB (when the
SATA lane is used) is slightly different.  Specifically, it would be:

val = pll_readl(SATA_PLL_CFG0, pll);
val &= ~SATA_PLL_CFG0_PADPLL_RESET_SWCTL;
val |= SATA_PLL_CFG0_PADPLL_USE_LOCKDET;
val |= SATA_PLL_CFG0_SEQ_START_STATE;
pll_writel(val, SATA_PLL_CFG0, pll);

udelay(1);

val = pll_readl(SATA_PLL_CFG0, pll);
val |= SATA_PLL_CFG0_SEQ_ENABLE;
pll_writel(val, SATA_PLL_CFG0, pll);

Do you know if this sequence also works when the SATA lane is used for SATA?

[PATCH] clk: tegra: Use XUSB-compatible SATA PLL sequence

[Mikko Perttunen]

Use a sequence for enabling hardware control of the SATA PLL
that works both when using the SATA lane with SATA and when
using it with XUSB.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
Andrew: yes, it does :)
Peter: Feel free to squash if that works for you.

 drivers/clk/tegra/clk-pll.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index f070c36..c7c6d8f 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -112,6 +112,9 @@
 
 #define SATA_PLL_CFG0		0x490
 #define SATA_PLL_CFG0_PADPLL_RESET_SWCTL	BIT(0)
+#define SATA_PLL_CFG0_PADPLL_USE_LOCKDET	BIT(2)
+#define SATA_PLL_CFG0_SEQ_ENABLE		BIT(24)
+#define SATA_PLL_CFG0_SEQ_START_STATE		BIT(25)
 
 #define PLLE_MISC_PLLE_PTS	BIT(8)
 #define PLLE_MISC_IDDQ_SW_VALUE	BIT(13)
@@ -1367,6 +1370,14 @@ static int clk_plle_tegra114_enable(struct clk_hw *hw)
 	/* Enable hw control of SATA pll */
 	val = pll_readl(SATA_PLL_CFG0, pll);
 	val &= ~SATA_PLL_CFG0_PADPLL_RESET_SWCTL;
+	val |= SATA_PLL_CFG0_PADPLL_USE_LOCKDET;
+	val |= SATA_PLL_CFG0_SEQ_START_STATE;
+	pll_writel(val, SATA_PLL_CFG0, pll);
+
+	udelay(1);
+
+	val = pll_readl(SATA_PLL_CFG0, pll);
+	val |= SATA_PLL_CFG0_SEQ_ENABLE;
 	pll_writel(val, SATA_PLL_CFG0, pll);
 
 out:
-- 
1.8.1.5

[PATCH] clk: tegra: Use XUSB-compatible SATA PLL sequence

[Peter De Schrijver]

On Tue, Jul 08, 2014 at 09:30:15AM +0200, Mikko Perttunen wrote:
> Use a sequence for enabling hardware control of the SATA PLL
> that works both when using the SATA lane with SATA and when
> using it with XUSB.
> 

I'm going to take it as a separate patch on top of what's there now.

Cheers,

Peter.

[PATCH v2 5/7] clk: tegra: Add SATA clocks to Tegra124 initialization table

[Mikko Perttunen]

This adds two clocks, SATA and SATA_OOB, to the Tegra124 clock initialization
table. The clocks are needed for working SATA support.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 80efe51..6770787 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1369,6 +1369,8 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{TEGRA124_CLK_XUSB_HS_SRC, TEGRA124_CLK_PLL_U_60M, 60000000, 0},
 	{TEGRA124_CLK_XUSB_FALCON_SRC, TEGRA124_CLK_PLL_RE_OUT, 224000000, 0},
 	{TEGRA124_CLK_XUSB_HOST_SRC, TEGRA124_CLK_PLL_RE_OUT, 112000000, 0},
+	{TEGRA124_CLK_SATA, TEGRA124_CLK_PLL_P, 104000000, 0},
+	{TEGRA124_CLK_SATA_OOB, TEGRA124_CLK_PLL_P, 204000000, 0},
 	/* This MUST be the last entry. */
 	{TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0},
 };
-- 
1.8.1.5

[PATCH v2 5/7] clk: tegra: Add SATA clocks to Tegra124 initialization table

[Stephen Warren]

On 06/18/2014 08:23 AM, Mikko Perttunen wrote:
> This adds two clocks, SATA and SATA_OOB, to the Tegra124 clock initialization
> table. The clocks are needed for working SATA support.

Acked-by: Stephen Warren <swarren@nvidia.com>
(When I wrote that for v1, it applied to both patches 4 and 5, not just
patch 4)

Peter, can you please pick up patches 4 and 5, and get them into
linux-next. Thanks.

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

This adds support for the integrated AHCI-compliant Serial ATA
controller present on the NVIDIA Tegra124 system-on-chip.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v2:
- removed redundant of_match_device
- fixed fuse offsets
- added newlines to dev_err calls
- get 5v/12v supplies
- mask calibration fuse value
- removed sata io helpers
- changed defines to use BIT macro
- removed use of ahci_platform resource management
- changed powergate code to use sequence function

 drivers/ata/Kconfig      |   9 +
 drivers/ata/Makefile     |   1 +
 drivers/ata/ahci_tegra.c | 448 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 458 insertions(+)
 create mode 100644 drivers/ata/ahci_tegra.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index b0d5b5a..e1b9278 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -142,6 +142,15 @@ config AHCI_SUNXI
 
 	  If unsure, say N.
 
+config AHCI_TEGRA
+	tristate "NVIDIA Tegra124 AHCI SATA support"
+	depends on ARCH_TEGRA
+	help
+	  This option enables support for the NVIDIA Tegra124 SoC's
+	  onboard AHCI SATA.
+
+	  If unsure, say N.
+
 config AHCI_XGENE
 	tristate "APM X-Gene 6.0Gbps AHCI SATA host controller support"
 	depends on PHY_XGENE
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 5a02aee..ae41107 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_MVEBU)	+= ahci_mvebu.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_SUNXI)	+= ahci_sunxi.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_ST)		+= ahci_st.o libahci.o libahci_platform.o
+obj-$(CONFIG_AHCI_TEGRA)	+= ahci_tegra.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_XGENE)	+= ahci_xgene.o libahci.o libahci_platform.o
 
 # SFF w/ custom DMA
diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
new file mode 100644
index 0000000..f4f8f33
--- /dev/null
+++ b/drivers/ata/ahci_tegra.c
@@ -0,0 +1,448 @@
+/*
+ * drivers/ata/ahci_tegra.c
+ *
+ * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author:
+ *	Mikko Perttunen <mperttunen@nvidia.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/ahci_platform.h>
+#include <linux/reset.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/tegra-powergate.h>
+#include <linux/regulator/consumer.h>
+#include <linux/tegra-soc.h>
+#include "ahci.h"
+
+#define SATA_CONFIGURATION_0				0x180
+#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)
+
+#define SCFG_OFFSET					0x1000
+
+#define T_SATA0_CFG_1					0x04
+#define		T_SATA0_CFG_1_IO_SPACE			BIT(0)
+#define		T_SATA0_CFG_1_MEMORY_SPACE		BIT(1)
+#define		T_SATA0_CFG_1_BUS_MASTER		BIT(2)
+#define		T_SATA0_CFG_1_SERR			BIT(8)
+
+#define T_SATA0_CFG_9					0x24
+#define		T_SATA0_CFG_9_BASE_ADDRESS_SHIFT	13
+
+#define SATA_FPCI_BAR5					0x94
+#define		SATA_FPCI_BAR5_START_SHIFT		4
+
+#define SATA_INTR_MASK					0x188
+#define		SATA_INTR_MASK_IP_INT_MASK		BIT(16)
+
+#define T_SATA0_AHCI_HBA_CAP_BKDR			0x300
+
+#define T_SATA0_BKDOOR_CC				0x4a4
+
+#define T_SATA0_CFG_SATA				0x54c
+#define		T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN	BIT(12)
+
+#define T_SATA0_CFG_MISC				0x550
+
+#define T_SATA0_INDEX					0x680
+
+#define T_SATA0_CHX_PHY_CTRL1_GEN1			0x690
+#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK	0xff
+#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT	0
+#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK	(0xff<<8)
+#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT 8
+
+#define T_SATA0_CHX_PHY_CTRL1_GEN2			0x694
+#define		T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK	0xff
+#define		T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_SHIFT	0
+#define		T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK	(0xff<<12)
+#define		T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_SHIFT 12
+
+#define T_SATA0_CHX_PHY_CTRL2				0x69c
+#define		T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1	0x23
+
+#define T_SATA0_CHX_PHY_CTRL11				0x6d0
+#define		T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ	(0x2800<<16)
+
+#define FUSE_SATA_CALIB					0x124
+#define		FUSE_SATA_CALIB_MASK			0x3
+
+struct sata_pad_calibration {
+	u8 gen1_tx_amp, gen1_tx_peak, gen2_tx_amp, gen2_tx_peak;
+};
+
+static const struct sata_pad_calibration tegra124_pad_calibration[] = {
+	{0x18, 0x04, 0x18, 0x0a},
+	{0x0e, 0x04, 0x14, 0x0a},
+	{0x0e, 0x07, 0x1a, 0x0e},
+	{0x14, 0x0e, 0x1a, 0x0e},
+};
+
+struct tegra_ahci_priv {
+	struct platform_device *pdev;
+	void __iomem *sata_regs;
+	struct reset_control *sata_rst;
+	struct reset_control *sata_oob_rst;
+	struct reset_control *sata_cold_rst;
+	struct clk *sata_clk;
+	struct clk *sata_oob_clk;
+	struct clk *cml1_clk;
+	struct clk *plle_clk;
+	struct regulator_bulk_data supplies[5];
+	struct phy *padctl_phy;
+};
+
+static int tegra_ahci_power_on(struct ahci_host_priv *hpriv)
+{
+	struct tegra_ahci_priv *tegra = hpriv->plat_data;
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
+				    tegra->supplies);
+	if (ret)
+		return ret;
+
+	ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
+		tegra->sata_clk, tegra->sata_rst);
+	if (ret)
+		goto disable_regulators;
+
+	reset_control_assert(tegra->sata_oob_rst);
+	reset_control_assert(tegra->sata_cold_rst);
+
+	ret = clk_prepare_enable(tegra->sata_oob_clk);
+	if (ret)
+		goto disable_power;
+
+	ret = clk_prepare_enable(tegra->cml1_clk);
+	if (ret) {
+		clk_disable_unprepare(tegra->sata_oob_clk);
+		goto disable_power;
+	}
+
+	ret = clk_prepare_enable(tegra->plle_clk);
+	if (ret) {
+		clk_disable_unprepare(tegra->cml1_clk);
+		clk_disable_unprepare(tegra->sata_oob_clk);
+		goto disable_power;
+	}
+
+	reset_control_deassert(tegra->sata_cold_rst);
+	reset_control_deassert(tegra->sata_oob_rst);
+
+	ret = phy_power_on(tegra->padctl_phy);
+	if (ret) {
+		clk_disable_unprepare(tegra->plle_clk);
+		clk_disable_unprepare(tegra->cml1_clk);
+		clk_disable_unprepare(tegra->sata_oob_clk);
+		goto disable_power;
+	}
+
+	return 0;
+
+disable_power:
+	clk_disable_unprepare(tegra->sata_clk);
+
+	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
+
+disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+
+	return ret;
+}
+
+static void tegra_ahci_power_off(struct ahci_host_priv *hpriv)
+{
+	struct tegra_ahci_priv *tegra = hpriv->plat_data;
+
+	phy_power_off(tegra->padctl_phy);
+
+	reset_control_assert(tegra->sata_rst);
+	reset_control_assert(tegra->sata_oob_rst);
+	reset_control_assert(tegra->sata_cold_rst);
+
+	clk_disable_unprepare(tegra->plle_clk);
+	clk_disable_unprepare(tegra->cml1_clk);
+	clk_disable_unprepare(tegra->sata_oob_clk);
+	clk_disable_unprepare(tegra->sata_clk);
+
+	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
+
+	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+}
+
+static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)
+{
+	struct tegra_ahci_priv *tegra = hpriv->plat_data;
+	int ret;
+	unsigned int val;
+	struct sata_pad_calibration calib;
+
+	ret = tegra_ahci_power_on(hpriv);
+	if (ret) {
+		dev_err(&tegra->pdev->dev,
+			"failed to power on ahci controller: %d\n", ret);
+		return ret;
+	}
+
+	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
+	val |= SATA_CONFIGURATION_EN_FPCI;
+	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
+
+	/* Pad calibration */
+
+	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
+	if (ret) {
+		dev_err(&tegra->pdev->dev,
+			"failed to read sata calibration fuse: %d\n", ret);
+		return ret;
+	}
+
+	calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
+
+	writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
+
+	val = readl(tegra->sata_regs +
+		SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
+	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
+	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
+	val |= calib.gen1_tx_amp <<
+			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
+	val |= calib.gen1_tx_peak <<
+			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
+	writel(val, tegra->sata_regs + SCFG_OFFSET +
+		T_SATA0_CHX_PHY_CTRL1_GEN1);
+
+	val = readl(tegra->sata_regs +
+			SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
+	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
+	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
+	val |= calib.gen2_tx_amp <<
+			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
+	val |= calib.gen2_tx_peak <<
+			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
+	writel(val, tegra->sata_regs + SCFG_OFFSET +
+		T_SATA0_CHX_PHY_CTRL1_GEN2);
+
+	writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
+		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
+	writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
+		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
+
+	writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
+
+	/* Program controller device ID */
+
+	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+	val |= T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
+	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+
+	writel(0x01060100, tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
+
+	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+	val &= ~T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
+	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+
+	/* Enable IO & memory access, bus master mode */
+
+	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
+	val |= T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
+		T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR;
+	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
+
+	/* Program SATA MMIO */
+
+	writel(0x10000 << SATA_FPCI_BAR5_START_SHIFT,
+	       tegra->sata_regs + SATA_FPCI_BAR5);
+
+	writel(0x08000 << T_SATA0_CFG_9_BASE_ADDRESS_SHIFT,
+	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
+
+	/* Unmask SATA interrupts */
+
+	val = readl(tegra->sata_regs + SATA_INTR_MASK);
+	val |= SATA_INTR_MASK_IP_INT_MASK;
+	writel(val, tegra->sata_regs + SATA_INTR_MASK);
+
+	return 0;
+}
+
+static void tegra_ahci_controller_deinit(struct ahci_host_priv *hpriv)
+{
+	tegra_ahci_power_off(hpriv);
+}
+
+static void tegra_ahci_host_stop(struct ata_host *host)
+{
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct tegra_ahci_priv *tegra = hpriv->plat_data;
+
+	tegra_ahci_controller_deinit(hpriv);
+
+	phy_exit(tegra->padctl_phy);
+}
+
+static struct ata_port_operations ahci_tegra_port_ops = {
+	.inherits	= &ahci_ops,
+	.host_stop	= tegra_ahci_host_stop,
+};
+
+static const struct ata_port_info ahci_tegra_port_info = {
+	.flags		= AHCI_FLAG_COMMON,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &ahci_tegra_port_ops,
+};
+
+static const struct of_device_id tegra_ahci_of_match[] = {
+	{ .compatible = "nvidia,tegra124-ahci" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, tegra_ahci_of_match);
+
+static int tegra_ahci_probe(struct platform_device *pdev)
+{
+	struct ahci_host_priv *hpriv;
+	struct tegra_ahci_priv *tegra;
+	int ret;
+
+	hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
+	if (!hpriv)
+		return -ENOMEM;
+
+	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
+	if (!tegra)
+		return -ENOMEM;
+
+	hpriv->plat_data = tegra;
+
+	tegra->pdev = pdev;
+
+	hpriv->mmio = devm_ioremap_resource(&pdev->dev,
+		platform_get_resource(pdev, IORESOURCE_MEM, 0));
+	if (IS_ERR(hpriv->mmio)) {
+		dev_err(&pdev->dev, "Failed to get AHCI IO memory\n");
+		return PTR_ERR(hpriv->mmio);
+	}
+
+	tegra->sata_regs = devm_ioremap_resource(&pdev->dev,
+		platform_get_resource(pdev, IORESOURCE_MEM, 1));
+	if (IS_ERR(tegra->sata_regs)) {
+		dev_err(&pdev->dev, "Failed to get SATA IO memory\n");
+		return PTR_ERR(tegra->sata_regs);
+	}
+
+	tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata");
+	if (IS_ERR(tegra->sata_rst)) {
+		dev_err(&pdev->dev, "Failed to get sata reset\n");
+		return PTR_ERR(tegra->sata_rst);
+	}
+
+	tegra->sata_oob_rst = devm_reset_control_get(&pdev->dev, "sata-oob");
+	if (IS_ERR(tegra->sata_oob_rst)) {
+		dev_err(&pdev->dev, "Failed to get sata-oob reset\n");
+		return PTR_ERR(tegra->sata_oob_rst);
+	}
+
+	tegra->sata_cold_rst = devm_reset_control_get(&pdev->dev, "sata-cold");
+	if (IS_ERR(tegra->sata_cold_rst)) {
+		dev_err(&pdev->dev, "Failed to get sata-cold reset\n");
+		return PTR_ERR(tegra->sata_cold_rst);
+	}
+
+	tegra->sata_clk = devm_clk_get(&pdev->dev, "sata");
+	if (IS_ERR(tegra->sata_clk)) {
+		dev_err(&pdev->dev, "Failed to get sata clock\n");
+		return PTR_ERR(tegra->sata_clk);
+	}
+
+	tegra->sata_oob_clk = devm_clk_get(&pdev->dev, "sata-oob");
+	if (IS_ERR(tegra->sata_oob_clk)) {
+		dev_err(&pdev->dev, "Failed to get sata-oob clock\n");
+		return PTR_ERR(tegra->sata_oob_clk);
+	}
+
+	tegra->cml1_clk = devm_clk_get(&pdev->dev, "cml1");
+	if (IS_ERR(tegra->cml1_clk)) {
+		dev_err(&pdev->dev, "Failed to get cml1 clock\n");
+		return PTR_ERR(tegra->cml1_clk);
+	}
+
+	tegra->plle_clk = devm_clk_get(&pdev->dev, "pll_e");
+	if (IS_ERR(tegra->plle_clk)) {
+		dev_err(&pdev->dev, "Failed to get pll_e clock\n");
+		return PTR_ERR(tegra->plle_clk);
+	}
+
+	tegra->padctl_phy = devm_phy_get(&pdev->dev, "sata-phy");
+	if (IS_ERR(tegra->padctl_phy)) {
+		dev_err(&pdev->dev, "Failed to get phy\n");
+		return PTR_ERR(tegra->padctl_phy);
+	}
+
+	tegra->supplies[0].supply = "avdd";
+	tegra->supplies[1].supply = "hvdd";
+	tegra->supplies[2].supply = "vddio";
+	tegra->supplies[3].supply = "target-5v";
+	tegra->supplies[4].supply = "target-12v";
+
+	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
+					tegra->supplies);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get regulators\n");
+		return ret;
+	}
+
+	ret = phy_init(tegra->padctl_phy);
+	if (ret)
+		return ret;
+
+	ret = tegra_ahci_controller_init(hpriv);
+	if (ret)
+		goto exit_phy;
+
+	ret = ahci_platform_init_host(pdev, hpriv, &ahci_tegra_port_info,
+				      0, 0, 0);
+	if (ret)
+		goto deinit_controller;
+
+	return 0;
+
+deinit_controller:
+	tegra_ahci_controller_deinit(hpriv);
+
+exit_phy:
+	phy_exit(tegra->padctl_phy);
+
+	return ret;
+};
+
+static struct platform_driver tegra_ahci_driver = {
+	.probe = tegra_ahci_probe,
+	.remove = ata_platform_remove_one,
+	.driver = {
+		.name = "tegra-ahci",
+		.owner = THIS_MODULE,
+		.of_match_table = tegra_ahci_of_match,
+	},
+	/* LP0 suspend support not implemented */
+};
+module_platform_driver(tegra_ahci_driver);
+
+MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
+MODULE_DESCRIPTION("Tegra124 AHCI SATA driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.5

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Stephen Warren]

On 06/18/2014 08:23 AM, Mikko Perttunen wrote:
> This adds support for the integrated AHCI-compliant Serial ATA
> controller present on the NVIDIA Tegra124 system-on-chip.

At a quick glance, this looks fine to me now. I'll wait for an ack to
take this patch through the Tegra tree, for the reasons I mentioned in
response to v1.

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

FWIW, from IRC, the series

 > Tested-by: Steev Klimaszewski <steev@gentoo.org>

On 24/06/14 22:35, Stephen Warren wrote:
> On 06/18/2014 08:23 AM, Mikko Perttunen wrote:
>> This adds support for the integrated AHCI-compliant Serial ATA
>> controller present on the NVIDIA Tegra124 system-on-chip.
>
> At a quick glance, this looks fine to me now. I'll wait for an ack to
> take this patch through the Tegra tree, for the reasons I mentioned in
> response to v1.
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Hey Tejun,

the prerequisites for this series have now been acked and it would be 
nice to have it for 3.17. Could you take a look at it?

Thanks,
Mikko

On 26/06/14 14:18, Mikko Perttunen wrote:
> FWIW, from IRC, the series
>
>   > Tested-by: Steev Klimaszewski <steev@gentoo.org>
>
> On 24/06/14 22:35, Stephen Warren wrote:
>> On 06/18/2014 08:23 AM, Mikko Perttunen wrote:
>>> This adds support for the integrated AHCI-compliant Serial ATA
>>> controller present on the NVIDIA Tegra124 system-on-chip.
>>
>> At a quick glance, this looks fine to me now. I'll wait for an ack to
>> take this patch through the Tegra tree, for the reasons I mentioned in
>> response to v1.
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Tejun Heo]

(cc'ing Hans)

Hans, can you please review this patch?

On Wed, Jun 18, 2014 at 05:23:25PM +0300, Mikko Perttunen wrote:
> +#define SATA_CONFIGURATION_0				0x180
> +#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)

Let's just indent uniformly.  The new line should give enough visual
hint on grouping.

> +struct tegra_ahci_priv {
> +	struct platform_device *pdev;
> +	void __iomem *sata_regs;
> +	struct reset_control *sata_rst;
> +	struct reset_control *sata_oob_rst;
> +	struct reset_control *sata_cold_rst;
> +	struct clk *sata_clk;
> +	struct clk *sata_oob_clk;
> +	struct clk *cml1_clk;
> +	struct clk *plle_clk;
> +	struct regulator_bulk_data supplies[5];
> +	struct phy *padctl_phy;
> +};

And please indent the declared fields uniformly too.

Except for the above nitpicks, generally looks good to me but let's
wait for Hans' review.

Thanks.

-- 
tejun

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Thanks, I'll fix these.

On 08/07/14 16:22, Tejun Heo wrote:
> (cc'ing Hans)
>
> Hans, can you please review this patch?
>
> On Wed, Jun 18, 2014 at 05:23:25PM +0300, Mikko Perttunen wrote:
>> +#define SATA_CONFIGURATION_0				0x180
>> +#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)
>
> Let's just indent uniformly.  The new line should give enough visual
> hint on grouping.
>
>> +struct tegra_ahci_priv {
>> +	struct platform_device *pdev;
>> +	void __iomem *sata_regs;
>> +	struct reset_control *sata_rst;
>> +	struct reset_control *sata_oob_rst;
>> +	struct reset_control *sata_cold_rst;
>> +	struct clk *sata_clk;
>> +	struct clk *sata_oob_clk;
>> +	struct clk *cml1_clk;
>> +	struct clk *plle_clk;
>> +	struct regulator_bulk_data supplies[5];
>> +	struct phy *padctl_phy;
>> +};
>
> And please indent the declared fields uniformly too.
>
> Except for the above nitpicks, generally looks good to me but let's
> wait for Hans' review.
>
> Thanks.
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Hi Hans, have you been able to take a look at this?

Thanks,
Mikko

On 08/07/14 16:22, Tejun Heo wrote:
> (cc'ing Hans)
>
> Hans, can you please review this patch?
>
> On Wed, Jun 18, 2014 at 05:23:25PM +0300, Mikko Perttunen wrote:
>> +#define SATA_CONFIGURATION_0				0x180
>> +#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)
>
> Let's just indent uniformly.  The new line should give enough visual
> hint on grouping.
>
>> +struct tegra_ahci_priv {
>> +	struct platform_device *pdev;
>> +	void __iomem *sata_regs;
>> +	struct reset_control *sata_rst;
>> +	struct reset_control *sata_oob_rst;
>> +	struct reset_control *sata_cold_rst;
>> +	struct clk *sata_clk;
>> +	struct clk *sata_oob_clk;
>> +	struct clk *cml1_clk;
>> +	struct clk *plle_clk;
>> +	struct regulator_bulk_data supplies[5];
>> +	struct phy *padctl_phy;
>> +};
>
> And please indent the declared fields uniformly too.
>
> Except for the above nitpicks, generally looks good to me but let's
> wait for Hans' review.
>
> Thanks.
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Hans de Goede]

Hi Miko,

On 07/14/2014 08:21 AM, Mikko Perttunen wrote:
> Hi Hans, have you been able to take a look at this?

Not yet, it is on my todo list I hope to get around to
it today or tomorrow.

Regards,

Hans

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Ok, that's good, thanks.

Mikko

On 14/07/14 09:25, Hans de Goede wrote:
> Hi Miko,
>
> On 07/14/2014 08:21 AM, Mikko Perttunen wrote:
>> Hi Hans, have you been able to take a look at this?
>
> Not yet, it is on my todo list I hope to get around to
> it today or tomorrow.
>
> Regards,
>
> Hans
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Hans de Goede]

Hi,

On 07/08/2014 03:22 PM, Tejun Heo wrote:
> (cc'ing Hans)
> 
> Hans, can you please review this patch?

Done.

Mikko, it looks like you are doing a lot of stuff
the DIY way. I can see there are good reasons for
that though.

Still it would be nice if you could use a little bit
more of the helper functions provided by libahci_platform.c

Specifically I think it would be better if you used
ahci_platform_get_resources, that would remove a lot of
duplicate code from the new driver.

To be specific I would like to suggest that you
raise AHCI_MAX_CLKS to 4, and then specify an order
in which the clks must be listed in the devicetree
binding. Then you can put that order in an enum
and use hpriv->clks[CLK_FOO] in your driver, where
CLK_FOO comes from the enum.

This way you should be able to use ahci_platform_get_resources
and drop doing the iomap of the base registers, all the
clk_gets and the phy_get yourself.

You could then also use ahci_platform_disable_clks() in
tegra_ahci_power_off

Regards,

Hans

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Heh, what you are describing is the v1 of this series :)

However,

17/06/14 Thierry Reding wrote:
 > I would've preferred tegra_powergate_sequence_power_up() to be used
 > consistently in all drivers. I'm still not convinced that using the
 > platform AHCI driver this way is really the best option, since we're
 > bending over backwards to fit into what this driver dictates. There
 > shouldn't be a need for that.

As you probably noticed, the issue is that on Tegra we want to use 
tegra_powergate_sequence_power_up to enable the SATA power rail. That 
function assumes that it gets a disabled clock and returns with the 
clock enabled. If we use ahci_platform's resource management functions,
this is not doable, since it wants to handle clocks by itself.

To be honest, I too would prefer handling the resources in the driver. 
The driver needs other resources than what ahci_platform currently 
manages (at least reset_controls and multiple regulators, later we might 
get more) and managing them in the driver allows the driver to do proper 
error handling and manage all resources consistently and in one place. 
Also, I don't think it is sensible to add support for loading every 
possible kind of DT resource to ahci_platform, since most drivers won't 
need them. Other subsystems I've been in don't have this kind of helper 
library, so diverging here seems weird.

That said, if you feel strongly about this, I can do what you described.

Thanks,
Mikko

On 14/07/14 16:36, Hans de Goede wrote:
> Hi,
>
> On 07/08/2014 03:22 PM, Tejun Heo wrote:
>> (cc'ing Hans)
>>
>> Hans, can you please review this patch?
>
> Done.
>
> Mikko, it looks like you are doing a lot of stuff
> the DIY way. I can see there are good reasons for
> that though.
>
> Still it would be nice if you could use a little bit
> more of the helper functions provided by libahci_platform.c
>
> Specifically I think it would be better if you used
> ahci_platform_get_resources, that would remove a lot of
> duplicate code from the new driver.
>
> To be specific I would like to suggest that you
> raise AHCI_MAX_CLKS to 4, and then specify an order
> in which the clks must be listed in the devicetree
> binding. Then you can put that order in an enum
> and use hpriv->clks[CLK_FOO] in your driver, where
> CLK_FOO comes from the enum.
>
> This way you should be able to use ahci_platform_get_resources
> and drop doing the iomap of the base registers, all the
> clk_gets and the phy_get yourself.
>
> You could then also use ahci_platform_disable_clks() in
> tegra_ahci_power_off
>
> Regards,
>
> Hans
>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Hans de Goede]

Hi,

On 07/15/2014 09:12 AM, Mikko Perttunen wrote:
> Heh, what you are describing is the v1 of this series :)
> 
> However,
> 
> 17/06/14 Thierry Reding wrote:
>> I would've preferred tegra_powergate_sequence_power_up() to be used
>> consistently in all drivers. I'm still not convinced that using the
>> platform AHCI driver this way is really the best option, since we're
>> bending over backwards to fit into what this driver dictates. There
>> shouldn't be a need for that.
> 
> As you probably noticed, the issue is that on Tegra we want to use tegra_powergate_sequence_power_up to enable the SATA power rail. That function assumes that it gets a disabled clock and returns with the clock enabled. If we use ahci_platform's resource management functions,
> this is not doable, since it wants to handle clocks by itself.

Right, note I was not suggesting that you use ahci_platform_enable_resources /
ahci_platform_disable_resources, that clearly won't work for your case.

As I said "I can see there are good reasons for that though."

> To be honest, I too would prefer handling the resources in the driver. The driver needs other resources than what ahci_platform currently manages (at least reset_controls and multiple regulators, later we might get more) and managing them in the driver allows the driver to do proper error handling and manage all resources consistently and in one place. Also, I don't think it is sensible to add support for loading every possible kind of DT resource to ahci_platform, since most drivers won't need them.

Actually I have considered asking you to add support for reset controllers
to libahci_platform too, since I see a pattern where more and more
SOCs are starting to use those. I've refrained from asking that for now,
but once a second ahci implementation needing those pops up we will likely
go that route. So you might as well do it now :)

> Other subsystems I've been in don't have this kind of helper library, so diverging here seems weird.

Well the usb uhci-platform and ehci-platform drivers are doing the same,
what we're trying to do here is avoid needless code duplication and
if possible (for ehci and uhci it currently is) make it so that adding
support for a new soc only requires adding dt stuff + a phy driver,
without touching the ?hci code at all. Again I can see that this
is not possible for the Tegra124.

> That said, if you feel strongly about this, I can do what you described.

I think you can safe a nice bit of code duplication (and if we ever find
we have bugs there bug duplication) by switching to
ahci_platform_get_resources, as described in my original mail.

I can understand if you don't want to use any of the other ahci_platform_*
functions, that makes total sense.

I would not say this is something you must do, but I have a strong
preference for you taking a shot at doing this. So can you please give
this a try, and if it turns out to become ugly, just say so (with
some explanation), and then you can keep things as is.
Does that work for you?

Regards,

Hans






> 
> Thanks,
> Mikko
> 
> On 14/07/14 16:36, Hans de Goede wrote:
>> Hi,
>>
>> On 07/08/2014 03:22 PM, Tejun Heo wrote:
>>> (cc'ing Hans)
>>>
>>> Hans, can you please review this patch?
>>
>> Done.
>>
>> Mikko, it looks like you are doing a lot of stuff
>> the DIY way. I can see there are good reasons for
>> that though.
>>
>> Still it would be nice if you could use a little bit
>> more of the helper functions provided by libahci_platform.c
>>
>> Specifically I think it would be better if you used
>> ahci_platform_get_resources, that would remove a lot of
>> duplicate code from the new driver.
>>
>> To be specific I would like to suggest that you
>> raise AHCI_MAX_CLKS to 4, and then specify an order
>> in which the clks must be listed in the devicetree
>> binding. Then you can put that order in an enum
>> and use hpriv->clks[CLK_FOO] in your driver, where
>> CLK_FOO comes from the enum.
>>
>> This way you should be able to use ahci_platform_get_resources
>> and drop doing the iomap of the base registers, all the
>> clk_gets and the phy_get yourself.
>>
>> You could then also use ahci_platform_disable_clks() in
>> tegra_ahci_power_off
>>
>> Regards,
>>
>> Hans
>>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

On 15/07/14 11:55, Hans de Goede wrote:
> Hi,
>
> On 07/15/2014 09:12 AM, Mikko Perttunen wrote:
>> Heh, what you are describing is the v1 of this series :)
>>
>> However,
>>
>> 17/06/14 Thierry Reding wrote:
>>> I would've preferred tegra_powergate_sequence_power_up() to be used
>>> consistently in all drivers. I'm still not convinced that using the
>>> platform AHCI driver this way is really the best option, since we're
>>> bending over backwards to fit into what this driver dictates. There
>>> shouldn't be a need for that.
>>
>> As you probably noticed, the issue is that on Tegra we want to use tegra_powergate_sequence_power_up to enable the SATA power rail. That function assumes that it gets a disabled clock and returns with the clock enabled. If we use ahci_platform's resource management functions,
>> this is not doable, since it wants to handle clocks by itself.
>
> Right, note I was not suggesting that you use ahci_platform_enable_resources /
> ahci_platform_disable_resources, that clearly won't work for your case.
>
> As I said "I can see there are good reasons for that though."
>
>> To be honest, I too would prefer handling the resources in the driver. The driver needs other resources than what ahci_platform currently manages (at least reset_controls and multiple regulators, later we might get more) and managing them in the driver allows the driver to do proper error handling and manage all resources consistently and in one place. Also, I don't think it is sensible to add support for loading every possible kind of DT resource to ahci_platform, since most drivers won't need them.
>
> Actually I have considered asking you to add support for reset controllers
> to libahci_platform too, since I see a pattern where more and more
> SOCs are starting to use those. I've refrained from asking that for now,
> but once a second ahci implementation needing those pops up we will likely
> go that route. So you might as well do it now :)
>
>> Other subsystems I've been in don't have this kind of helper library, so diverging here seems weird.
>
> Well the usb uhci-platform and ehci-platform drivers are doing the same,
> what we're trying to do here is avoid needless code duplication and
> if possible (for ehci and uhci it currently is) make it so that adding
> support for a new soc only requires adding dt stuff + a phy driver,
> without touching the ?hci code at all. Again I can see that this
> is not possible for the Tegra124.
>
>> That said, if you feel strongly about this, I can do what you described.
>
> I think you can safe a nice bit of code duplication (and if we ever find
> we have bugs there bug duplication) by switching to
> ahci_platform_get_resources, as described in my original mail.
>
> I can understand if you don't want to use any of the other ahci_platform_*
> functions, that makes total sense.
>
> I would not say this is something you must do, but I have a strong
> preference for you taking a shot at doing this. So can you please give
> this a try, and if it turns out to become ugly, just say so (with
> some explanation), and then you can keep things as is.
> Does that work for you?
>
> Regards,
>
> Hans
>

Sure, I'll have a go at it. I already sent one bugfix for 
libahci_platform anyway :)

Mikko

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Thierry Reding]

On Tue, Jul 15, 2014 at 10:55:29AM +0200, Hans de Goede wrote:
> On 07/15/2014 09:12 AM, Mikko Perttunen wrote:
[...]
> > Other subsystems I've been in don't have this kind of helper
> > library, so diverging here seems weird.
> 
> Well the usb uhci-platform and ehci-platform drivers are doing the same,
> what we're trying to do here is avoid needless code duplication and
> if possible (for ehci and uhci it currently is) make it so that adding
> support for a new soc only requires adding dt stuff + a phy driver,
> without touching the ?hci code at all. Again I can see that this
> is not possible for the Tegra124.

I don't think helper libraries are good at managing resources. They are
very good for extracting common /functionality/ so that it doesn't have
to be duplicated. That's where the commonalities really are. Resource
allocation and setup are usually where the differences are. Trying to
extract that into common helpers often results in clumsy code.

Also, since resources and setup sequences are often very different, the
chances are that adding support for a new SoC will always require
extending the helpers to cope with that new type of resource that the
new SoC now requires, or bumping the maximum number of resources of an
existing type.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140715/1f255e55/attachment.sig>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Thierry Reding]

On Wed, Jun 18, 2014 at 05:23:25PM +0300, Mikko Perttunen wrote:
[...]
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
[...]
> +#include <linux/ahci_platform.h>
> +#include <linux/reset.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/tegra-powergate.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/tegra-soc.h>
> +#include "ahci.h"
> +
> +#define SATA_CONFIGURATION_0				0x180
> +#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)

This, and the register/field definitions below look weirdly indented,
but I guess that's mostly a matter of taste.

> +#define T_SATA0_CHX_PHY_CTRL1_GEN1			0x690
> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK	0xff
> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT	0
> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK	(0xff<<8)

Perhaps single spaces around "<<"? The same for other occurrences below.

> +struct sata_pad_calibration {
> +	u8 gen1_tx_amp, gen1_tx_peak, gen2_tx_amp, gen2_tx_peak;
> +};

I think a more idiomatic way would be to put the fields in a structure
declaration on separate lines.

> +static const struct sata_pad_calibration tegra124_pad_calibration[] = {
> +	{0x18, 0x04, 0x18, 0x0a},

Maybe spaces after '{' and before '}'?

> +static int tegra_ahci_power_on(struct ahci_host_priv *hpriv)
> +{
> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
> +				    tegra->supplies);
> +	if (ret)
> +		return ret;
> +
> +	ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
> +		tegra->sata_clk, tegra->sata_rst);

These could be aligned with TEGRA_POWERGATE_SATA.

> +	ret = clk_prepare_enable(tegra->cml1_clk);
> +	if (ret) {
> +		clk_disable_unprepare(tegra->sata_oob_clk);
> +		goto disable_power;
> +	}
> +
> +	ret = clk_prepare_enable(tegra->plle_clk);
> +	if (ret) {
> +		clk_disable_unprepare(tegra->cml1_clk);
> +		clk_disable_unprepare(tegra->sata_oob_clk);
> +		goto disable_power;
> +	}
> +
> +	reset_control_deassert(tegra->sata_cold_rst);
> +	reset_control_deassert(tegra->sata_oob_rst);
> +
> +	ret = phy_power_on(tegra->padctl_phy);
> +	if (ret) {
> +		clk_disable_unprepare(tegra->plle_clk);
> +		clk_disable_unprepare(tegra->cml1_clk);
> +		clk_disable_unprepare(tegra->sata_oob_clk);
> +		goto disable_power;
> +	}

These error paths might be more readable if you did unwind them similar
to how you did with sata_clk below.

> +static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)
> +{
> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
> +	int ret;
> +	unsigned int val;
> +	struct sata_pad_calibration calib;
> +
> +	ret = tegra_ahci_power_on(hpriv);
> +	if (ret) {
> +		dev_err(&tegra->pdev->dev,
> +			"failed to power on ahci controller: %d\n", ret);

s/ahci/AHCI/

> +		return ret;
> +	}
> +
> +	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
> +	val |= SATA_CONFIGURATION_EN_FPCI;
> +	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
> +
> +	/* Pad calibration */
> +
> +	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
> +	if (ret) {
> +		dev_err(&tegra->pdev->dev,
> +			"failed to read sata calibration fuse: %d\n", ret);

s/sata/SATA/

> +static struct ata_port_operations ahci_tegra_port_ops = {
> +	.inherits	= &ahci_ops,
> +	.host_stop	= tegra_ahci_host_stop,
> +};
> +
> +static const struct ata_port_info ahci_tegra_port_info = {
> +	.flags		= AHCI_FLAG_COMMON,
> +	.pio_mask	= ATA_PIO4,
> +	.udma_mask	= ATA_UDMA6,
> +	.port_ops	= &ahci_tegra_port_ops,
> +};

I prefer these to not be arbitrarily indented, but Tejun seemed to
indicate he did, so it's ultimately his call.

> +static const struct of_device_id tegra_ahci_of_match[] = {
> +	{ .compatible = "nvidia,tegra124-ahci" },
> +	{},

Technically there's no need for the comma at the end here. Usually you'd
put one here so that if somebody ever were to add a new entry after this
the diff wouldn't need to include the line that you only add a comma to.
But in this particular case the last entry is used as a sentinel, so
leaving away the comma is actually useful as a hint that entries should
be added in front rather than at the end.

> +	hpriv->mmio = devm_ioremap_resource(&pdev->dev,
> +		platform_get_resource(pdev, IORESOURCE_MEM, 0));

Perhaps a temporary variable for the result of platform_get_resource()
would help make this look less cluttered.

> +	if (IS_ERR(hpriv->mmio)) {
> +		dev_err(&pdev->dev, "Failed to get AHCI IO memory\n");

This isn't necessary. devm_ioremap_resource() will already output an
error message.

> +	tegra->sata_regs = devm_ioremap_resource(&pdev->dev,
> +		platform_get_resource(pdev, IORESOURCE_MEM, 1));
> +	if (IS_ERR(tegra->sata_regs)) {
> +		dev_err(&pdev->dev, "Failed to get SATA IO memory\n");

Same here.

> +	tegra->padctl_phy = devm_phy_get(&pdev->dev, "sata-phy");
> +	if (IS_ERR(tegra->padctl_phy)) {
> +		dev_err(&pdev->dev, "Failed to get phy\n");

s/phy/PHY/

> +	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
> +					tegra->supplies);

The trailing argument here isn't aligned as in other places.

> +static struct platform_driver tegra_ahci_driver = {
> +	.probe = tegra_ahci_probe,
> +	.remove = ata_platform_remove_one,
> +	.driver = {
> +		.name = "tegra-ahci",
> +		.owner = THIS_MODULE,

This isn't really needed anymore, module_platform_driver will end up
setting this for you anyway.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140709/cc582e79/attachment.sig>

[PATCH v2 6/7] ata: Add support for the Tegra124 SATA controller

[Mikko Perttunen]

Thanks, will fix these. Looks like I will have to change the fuse code 
to the old style as well.

On 09/07/14 09:49, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jun 18, 2014 at 05:23:25PM +0300, Mikko Perttunen wrote:
> [...]
>> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> [...]
>> +#include <linux/ahci_platform.h>
>> +#include <linux/reset.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/tegra-powergate.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/tegra-soc.h>
>> +#include "ahci.h"
>> +
>> +#define SATA_CONFIGURATION_0				0x180
>> +#define		SATA_CONFIGURATION_EN_FPCI		BIT(0)
>
> This, and the register/field definitions below look weirdly indented,
> but I guess that's mostly a matter of taste.
>
>> +#define T_SATA0_CHX_PHY_CTRL1_GEN1			0x690
>> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK	0xff
>> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT	0
>> +#define		T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK	(0xff<<8)
>
> Perhaps single spaces around "<<"? The same for other occurrences below.
>
>> +struct sata_pad_calibration {
>> +	u8 gen1_tx_amp, gen1_tx_peak, gen2_tx_amp, gen2_tx_peak;
>> +};
>
> I think a more idiomatic way would be to put the fields in a structure
> declaration on separate lines.
>
>> +static const struct sata_pad_calibration tegra124_pad_calibration[] = {
>> +	{0x18, 0x04, 0x18, 0x0a},
>
> Maybe spaces after '{' and before '}'?
>
>> +static int tegra_ahci_power_on(struct ahci_host_priv *hpriv)
>> +{
>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>> +	int ret;
>> +
>> +	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
>> +				    tegra->supplies);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
>> +		tegra->sata_clk, tegra->sata_rst);
>
> These could be aligned with TEGRA_POWERGATE_SATA.
>
>> +	ret = clk_prepare_enable(tegra->cml1_clk);
>> +	if (ret) {
>> +		clk_disable_unprepare(tegra->sata_oob_clk);
>> +		goto disable_power;
>> +	}
>> +
>> +	ret = clk_prepare_enable(tegra->plle_clk);
>> +	if (ret) {
>> +		clk_disable_unprepare(tegra->cml1_clk);
>> +		clk_disable_unprepare(tegra->sata_oob_clk);
>> +		goto disable_power;
>> +	}
>> +
>> +	reset_control_deassert(tegra->sata_cold_rst);
>> +	reset_control_deassert(tegra->sata_oob_rst);
>> +
>> +	ret = phy_power_on(tegra->padctl_phy);
>> +	if (ret) {
>> +		clk_disable_unprepare(tegra->plle_clk);
>> +		clk_disable_unprepare(tegra->cml1_clk);
>> +		clk_disable_unprepare(tegra->sata_oob_clk);
>> +		goto disable_power;
>> +	}
>
> These error paths might be more readable if you did unwind them similar
> to how you did with sata_clk below.
>
>> +static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)
>> +{
>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>> +	int ret;
>> +	unsigned int val;
>> +	struct sata_pad_calibration calib;
>> +
>> +	ret = tegra_ahci_power_on(hpriv);
>> +	if (ret) {
>> +		dev_err(&tegra->pdev->dev,
>> +			"failed to power on ahci controller: %d\n", ret);
>
> s/ahci/AHCI/
>
>> +		return ret;
>> +	}
>> +
>> +	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
>> +	val |= SATA_CONFIGURATION_EN_FPCI;
>> +	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
>> +
>> +	/* Pad calibration */
>> +
>> +	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
>> +	if (ret) {
>> +		dev_err(&tegra->pdev->dev,
>> +			"failed to read sata calibration fuse: %d\n", ret);
>
> s/sata/SATA/
>
>> +static struct ata_port_operations ahci_tegra_port_ops = {
>> +	.inherits	= &ahci_ops,
>> +	.host_stop	= tegra_ahci_host_stop,
>> +};
>> +
>> +static const struct ata_port_info ahci_tegra_port_info = {
>> +	.flags		= AHCI_FLAG_COMMON,
>> +	.pio_mask	= ATA_PIO4,
>> +	.udma_mask	= ATA_UDMA6,
>> +	.port_ops	= &ahci_tegra_port_ops,
>> +};
>
> I prefer these to not be arbitrarily indented, but Tejun seemed to
> indicate he did, so it's ultimately his call.
>
>> +static const struct of_device_id tegra_ahci_of_match[] = {
>> +	{ .compatible = "nvidia,tegra124-ahci" },
>> +	{},
>
> Technically there's no need for the comma at the end here. Usually you'd
> put one here so that if somebody ever were to add a new entry after this
> the diff wouldn't need to include the line that you only add a comma to.
> But in this particular case the last entry is used as a sentinel, so
> leaving away the comma is actually useful as a hint that entries should
> be added in front rather than at the end.
>
>> +	hpriv->mmio = devm_ioremap_resource(&pdev->dev,
>> +		platform_get_resource(pdev, IORESOURCE_MEM, 0));
>
> Perhaps a temporary variable for the result of platform_get_resource()
> would help make this look less cluttered.
>
>> +	if (IS_ERR(hpriv->mmio)) {
>> +		dev_err(&pdev->dev, "Failed to get AHCI IO memory\n");
>
> This isn't necessary. devm_ioremap_resource() will already output an
> error message.
>
>> +	tegra->sata_regs = devm_ioremap_resource(&pdev->dev,
>> +		platform_get_resource(pdev, IORESOURCE_MEM, 1));
>> +	if (IS_ERR(tegra->sata_regs)) {
>> +		dev_err(&pdev->dev, "Failed to get SATA IO memory\n");
>
> Same here.
>
>> +	tegra->padctl_phy = devm_phy_get(&pdev->dev, "sata-phy");
>> +	if (IS_ERR(tegra->padctl_phy)) {
>> +		dev_err(&pdev->dev, "Failed to get phy\n");
>
> s/phy/PHY/
>
>> +	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
>> +					tegra->supplies);
>
> The trailing argument here isn't aligned as in other places.
>
>> +static struct platform_driver tegra_ahci_driver = {
>> +	.probe = tegra_ahci_probe,
>> +	.remove = ata_platform_remove_one,
>> +	.driver = {
>> +		.name = "tegra-ahci",
>> +		.owner = THIS_MODULE,
>
> This isn't really needed anymore, module_platform_driver will end up
> setting this for you anyway.
>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

[PATCH v2 7/7] ARM: tegra: Add options for Tegra AHCI support to tegra_defconfig

[Mikko Perttunen]

This adds ATA, SATA_AHCI and AHCI_TEGRA support to tegra_defconfig
so that the SATA support will be automatically enabled.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/configs/tegra_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 285c433..2b8be48 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -102,6 +102,9 @@ CONFIG_BLK_DEV_SD=y
 CONFIG_BLK_DEV_SR=y
 CONFIG_SCSI_MULTI_LUN=y
 # CONFIG_SCSI_LOWLEVEL is not set
+CONFIG_ATA=y
+CONFIG_SATA_AHCI=y
+CONFIG_AHCI_TEGRA=y
 CONFIG_NETDEVICES=y
 CONFIG_DUMMY=y
 CONFIG_IGB=y
-- 
1.8.1.5