u-boot: arndale: Support for SATA controller.

RFC PATCH New Ian Campbell 4 participants 8 messages Oct 7, 2014 – Oct 29, 2014

Subsystems (3)

Arm (32-bit) Block layer Arm Samsung Exynos

lore

Share & tools

lore

Ian Campbell

author

2014-10-07 13:56 UTC #0 lore raw

cover u-boot: arndale: Support for SATA controller.

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.

Ian Campbell

author

2014-10-07 13:56 UTC #1 lore raw

PATCH 1/2 exynos5250/arndale: Enable SATA/AHCI support.

From: Ian Campbell <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

Cc Taylor Hutt <[email protected]>

Cc Simon Glass <[email protected]>

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

arch/arm/cpu/armv7/exynos/Makefile +4 -0

index e207bd6..c74a2d4 100644
@@ -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

arch/arm/cpu/armv7/exynos/sata.c new file +370 -0

370 lines not on this page · raw message

arch/arm/include/asm/arch-exynos/sata.h new file +29 -0

new file mode 100644
index 0000000..a181d01
@@ -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
+

arch/arm/lib/board.c +1 -0

index 76adaf3..c06af34 100644
@@ -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>

board/samsung/arndale/arndale.c +9 -0

index 83fd3bd..551bfce 100644
@@ -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);

include/configs/arndale.h +13 -0

index 292729b..e992769 100644
@@ -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)

Signature · 1 line

2.1.0

Minkyu Kang

2014-10-26 07:27 UTC #2 lore raw

PATCH 1/2 exynos5250/arndale: Enable SATA/AHCI support.

Dear Ian Campbell,

Quoted 34 lines · Ian Campbell

From: Ian Campbell <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.

Quoted 9 lines

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

Quoted 72 lines

Signed-off-by Ian Campbell <[email protected]>

Cc Taylor Hutt <[email protected]>

Cc Simon Glass <[email protected]>

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

arch/arm/cpu/armv7/exynos/Makefile +4 -0

index e207bd6..c74a2d4 100644
@@ -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

arch/arm/cpu/armv7/exynos/sata.c new file +32 -0

new file mode 100644
index 0000000..14d42e7
@@ -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>

arch/arm/cpu/armv7/exynos/sata.c line 42

Quoted 10 lines

arch/arm/cpu/armv7/exynos/sata.c new file +10 -0

+#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.

Quoted 20 lines

arch/arm/cpu/armv7/exynos/sata.c new file +19 -0

+
+#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)

arch/arm/cpu/armv7/exynos/sata.c line 71

Quoted 10 lines

arch/arm/cpu/armv7/exynos/sata.c new file +10 -0

+#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.

Quoted 20 lines

arch/arm/cpu/armv7/exynos/sata.c new file +19 -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)

arch/arm/cpu/armv7/exynos/sata.c line 100

Quoted 10 lines

arch/arm/cpu/armv7/exynos/sata.c new file +10 -0

+#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.

Quoted 9 lines

arch/arm/cpu/armv7/exynos/sata.c new file +7 -0

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

arch/arm/cpu/armv7/exynos/sata.c line 109

Quoted 3 lines

arch/arm/cpu/armv7/exynos/sata.c new file +2 -0
+{
+       if ((__raw_readl(base + reg) & checkbit) == Status)

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

arch/arm/cpu/armv7/exynos/sata.c line 118

Quoted 10 lines

arch/arm/cpu/armv7/exynos/sata.c new file +9 -0

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

arch/arm/cpu/armv7/exynos/sata.c line 122

Quoted 5 lines

arch/arm/cpu/armv7/exynos/sata.c new file +4 -0

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

arch/arm/cpu/armv7/exynos/sata.c line 127

Quoted 6 lines

arch/arm/cpu/armv7/exynos/sata.c new file +5 -0

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

Thanks,
Minkyu Kang.

Signature · 1 line

from. prom.

Ian Campbell

author

2014-10-29 10:34 UTC #3 lore raw

PATCH 1/2 exynos5250/arndale: Enable SATA/AHCI support.

Quoted 41 lines · Minkyu Kang

Dear Ian Campbell,
On 7 October 2014 22:56, Ian Campbell <[email protected]> wrote:
From: Ian Campbell <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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"

Quoted 8 lines

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?

Quoted 2 lines

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.

Quoted 6 lines

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

[...]

Quoted 1 line

please remove blank line.

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

Ian.

Minkyu Kang

2014-10-29 14:09 UTC #4 lore raw

PATCH 1/2 exynos5250/arndale: Enable SATA/AHCI support.

Dear Ian Campbell,

Quoted 49 lines · Ian Campbell

Quoted 48 lines · Minkyu Kang
Dear Ian Campbell,
On 7 October 2014 22:56, Ian Campbell <[email protected]> wrote:
From: Ian Campbell <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.

Quoted 7 lines

Quoted 1 line
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.

Quoted 10 lines

Quoted 3 lines
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.
Quoted 4 lines
        +void * const phy_ctrl = (void *)SATA_PHCTRL_APB;
        +void * const phy_i2c_base = (void *)SATA_PHY_I2C_ABP;

Quoted 6 lines

Quoted 2 lines
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.

Ian Campbell

author

2014-10-07 13:56 UTC #5 lore raw

PATCH 2/2 HACK: arndale: deinit scsi before launching Linux

In reply to Ian Campbell

From: Ian Campbell <[email protected]>

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(+)

arch/arm/cpu/armv7/exynos/sata.c +11 -0

index 14d42e7..05f1372 100644
@@ -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;
+}

arch/arm/lib/bootm.c +4 -0

index 39fe7a1..9c8d242 100644
@@ -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();
 }

board/samsung/arndale/arndale.c +5 -0

index 551bfce..43c9694 100644
@@ -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)

drivers/block/ahci.c +4 -0

index a93a8e1..a7e57be 100644
@@ -959,6 +959,10 @@ void __weak scsi_init(void)
 {
 }
 
+void __weak scsi_deinit(void)
+{
+}
+
 #endif
 
 /*

include/scsi.h +1 -0

index 73de7b7..06ad192 100644
@@ -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 */

Signature · 1 line

2.1.0

Wolfgang Denk

2014-10-08 08:18 UTC #6 lore raw

PATCH 2/2 HACK: arndale: deinit scsi before launching Linux

Dear Ian Campbell,

Quoted 9 lines · In message <[email protected]> you wrote:

From: Ian Campbell <[email protected]>
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

Signature · 6 lines

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

Ian Campbell

author

2014-10-09 07:40 UTC #7 lore raw

PATCH 2/2 HACK: arndale: deinit scsi before launching Linux

Quoted 7 lines · Wolfgang Denk

Dear Ian Campbell,
Quoted 4 lines · In message <[email protected]> you wrote:
From: Ian Campbell <[email protected]>
NOT TO BE APPLIED AS IS

Quoted 7 lines

Quoted 5 lines
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.

Quoted 1 line

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.

Quoted 9 lines

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