[U-Boot] [PATCH RFC 0/2] u-boot: arndale: Support for SATA controller.

[U-Boot] [PATCH RFC 0/2] u-boot: arndale: Support for SATA controller.

[Ian Campbell]

Hello,

The following series ports over Taylor Hutt's patches to enable SATA for
the Chromeos smdk5250 u-boot and enables it for arndale (but not
smdk5250 since I don't have one).

In terms of basic access from u-boot it works for me but there is a
strange issue once booted Linux, which is that it will fail to detect
the disk if the PMU bit for the SATA is not disabled prior to launching
Linux, see the second patch for a hack which helps, but not completely.
Even with that hack it still fails to detect the disk some fraction of
the time, in both failure cases the output is essentially the same:

[    2.380814] ahci 122f0000.sata: SSS flag set, parallel bus scan disabled
[    2.386096] ahci 122f0000.sata: AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl platform mode
[    2.394583] ahci 122f0000.sata: flags: ncq sntf stag pm led clo only pmp pio slum part ccc apst 
[    2.424239] scsi0 : ahci_platform
[    2.427650] ata1: SATA max UDMA/133 mmio [mem 0x122f0000-0x122f01fe] port 0x100 irq 147
[   12.433897] ata1: softreset failed (1st FIS failed)
[   22.433896] ata1: softreset failed (1st FIS failed)
[   57.433895] ata1: softreset failed (1st FIS failed)
[   57.437299] ata1: limiting SATA link speed to 1.5 Gbps
[   62.629902] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[   62.634623] ata1.00: link online but device misclassified
[   62.634632] ata1: link online but 1 devices misclassified, device detection might fail

Usually a warm reset will succeed, although sometimes it seems a hard
reset is required. When it works I see instead:

[    2.337636] ahci 122f0000.sata: SSS flag set, parallel bus scan disabled
[    2.342901] ahci 122f0000.sata: AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl platform mode
[    2.351392] ahci 122f0000.sata: flags: ncq sntf stag pm led clo only pmp pio slum part ccc apst 
[    2.364181] scsi0 : ahci_platform
[    2.368469] ata1: SATA max UDMA/133 mmio [mem 0x122f0000-0x122f01fe] port 0x100 irq 147
[    2.865454] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[    2.870972] ata1.00: ATA-8: HGST HTS545050A7E380, GG2OAC90, max UDMA/133
[    2.876867] ata1.00: 976773168 sectors, multi 16: LBA48 NCQ (depth 31/32)
[    2.884496] ata1.00: configured for UDMA/133
[    2.888172] scsi 0:0:0:0: Direct-Access     ATA      HGST HTS545050A7 AC90 PQ: 0 ANSI: 5
[    3.277166] sd 0:0:0:0: [sda] Attached SCSI disk
[    3.286625] sd 0:0:0:0: Attached scsi generic sg0 type 0

My suspicion is that the SATA PHY is not being properly reset by Linux,
but I've mucked around with it in various ways (on the u-boot side only)
to no avail. The u-boot and Linux code here is identical AFAICT (modulo
my mucking around and the use of clr/setbits in u-boot, but Linux open
codes the same thing).

Kukjin & Yuvaraj, I've included you in the CC line in case you can shed
any light on the issue from the Linux side.

Thanks,
Ian.

[U-Boot] [PATCH 1/2] exynos5250/arndale: Enable SATA/AHCI support.

[Ian Campbell]

From: Ian Campbell <ian.campbell@citrix.com>

This is based on some old patches from the chromeos-v2011.12 branch of
http://git.chromium.org/chromiumos/third_party/u-boot.git by Taylor Hutt.
Specifically:

http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=26f6c570b5deb37c52306920ae049203c68f014a
    exynos: sata: on-board controller initialization
    Signed-off-by: Taylor Hutt <thutt@chromium.org>

http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=d8cac5cf0b63df00d2d6ac7df814613e4b60b9d1
    exynos: sata: Add sata_initialize() interface
    Signed-off-by: Taylor Hutt <thutt@chromium.org>

http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=dd32462453d6328bc5770859d1b56501f7920d7d
    exynos: sata: SATA self-configuration for when SATA device is enabled
    Signed-off-by: Taylor Hutt <thutt@chromium.org>

As well as rebasing there have been some significant changes.

 - Drop support for smdk5250, which I don't own.
 - Implement support for arndale, which I do.
 - Since arndale has no need to frob a GPIO on SATA init drop the associated
   code.
 - Initialise via the existing scsi_init hook rather than introducing
   sata_initialize, associated build system and include/configs/*.h changes.
 - Use set/clrbits in a bunch of places
 - Add some #defines for some magic numbers.

This relies on "ahci: Don't start command DMA engine before buffers are set"

NOTE: For some reason when running u-boot with this patch Linux is unable to
correct probe devices. See the next patch for an attempt at a hack/workaround.
Any ideas would be appreciated.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Taylor Hutt <thutt@chromium.org>
Cc: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/exynos/Makefile      |   4 +
 arch/arm/cpu/armv7/exynos/sata.c        | 370 ++++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-exynos/sata.h |  29 +++
 arch/arm/lib/board.c                    |   1 +
 board/samsung/arndale/arndale.c         |   9 +
 include/configs/arndale.h               |  13 ++
 6 files changed, 426 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/exynos/sata.c
 create mode 100644 arch/arm/include/asm/arch-exynos/sata.h

diff --git a/arch/arm/cpu/armv7/exynos/Makefile b/arch/arm/cpu/armv7/exynos/Makefile
index e207bd6..c74a2d4 100644
--- a/arch/arm/cpu/armv7/exynos/Makefile
+++ b/arch/arm/cpu/armv7/exynos/Makefile
@@ -7,6 +7,10 @@
 
 obj-y	+= clock.o power.o soc.o system.o pinmux.o tzpc.o
 
+ifndef CONFIG_SPL_BUILD
+obj-$(CONFIG_EXYNOS5250_AHCI) += sata.o
+endif
+
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_EXYNOS5)	+= clock_init_exynos5.o
 obj-$(CONFIG_EXYNOS5)	+= dmc_common.o dmc_init_ddr3.o
diff --git a/arch/arm/cpu/armv7/exynos/sata.c b/arch/arm/cpu/armv7/exynos/sata.c
new file mode 100644
index 0000000..14d42e7
--- /dev/null
+++ b/arch/arm/cpu/armv7/exynos/sata.c
@@ -0,0 +1,370 @@
+/*
+ * Copyright (c) 2012 The Chromium OS Authors.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <asm/types.h>
+#include <ahci.h>
+#include <common.h>
+#include <fdtdec.h>
+#include <scsi.h>
+#include <asm/arch/sata.h>
+#include <asm/arch/pinmux.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/clk.h>
+#include <asm/errno.h>
+#include <asm/gpio.h>
+#include <linux/compiler.h>
+
+#define SATA_AHCI_AXI			0x122f0000
+#define SATA_PHCTRL_APB			0x12170000
+#define SATA_PHY_I2C_ABP		0x121d0000
+#define EXYNOS5_SATA_PHY_CONTROL	(0x10040000 + 0x724)
+#define S5P_PMU_SATA_PHY_CONTROL_EN	0x1
+
+void * const phy_ctrl = (void *)SATA_PHCTRL_APB;
+void * const phy_i2c_base = (void *)SATA_PHY_I2C_ABP;
+
+#define SATA_TIME_LIMIT			10000
+#define SATA_PHY_I2C_SLAVE_ADDRS	0x70
+
+#define SATA_RESET			0x4
+#define RESET_GLOBAL_RST_N              (1 << 0)
+#define RESET_CMN_RST_N			(1 << 1)
+#define RESET_CMN_BLOCK_RST_N           (1 << 2)
+#define RESET_CMN_I2C_RST_N             (1 << 3)
+#define RESET_TX_RX_PIPE_RST_N          (1 << 4)
+#define RESET_TX_RX_BLOCK_RST_N         (1 << 5)
+#define RESET_TX_RX_I2C_RST_N           ((1 << 6) | BIT(7))
+
+#define LINK_RESET			0xF0000
+
+#define SATA_MODE0			0x10
+
+#define SATA_CTRL0			0x14
+#define CTRL0_P0_PHY_CALIBRATED_SEL	(1 << 9)
+#define CTRL0_P0_PHY_CALIBRATED		(1 << 8)
+
+#define SATA_PHSATA_CTRLM		0xE0
+#define PHCTRLM_REF_RATE		(1 << 1)
+#define PHCTRLM_HIGH_SPEED		(1 << 0)
+
+#define SATA_PHSATA_STATM		0xF0
+#define PHSTATM_PLL_LOCKED		(1 << 0)
+
+
+/********************** I2C**************/
+#define SATA_I2C_CON			0x00
+#define SATA_I2C_STAT			0x04
+#define SATA_I2C_ADDR			0x08
+#define SATA_I2C_DS			0x0C
+#define SATA_I2C_LC			0x10
+
+/* I2CCON reg */
+#define CON_ACKEN			(1 << 7)
+#define CON_CLK512			(1 << 6)
+#define CON_CLK16			(~CON_CLK512)
+#define CON_INTEN			(1 << 5)
+#define CON_INTPND			(1 << 4)
+#define CON_TXCLK_PS			(0xF)
+
+/* I2CSTAT reg */
+#define STAT_MSTT			(0x3 << 6)
+#define STAT_BSYST			(1 << 5)
+#define STAT_RTEN			(1 << 4)
+#define STAT_LAST			(1 << 0)
+
+#define LC_FLTR_EN			(1 << 2)
+
+#define SATA_PHY_CON_RESET		0xF003F
+
+#define SCLK_SATA_FREQ			(66 * MHZ)
+
+
+
+enum {
+	SATA_GENERATION1,
+	SATA_GENERATION2,
+	SATA_GENERATION3,
+};
+
+static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32 Status)
+{
+	if ((__raw_readl(base + reg) & checkbit) == Status)
+		return true;
+	else
+		return false;
+}
+
+static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
+		u32 Status)
+{
+	u32 time_limit_cnt = 0;
+	while (!sata_is_reg(base, reg, checkbit, Status)) {
+		if (time_limit_cnt == SATA_TIME_LIMIT) {
+			return false;
+		}
+		udelay(1000);
+		time_limit_cnt++;
+	}
+	return true;
+}
+
+
+static void sata_set_gen(u8 gen)
+{
+	setbits_le32(phy_ctrl + SATA_MODE0, gen);
+}
+
+/* Address :I2C Address */
+static void sata_i2c_write_addrs(u8 data)
+{
+	__raw_writeb((data & 0xFE), phy_i2c_base + SATA_I2C_DS);
+}
+
+static void sata_i2c_write_data(u8 data)
+{
+	__raw_writeb((data), phy_i2c_base + SATA_I2C_DS);
+}
+
+static void sata_i2c_start(void)
+{
+	setbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_BSYST);
+}
+
+static void sata_i2c_stop(void)
+{
+	clrbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_BSYST);
+}
+
+static bool sata_i2c_get_int_status(void)
+{
+	if ((__raw_readl(phy_i2c_base + SATA_I2C_CON)) & CON_INTPND)
+		return true;
+	else
+		return false;
+}
+
+static bool sata_i2c_is_tx_ack(void)
+{
+	if ((__raw_readl(phy_i2c_base + SATA_I2C_STAT)) & STAT_LAST)
+		return false;
+	else
+		return true;
+}
+
+static bool sata_i2c_is_bus_ready(void)
+{
+	if ((__raw_readl(phy_i2c_base + SATA_I2C_STAT)) & STAT_BSYST)
+		return false;
+	else
+		return true;
+}
+
+static bool sata_i2c_wait_for_busready(u32 time_out)
+{
+	while (--time_out) {
+		if (sata_i2c_is_bus_ready())
+			return true;
+		udelay(100);
+	}
+	return false;
+}
+
+static bool sata_i2c_wait_for_tx_ack(u32 time_out)
+{
+	while (--time_out) {
+		if (sata_i2c_get_int_status()) {
+			if (sata_i2c_is_tx_ack())
+				return true;
+		}
+		udelay(100);
+	}
+	return false;
+}
+
+static void sata_i2c_clear_int_status(void)
+{
+	clrbits_le32(phy_i2c_base + SATA_I2C_CON, CON_INTPND);
+}
+
+static void sata_i2c_set_ack_gen(bool enable)
+{
+	if (enable)
+		setbits_le32(phy_i2c_base + SATA_I2C_CON, CON_ACKEN);
+	else
+		clrbits_le32(phy_i2c_base + SATA_I2C_CON, CON_ACKEN);
+}
+
+static void sata_i2c_set_master_tx(void)
+{
+	/* Disable I2C */
+	clrbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_RTEN);
+
+	/* Clear Mode */
+	clrbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_MSTT);
+
+	sata_i2c_clear_int_status();
+
+	/* interrupt disable */
+	clrbits_le32(phy_i2c_base + SATA_I2C_CON, CON_INTEN);
+
+	/* Master, Send mode */
+	setbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_MSTT);
+
+	/* interrupt enable */
+	setbits_le32(phy_i2c_base + SATA_I2C_CON, CON_INTEN);
+
+	/* Enable I2C */
+	setbits_le32(phy_i2c_base + SATA_I2C_STAT, STAT_RTEN);
+}
+
+static void sata_i2c_init(void)
+{
+	clrbits_le32(phy_i2c_base + SATA_I2C_CON, ~CON_CLK16);
+	clrbits_le32(phy_i2c_base + SATA_I2C_CON, CON_TXCLK_PS);
+	setbits_le32(phy_i2c_base + SATA_I2C_CON, 2 & CON_TXCLK_PS);
+	clrbits_le32(phy_i2c_base + SATA_I2C_LC, LC_FLTR_EN);
+
+	sata_i2c_set_ack_gen(false);
+}
+static bool sata_i2c_send(u8 slave_addrs, u8 addrs, u8 ucData)
+{
+	s32 ret = 0;
+	if (!sata_i2c_wait_for_busready(SATA_TIME_LIMIT))
+		return false;
+
+	sata_i2c_init();
+	sata_i2c_set_master_tx();
+
+	__raw_writel(SATA_PHY_CON_RESET, phy_ctrl + SATA_RESET);
+	sata_i2c_write_addrs(slave_addrs);
+	sata_i2c_start();
+	if (!sata_i2c_wait_for_tx_ack(SATA_TIME_LIMIT)) {
+		ret = false;
+		goto STOP;
+	}
+	sata_i2c_write_data(addrs);
+	sata_i2c_clear_int_status();
+	if (!sata_i2c_wait_for_tx_ack(SATA_TIME_LIMIT)) {
+		ret = false;
+		goto STOP;
+	}
+	sata_i2c_write_data(ucData);
+	sata_i2c_clear_int_status();
+	if (!sata_i2c_wait_for_tx_ack(SATA_TIME_LIMIT)) {
+		ret = false;
+		goto STOP;
+	}
+	ret = true;
+
+STOP:
+	sata_i2c_stop();
+	sata_i2c_clear_int_status();
+	sata_i2c_wait_for_busready(SATA_TIME_LIMIT);
+
+	return ret;
+}
+
+static bool ahci_phy_init(void __iomem *mmio)
+{
+	u8 uCount, i = 0;
+	/* 0x3A for 40bit I/F */
+	u8 reg_addrs[] = {0x3A};
+	/* 0x0B for 40bit I/F */
+	u8 default_setting_value[] = {0x0B};
+
+	uCount = sizeof(reg_addrs)/sizeof(u8);
+	while (i < uCount) {
+		if (!sata_i2c_send(SATA_PHY_I2C_SLAVE_ADDRS, reg_addrs[i],
+					default_setting_value[i]))
+			return false;
+		i++;
+	}
+	return true;
+}
+
+static int exynos5_ahci_init(void __iomem *mmio)
+{
+	int ret;
+
+	if (sata_is_reg(phy_ctrl, SATA_CTRL0,
+		CTRL0_P0_PHY_CALIBRATED, CTRL0_P0_PHY_CALIBRATED))
+	{
+		printf("%s: already calibrated?\n", __func__);
+	}
+
+	setbits_le32(EXYNOS5_SATA_PHY_CONTROL, S5P_PMU_SATA_PHY_CONTROL_EN);
+
+	__raw_writel(0, phy_ctrl + SATA_RESET);
+	setbits_le32(phy_ctrl + SATA_RESET,
+		     RESET_GLOBAL_RST_N|
+		     RESET_CMN_BLOCK_RST_N|RESET_CMN_I2C_RST_N|
+		     RESET_TX_RX_PIPE_RST_N|RESET_TX_RX_BLOCK_RST_N);
+
+	setbits_le32(phy_ctrl + SATA_RESET, LINK_RESET);
+
+	setbits_le32(phy_ctrl + SATA_RESET, RESET_CMN_RST_N);
+
+	clrbits_le32(phy_ctrl + SATA_PHSATA_CTRLM, PHCTRLM_REF_RATE);
+
+	/* High speed enable for Gen3 */
+	setbits_le32(phy_ctrl + SATA_PHSATA_CTRLM, PHCTRLM_HIGH_SPEED);
+
+	setbits_le32(phy_ctrl + SATA_CTRL0,
+		     CTRL0_P0_PHY_CALIBRATED_SEL|CTRL0_P0_PHY_CALIBRATED);
+
+	sata_set_gen(SATA_GENERATION3);
+
+	ret = ahci_phy_init(mmio);
+
+	/* release cmu reset */
+	clrbits_le32(phy_ctrl + SATA_RESET, RESET_CMN_RST_N);
+
+	setbits_le32(phy_ctrl + SATA_RESET, RESET_CMN_RST_N);
+
+	if (wait_for_reg_status(phy_ctrl, SATA_PHSATA_STATM,
+				PHSTATM_PLL_LOCKED, 1))
+		return ret;
+
+	return 0;
+}
+
+static void exynos5_enable_clock_gates(void)
+{
+	struct exynos5_clock *clk =
+		(struct exynos5_clock *)samsung_get_base_clock();
+
+	/* Turn on all SATA clock gates & DMA gates.  */
+	const unsigned sata_clocks	= (1 << 25) | (1 << 24) | (1 << 6);
+	const unsigned dma_clocks	= (2 << 1) | (1 << 1);
+
+	setbits_le32(&clk->gate_ip_fsys, sata_clocks|dma_clocks);
+}
+
+int exynos5_sata_init(void)
+{
+	exynos5_enable_clock_gates();
+
+	if (exynos5_ahci_init((void *)SATA_AHCI_AXI)) {
+		ahci_init(SATA_AHCI_AXI);
+		return 0;
+	}
+	return -ENODEV;
+}
diff --git a/arch/arm/include/asm/arch-exynos/sata.h b/arch/arm/include/asm/arch-exynos/sata.h
new file mode 100644
index 0000000..a181d01
--- /dev/null
+++ b/arch/arm/include/asm/arch-exynos/sata.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2012 The Chromium OS Authors.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#ifndef __EXYNOS5_SATA_H
+#define __EXYNOS5_SATA_H
+
+int exynos5_sata_init(void);
+int exynos5_sata_deinit(void);
+
+#endif
+
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 76adaf3..c06af34 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -34,6 +34,7 @@
 #include <onenand_uboot.h>
 #include <mmc.h>
 #include <scsi.h>
+#include <sata.h>
 #include <libfdt.h>
 #include <fdtdec.h>
 #include <post.h>
diff --git a/board/samsung/arndale/arndale.c b/board/samsung/arndale/arndale.c
index 83fd3bd..551bfce 100644
--- a/board/samsung/arndale/arndale.c
+++ b/board/samsung/arndale/arndale.c
@@ -10,6 +10,7 @@
 #include <asm/arch/dwmmc.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/power.h>
+#include <asm/arch/sata.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -29,6 +30,14 @@ int board_usb_init(int index, enum usb_init_type init)
 }
 #endif
 
+#ifdef CONFIG_SCSI_AHCI
+int scsi_init(void)
+{
+	printf("ARNDALE SCSI INIT\n");
+	return exynos5_sata_init();
+}
+#endif
+
 int board_init(void)
 {
 	gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
diff --git a/include/configs/arndale.h b/include/configs/arndale.h
index 292729b..e992769 100644
--- a/include/configs/arndale.h
+++ b/include/configs/arndale.h
@@ -75,6 +75,7 @@
 #define CONFIG_SUPPORT_EMMC_BOOT
 #define CONFIG_BOUNCE_BUFFER
 
+#define CONFIG_EXYNOS5250_AHCI
 
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_SKIP_LOWLEVEL_INIT
@@ -91,6 +92,17 @@
 #define CONFIG_CMD_MMC
 #define CONFIG_CMD_HASH
 
+#ifdef CONFIG_EXYNOS5250_AHCI
+#define CONFIG_CMD_SCSI
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID     1
+#define CONFIG_SYS_SCSI_MAX_LUN         1
+#define CONFIG_SYS_SCSI_MAX_DEVICE      (CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+                                         CONFIG_SYS_SCSI_MAX_LUN)
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#endif
+
 #define CONFIG_ZERO_BOOTDELAY_CHECK
 
 /* USB */
@@ -204,6 +216,7 @@
 #define BOOT_TARGET_DEVICES(func) \
 	func(MMC, mmc, 1) \
 	func(MMC, mmc, 0) \
+	func(SCSI, scsi, 0) \
 	func(PXE, pxe, na) \
 	func(DHCP, dhcp, na)
 
-- 
2.1.0

[U-Boot] [PATCH 1/2] exynos5250/arndale: Enable SATA/AHCI support.

[Minkyu Kang]

Dear Ian Campbell,

On 7 October 2014 22:56, Ian Campbell <ijc@hellion.org.uk> wrote:

> From: Ian Campbell <ian.campbell@citrix.com>
>
> This is based on some old patches from the chromeos-v2011.12 branch of
> http://git.chromium.org/chromiumos/third_party/u-boot.git by Taylor Hutt.
> Specifically:
>
>
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=26f6c570b5deb37c52306920ae049203c68f014a
>     exynos: sata: on-board controller initialization
>     Signed-off-by: Taylor Hutt <thutt@chromium.org>
>
>
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=d8cac5cf0b63df00d2d6ac7df814613e4b60b9d1
>     exynos: sata: Add sata_initialize() interface
>     Signed-off-by: Taylor Hutt <thutt@chromium.org>
>
>
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=dd32462453d6328bc5770859d1b56501f7920d7d
>     exynos: sata: SATA self-configuration for when SATA device is enabled
>     Signed-off-by: Taylor Hutt <thutt@chromium.org>
>
> As well as rebasing there have been some significant changes.
>
>  - Drop support for smdk5250, which I don't own.
>  - Implement support for arndale, which I do.
>  - Since arndale has no need to frob a GPIO on SATA init drop the
> associated
>    code.
>  - Initialise via the existing scsi_init hook rather than introducing
>    sata_initialize, associated build system and include/configs/*.h
> changes.
>  - Use set/clrbits in a bunch of places
>  - Add some #defines for some magic numbers.
>
> This relies on "ahci: Don't start command DMA engine before buffers are
> set"
>
> NOTE: For some reason when running u-boot with this patch Linux is unable
> to
> correct probe devices. See the next patch for an attempt at a
> hack/workaround.
> Any ideas would be appreciated.
>

So, is it RFC? or not?
Why you tied up this RFC patch? ([U-Boot,2/2] HACK: arndale: deinit scsi
before launching Linux <http://patchwork.ozlabs.org/patch/397316/>)


>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Taylor Hutt <thutt@chromium.org>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/cpu/armv7/exynos/Makefile      |   4 +
>  arch/arm/cpu/armv7/exynos/sata.c        | 370
> ++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-exynos/sata.h |  29 +++
>  arch/arm/lib/board.c                    |   1 +
>  board/samsung/arndale/arndale.c         |   9 +
>  include/configs/arndale.h               |  13 ++
>  6 files changed, 426 insertions(+)
>  create mode 100644 arch/arm/cpu/armv7/exynos/sata.c
>  create mode 100644 arch/arm/include/asm/arch-exynos/sata.h
>
> diff --git a/arch/arm/cpu/armv7/exynos/Makefile
> b/arch/arm/cpu/armv7/exynos/Makefile
> index e207bd6..c74a2d4 100644
> --- a/arch/arm/cpu/armv7/exynos/Makefile
> +++ b/arch/arm/cpu/armv7/exynos/Makefile
> @@ -7,6 +7,10 @@
>
>  obj-y  += clock.o power.o soc.o system.o pinmux.o tzpc.o
>
> +ifndef CONFIG_SPL_BUILD
> +obj-$(CONFIG_EXYNOS5250_AHCI) += sata.o
> +endif
> +
>  ifdef CONFIG_SPL_BUILD
>  obj-$(CONFIG_EXYNOS5)  += clock_init_exynos5.o
>  obj-$(CONFIG_EXYNOS5)  += dmc_common.o dmc_init_ddr3.o
> diff --git a/arch/arm/cpu/armv7/exynos/sata.c
> b/arch/arm/cpu/armv7/exynos/sata.c
> new file mode 100644
> index 0000000..14d42e7
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/exynos/sata.c
> @@ -0,0 +1,370 @@
> +/*
> + * Copyright (c) 2012 The Chromium OS Authors.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +#include <asm/types.h>
> +#include <ahci.h>
> +#include <common.h>
> +#include <fdtdec.h>
> +#include <scsi.h>
> +#include <asm/arch/sata.h>
> +#include <asm/arch/pinmux.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/clk.h>
> +#include <asm/errno.h>
> +#include <asm/gpio.h>
> +#include <linux/compiler.h>
> +
> +#define SATA_AHCI_AXI                  0x122f0000
> +#define SATA_PHCTRL_APB                        0x12170000
> +#define SATA_PHY_I2C_ABP               0x121d0000
> +#define EXYNOS5_SATA_PHY_CONTROL       (0x10040000 + 0x724)
> +#define S5P_PMU_SATA_PHY_CONTROL_EN    0x1
> +
> +void * const phy_ctrl = (void *)SATA_PHCTRL_APB;
> +void * const phy_i2c_base = (void *)SATA_PHY_I2C_ABP;
>

we don't allow direct access to the register.
please refer to other driver.


> +
> +#define SATA_TIME_LIMIT                        10000
> +#define SATA_PHY_I2C_SLAVE_ADDRS       0x70
> +
> +#define SATA_RESET                     0x4
> +#define RESET_GLOBAL_RST_N              (1 << 0)
> +#define RESET_CMN_RST_N                        (1 << 1)
> +#define RESET_CMN_BLOCK_RST_N           (1 << 2)
> +#define RESET_CMN_I2C_RST_N             (1 << 3)
> +#define RESET_TX_RX_PIPE_RST_N          (1 << 4)
> +#define RESET_TX_RX_BLOCK_RST_N         (1 << 5)
> +#define RESET_TX_RX_I2C_RST_N           ((1 << 6) | BIT(7))
> +
> +#define LINK_RESET                     0xF0000
> +
> +#define SATA_MODE0                     0x10
> +
> +#define SATA_CTRL0                     0x14
> +#define CTRL0_P0_PHY_CALIBRATED_SEL    (1 << 9)
> +#define CTRL0_P0_PHY_CALIBRATED                (1 << 8)
> +
> +#define SATA_PHSATA_CTRLM              0xE0
> +#define PHCTRLM_REF_RATE               (1 << 1)
> +#define PHCTRLM_HIGH_SPEED             (1 << 0)
> +
> +#define SATA_PHSATA_STATM              0xF0
> +#define PHSTATM_PLL_LOCKED             (1 << 0)
> +
> +
>

please remove blank line.


> +/********************** I2C**************/
> +#define SATA_I2C_CON                   0x00
> +#define SATA_I2C_STAT                  0x04
> +#define SATA_I2C_ADDR                  0x08
> +#define SATA_I2C_DS                    0x0C
> +#define SATA_I2C_LC                    0x10
> +
> +/* I2CCON reg */
> +#define CON_ACKEN                      (1 << 7)
> +#define CON_CLK512                     (1 << 6)
> +#define CON_CLK16                      (~CON_CLK512)
> +#define CON_INTEN                      (1 << 5)
> +#define CON_INTPND                     (1 << 4)
> +#define CON_TXCLK_PS                   (0xF)
> +
> +/* I2CSTAT reg */
> +#define STAT_MSTT                      (0x3 << 6)
> +#define STAT_BSYST                     (1 << 5)
> +#define STAT_RTEN                      (1 << 4)
> +#define STAT_LAST                      (1 << 0)
> +
> +#define LC_FLTR_EN                     (1 << 2)
> +
> +#define SATA_PHY_CON_RESET             0xF003F
> +
> +#define SCLK_SATA_FREQ                 (66 * MHZ)
> +
> +
> +
>

ditto.
Do not add more than 2 consecutive empty lines to source files.


> +enum {
> +       SATA_GENERATION1,
> +       SATA_GENERATION2,
> +       SATA_GENERATION3,
> +};
> +
> +static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32
> Status)
>

Please do not use uppercase.


> +{
> +       if ((__raw_readl(base + reg) & checkbit) == Status)
>

Please define registers as structure and access via structure.
Please fix it globally.


> +               return true;
> +       else
> +               return false;
> +}
> +
> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
> +               u32 Status)
> +{
> +       u32 time_limit_cnt = 0;
>

please add blank line.


> +       while (!sata_is_reg(base, reg, checkbit, Status)) {
> +               if (time_limit_cnt == SATA_TIME_LIMIT) {
> +                       return false;
> +               }
>

please remove brace
please check the rule before you make a patch.
http://www.denx.de/wiki/U-Boot/CodingStyle

I'll stop the review at here.
Please send next version patch.


> +               udelay(1000);
> +               time_limit_cnt++;
> +       }
> +       return true;
> +}
>


Thanks,
Minkyu Kang.
-- 
from. prom.

[U-Boot] [PATCH 1/2] exynos5250/arndale: Enable SATA/AHCI support.

[Ian Campbell]

On Sun, 2014-10-26 at 16:27 +0900, Minkyu Kang wrote:
> Dear Ian Campbell,
> 
> On 7 October 2014 22:56, Ian Campbell <ijc@hellion.org.uk> wrote:
>         From: Ian Campbell <ian.campbell@citrix.com>
>         
>         This is based on some old patches from the chromeos-v2011.12
>         branch of
>         http://git.chromium.org/chromiumos/third_party/u-boot.git by
>         Taylor Hutt.
>         Specifically:
>         
>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=26f6c570b5deb37c52306920ae049203c68f014a
>             exynos: sata: on-board controller initialization
>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>         
>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=d8cac5cf0b63df00d2d6ac7df814613e4b60b9d1
>             exynos: sata: Add sata_initialize() interface
>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>         
>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=dd32462453d6328bc5770859d1b56501f7920d7d
>             exynos: sata: SATA self-configuration for when SATA device
>         is enabled
>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>         
>         As well as rebasing there have been some significant changes.
>         
>          - Drop support for smdk5250, which I don't own.
>          - Implement support for arndale, which I do.
>          - Since arndale has no need to frob a GPIO on SATA init drop
>         the associated
>            code.
>          - Initialise via the existing scsi_init hook rather than
>         introducing
>            sata_initialize, associated build system and
>         include/configs/*.h changes.
>          - Use set/clrbits in a bunch of places
>          - Add some #defines for some magic numbers.
>         
>         This relies on "ahci: Don't start command DMA engine before
>         buffers are set"
>         
>         NOTE: For some reason when running u-boot with this patch
>         Linux is unable to
>         correct probe devices. See the next patch for an attempt at a
>         hack/workaround.
>         Any ideas would be appreciated.
> 
> 
> So, is it RFC? or not?

This patch is, so far as I know, OK wrt the functionality it tries to
enable (i.e. sata access from u-boot), but there is a caveat wrt
launching Linux afterwards, which I obviously expect means it cannot be
applied as is. I have requested any help/ideas you may have. So, do you
have advice/ideas on the issue I raised?

> Why you tied up this RFC patch? ([U-Boot,2/2] HACK: arndale: deinit
> scsi before launching Linux)

I'm afraid I don't understand this question.

>         +void * const phy_ctrl = (void *)SATA_PHCTRL_APB;
>         +void * const phy_i2c_base = (void *)SATA_PHY_I2C_ABP;
> 
> 
> we don't allow direct access to the register.
> please refer to other driver.

There seem to be plenty of drivers in u-boot which use this pattern.
Perhaps you could be more specific about what you would like to be done
here?

[...]
> please remove blank line.

I'll fix up all the coding style issues before reposting.

Ian.

[U-Boot] [PATCH 1/2] exynos5250/arndale: Enable SATA/AHCI support.

[Minkyu Kang]

Dear Ian Campbell,

On 29/10/14 19:34, Ian Campbell wrote:
> On Sun, 2014-10-26 at 16:27 +0900, Minkyu Kang wrote:
>> Dear Ian Campbell,
>>
>> On 7 October 2014 22:56, Ian Campbell <ijc@hellion.org.uk> wrote:
>>         From: Ian Campbell <ian.campbell@citrix.com>
>>         
>>         This is based on some old patches from the chromeos-v2011.12
>>         branch of
>>         http://git.chromium.org/chromiumos/third_party/u-boot.git by
>>         Taylor Hutt.
>>         Specifically:
>>         
>>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=26f6c570b5deb37c52306920ae049203c68f014a
>>             exynos: sata: on-board controller initialization
>>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>>         
>>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=d8cac5cf0b63df00d2d6ac7df814613e4b60b9d1
>>             exynos: sata: Add sata_initialize() interface
>>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>>         
>>         http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=commit;h=dd32462453d6328bc5770859d1b56501f7920d7d
>>             exynos: sata: SATA self-configuration for when SATA device
>>         is enabled
>>             Signed-off-by: Taylor Hutt <thutt@chromium.org>
>>         
>>         As well as rebasing there have been some significant changes.
>>         
>>          - Drop support for smdk5250, which I don't own.
>>          - Implement support for arndale, which I do.
>>          - Since arndale has no need to frob a GPIO on SATA init drop
>>         the associated
>>            code.
>>          - Initialise via the existing scsi_init hook rather than
>>         introducing
>>            sata_initialize, associated build system and
>>         include/configs/*.h changes.
>>          - Use set/clrbits in a bunch of places
>>          - Add some #defines for some magic numbers.
>>         
>>         This relies on "ahci: Don't start command DMA engine before
>>         buffers are set"
>>         
>>         NOTE: For some reason when running u-boot with this patch
>>         Linux is unable to
>>         correct probe devices. See the next patch for an attempt at a
>>         hack/workaround.
>>         Any ideas would be appreciated.
>>
>>
>> So, is it RFC? or not?
> 
> This patch is, so far as I know, OK wrt the functionality it tries to
> enable (i.e. sata access from u-boot), but there is a caveat wrt
> launching Linux afterwards, which I obviously expect means it cannot be
> applied as is. I have requested any help/ideas you may have. So, do you
> have advice/ideas on the issue I raised?

Then it's a RFC patch.

> 
>> Why you tied up this RFC patch? ([U-Boot,2/2] HACK: arndale: deinit
>> scsi before launching Linux)
> 
> I'm afraid I don't understand this question.
> 
>>         +void * const phy_ctrl = (void *)SATA_PHCTRL_APB;
>>         +void * const phy_i2c_base = (void *)SATA_PHY_I2C_ABP;
>>
>>
>> we don't allow direct access to the register.
>> please refer to other driver.
> 
> There seem to be plenty of drivers in u-boot which use this pattern.
> Perhaps you could be more specific about what you would like to be done
> here?

first, we get the base address by accessor function (samsung_get_base_xxx)
second, when read the register, we use structures instead of offsets.
You can refer other drivers.

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 2/2] HACK: arndale: deinit scsi before launching Linux

[Ian Campbell]

From: Ian Campbell <ian.campbell@citrix.com>

NOT TO BE APPLIED AS IS

Without this Linux fails to correctly init the phy (or something) and cannot
detect the disk.

Even with this we can fail to detect the disk outselves on some fraction of
boots, so something else is clearly up too.
---
 arch/arm/cpu/armv7/exynos/sata.c | 11 +++++++++++
 arch/arm/lib/bootm.c             |  4 ++++
 board/samsung/arndale/arndale.c  |  5 +++++
 drivers/block/ahci.c             |  4 ++++
 include/scsi.h                   |  1 +
 5 files changed, 25 insertions(+)

diff --git a/arch/arm/cpu/armv7/exynos/sata.c b/arch/arm/cpu/armv7/exynos/sata.c
index 14d42e7..05f1372 100644
--- a/arch/arm/cpu/armv7/exynos/sata.c
+++ b/arch/arm/cpu/armv7/exynos/sata.c
@@ -310,6 +310,11 @@ static int exynos5_ahci_init(void __iomem *mmio)
 		printf("%s: already calibrated?\n", __func__);
 	}
 
+	/* Clear phy control enable. Seems to be necessary to
+	 * reinitialise on a warm reboot, at least sometimes. */
+	clrbits_le32(EXYNOS5_SATA_PHY_CONTROL, S5P_PMU_SATA_PHY_CONTROL_EN);
+	udelay(1000);
+
 	setbits_le32(EXYNOS5_SATA_PHY_CONTROL, S5P_PMU_SATA_PHY_CONTROL_EN);
 
 	__raw_writel(0, phy_ctrl + SATA_RESET);
@@ -368,3 +373,9 @@ int exynos5_sata_init(void)
 	}
 	return -ENODEV;
 }
+
+int exynos5_sata_deinit(void)
+{
+	clrbits_le32(EXYNOS5_SATA_PHY_CONTROL, S5P_PMU_SATA_PHY_CONTROL_EN);
+	return 0;
+}
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 39fe7a1..9c8d242 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -15,6 +15,7 @@
 #include <common.h>
 #include <command.h>
 #include <image.h>
+#include <scsi.h>
 #include <u-boot/zlib.h>
 #include <asm/byteorder.h>
 #include <libfdt.h>
@@ -81,6 +82,9 @@ static void announce_and_cleanup(int fake)
 #ifdef CONFIG_USB_DEVICE
 	udc_disconnect();
 #endif
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	scsi_deinit();
+#endif
 	cleanup_before_linux();
 }
 
diff --git a/board/samsung/arndale/arndale.c b/board/samsung/arndale/arndale.c
index 551bfce..43c9694 100644
--- a/board/samsung/arndale/arndale.c
+++ b/board/samsung/arndale/arndale.c
@@ -36,6 +36,11 @@ int scsi_init(void)
 	printf("ARNDALE SCSI INIT\n");
 	return exynos5_sata_init();
 }
+
+int scsi_deinit(void)
+{
+	return exynos5_sata_deinit();
+}
 #endif
 
 int board_init(void)
diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c
index a93a8e1..a7e57be 100644
--- a/drivers/block/ahci.c
+++ b/drivers/block/ahci.c
@@ -959,6 +959,10 @@ void __weak scsi_init(void)
 {
 }
 
+void __weak scsi_deinit(void)
+{
+}
+
 #endif
 
 /*
diff --git a/include/scsi.h b/include/scsi.h
index 73de7b7..06ad192 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -172,6 +172,7 @@ void scsi_low_level_init(int busdevfunc);
  * functions residing inside cmd_scsi.c
  */
 void scsi_init(void);
+void scsi_deinit(void);
 void scsi_scan(int mode);
 
 /** @return the number of scsi disks */
-- 
2.1.0

[U-Boot] [PATCH 2/2] HACK: arndale: deinit scsi before launching Linux

[Wolfgang Denk]

Dear Ian Campbell,

In message <1412690200-6630-2-git-send-email-ijc@hellion.org.uk> you wrote:
> From: Ian Campbell <ian.campbell@citrix.com>
> 
> NOT TO BE APPLIED AS IS
> 
> Without this Linux fails to correctly init the phy (or something) and cannot
> detect the disk.
> 
> Even with this we can fail to detect the disk outselves on some fraction of
> boots, so something else is clearly up too.

Why does the Subject: say "scsi" when you actually mean SATA?

Note that this is a bug fix.  U-Boot should _always_ shut down all
peripherals it used before booting an OS, see [1]

[1] http://www.denx.de/wiki/U-Boot/DesignPrinciples

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you want strict real-time behavior, run in the real  time  schedu-
ling class.  But there are no seatbelts or airbags;  main(){for(;;);}
can hard hang your system.                          -- Bart Smaalders

[U-Boot] [PATCH 2/2] HACK: arndale: deinit scsi before launching Linux

[Ian Campbell]

On Wed, 2014-10-08 at 10:18 +0200, Wolfgang Denk wrote:
> Dear Ian Campbell,
> 
> In message <1412690200-6630-2-git-send-email-ijc@hellion.org.uk> you wrote:
> > From: Ian Campbell <ian.campbell@citrix.com>
> > 
> > NOT TO BE APPLIED AS IS
> > 
> > Without this Linux fails to correctly init the phy (or something) and cannot
> > detect the disk.
> > 
> > Even with this we can fail to detect the disk outselves on some fraction of
> > boots, so something else is clearly up too.
> 
> Why does the Subject: say "scsi" when you actually mean SATA?

The patch is adding "scsi_deinit" to mirror the existing "scsi_init"
function, and the u-boot CLI command (and dev name to load etc) is
"scsi". On this platform (as with several others) it happens that scsi
is backed by a sata device, that's all.

> Note that this is a bug fix.

But as noted above and in the series cover letter (more details there)
it is not a complete one. I'm hoping the maintainers might know of a
correct fix, which probably means "correct peripheral shutdown
processs".

Ian.

>   U-Boot should _always_ shut down all
> peripherals it used before booting an OS, see [1]
> 
> [1] http://www.denx.de/wiki/U-Boot/DesignPrinciples
> 
> Best regards,
> 
> Wolfgang Denk
>