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

Minkyu Kang promsoft at gmail.com
Sun Oct 26 08:27:32 CET 2014


Dear Ian Campbell,

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

> From: Ian Campbell <ian.campbell at 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 at 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 at 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 at 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 at citrix.com>
> Cc: Taylor Hutt <thutt at chromium.org>
> Cc: Simon Glass <sjg at 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.


More information about the U-Boot mailing list