[U-Boot] [RFC] odroid: Add support for the ODROID-X board variant
Siarhei Siamashka
siarhei.siamashka at gmail.com
Mon Mar 6 23:57:34 UTC 2017
Hello,
On Fri, 23 Oct 2015 19:53:28 +0900
Minkyu Kang <mk7.kang at samsung.com> wrote:
> Dear Siarhei Siamashka,
>
> On 20/10/15 08:39, Siarhei Siamashka wrote:
> > ODROID-X uses a slightly older revision of the same base board
> > as the ODROID-X2. But the CPU module in ODROID-X uses an older
> > 1.4GHz revision of Exynos4412 SoC and less RAM (1GiB instead
> > of 2GiB).
> >
> > The current U-Boot code deadlocks on ODROID-X when probing the RAM
> > size via get_ram_size() function. Reducing CONFIG_NR_DRAM_BANKS
> > from 8 to 4 can fix this problem. But this constant is used in
> > a lot of places in U-Boot, while SDRAM_BANK_SIZE can be easily
> > replaced with the code which relies on runtime detection of the
> > board type. So we change 4 or 8 banks of 256MiB with just one
> > fake bank of 2GiB or 1GiB. The runtime detection check tries to
> > read the PRO_ID register in the hope that it might be different
> > between Exynos4412 and Exynos4412 Prime and enough to detect
> > the difference between X and X2 board variants.
> >
> > Signed-off-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
> > ---
> >
> > I'm assuming that the X and X2 hardware is nearly identical
> > and can be supported with a common code in U-Boot, based on
> > the changelog of PCB revisions:
> > http://com.odroid.com/sigong/blog/blog_list.php?bid=132
> >
> > This is an RFC patch because I don't have an X2 board and can't
> > test if checking the PRO_ID register is good enough. If it does
> > not work, then maybe some other method could be tried (probe
> > the DRAM controller registers?).
> >
> > Also does the number of DRAM banks have any special meaning on
> > this platform in U-Boot and Linux (after the proprietary blob
> > has already initialized the DRAM controller)?
> >
> > If this approach is wrong, then what would be the best way to
> > add support for the ODROID-X board?
> >
> > Thanks.
> >
> > board/samsung/common/board.c | 21 +++++++++++++++++----
> > board/samsung/odroid/odroid.c | 14 ++++++++++----
> > include/configs/odroid.h | 10 ++++++----
> > 3 files changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
> > index d32c75d..ac3ed4c 100644
> > --- a/board/samsung/common/board.c
> > +++ b/board/samsung/common/board.c
> > @@ -94,14 +94,26 @@ int board_init(void)
> > return exynos_init();
> > }
> >
> > +static u32 get_sdram_bank_size(void)
> > +{
> > + u32 sdram_bank_size = SDRAM_BANK_SIZE;
> > +#ifdef CONFIG_BOARD_TYPES
> > + if (strcmp(CONFIG_SYS_BOARD, "odroid") == 0 &&
> > + strcmp(get_board_type(), "x") == 0)
> > + sdram_bank_size = SDRAM_BANK_SIZE_ODROIDX;
> > +#endif
> > + return sdram_bank_size;
> > +}
> > +
> > int dram_init(void)
> > {
> > unsigned int i;
> > u32 addr;
> > + u32 sdram_bank_size = get_sdram_bank_size();
> >
> > for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > - addr = CONFIG_SYS_SDRAM_BASE + (i * SDRAM_BANK_SIZE);
> > - gd->ram_size += get_ram_size((long *)addr, SDRAM_BANK_SIZE);
> > + addr = CONFIG_SYS_SDRAM_BASE + (i * sdram_bank_size);
> > + gd->ram_size += get_ram_size((long *)addr, sdram_bank_size);
> > }
> > return 0;
> > }
> > @@ -110,10 +122,11 @@ void dram_init_banksize(void)
> > {
> > unsigned int i;
> > u32 addr, size;
> > + u32 sdram_bank_size = get_sdram_bank_size();
> >
> > for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > - addr = CONFIG_SYS_SDRAM_BASE + (i * SDRAM_BANK_SIZE);
> > - size = get_ram_size((long *)addr, SDRAM_BANK_SIZE);
> > + addr = CONFIG_SYS_SDRAM_BASE + (i * sdram_bank_size);
> > + size = get_ram_size((long *)addr, sdram_bank_size);
> >
> > gd->bd->bi_dram[i].start = addr;
> > gd->bd->bi_dram[i].size = size;
> > diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c
> > index 32155f1..e98e3be 100644
> > --- a/board/samsung/odroid/odroid.c
> > +++ b/board/samsung/odroid/odroid.c
> > @@ -30,6 +30,7 @@ DECLARE_GLOBAL_DATA_PTR;
> > enum {
> > ODROID_TYPE_U3,
> > ODROID_TYPE_X2,
> > + ODROID_TYPE_X,
> > ODROID_TYPES,
> > };
> >
> > @@ -56,15 +57,20 @@ void set_board_type(void)
> > sdelay(200000);
> >
> > /* Check GPC1 pin2 - LED supplied by XCL205 - X2 only */
> > - if (readl(XCL205_STATE_GPIO_DAT) & (1 << XCL205_STATE_GPIO_PIN))
> > - gd->board_type = ODROID_TYPE_X2;
> > - else
> > + if (readl(XCL205_STATE_GPIO_DAT) & (1 << XCL205_STATE_GPIO_PIN)) {
> > + /* Check the SoC product information ID */
> > + if (readl(EXYNOS4_PRO_ID) == 0xE4412211)
>
> you can use s5p_cpu_id and s5p_cpu_rev.
Thanks, that's a good point. I can update the patch to include this
change.
> > + gd->board_type = ODROID_TYPE_X;
> > + else
> > + gd->board_type = ODROID_TYPE_X2;
> > + } else {
> > gd->board_type = ODROID_TYPE_U3;
> > + }
> > }
> >
> > const char *get_board_type(void)
> > {
> > - const char *board_type[] = {"u3", "x2"};
> > + const char *board_type[] = {"u3", "x2", "x"};
> >
> > return board_type[gd->board_type];
> > }
> > diff --git a/include/configs/odroid.h b/include/configs/odroid.h
> > index 4c85e85..ae1b2b4 100644
> > --- a/include/configs/odroid.h
> > +++ b/include/configs/odroid.h
> > @@ -22,9 +22,10 @@
> >
> > #define CONFIG_MACH_TYPE 4289
> >
> > -#define CONFIG_NR_DRAM_BANKS 8
> > +#define CONFIG_NR_DRAM_BANKS 1
> > #define CONFIG_SYS_SDRAM_BASE 0x40000000
> > -#define SDRAM_BANK_SIZE (256 << 20) /* 256 MB */
> > +#define SDRAM_BANK_SIZE ((8 * 256) << 20) /* 8 * 256 MB */
> > +#define SDRAM_BANK_SIZE_ODROIDX ((4 * 256) << 20) /* 4 * 256 MB */
>
> Sorry. I don't agree this code.
It's fine, there is no need to be sorry.
I just would like to know the exact reason why you don't agree. The
point is that ODROID boards don't have an SPL and when U-Boot starts,
we already have the DRAM initialized as a single physically contiguous
area. It is irrelevant how many DRAM banks are supported in the
underlying hardware because this internal implementation detail is
entirely concealed from us.
Everything else in the software stack (starting from U-Boot and upwards)
does not really care about the number of banks. Or am I missing
something?
> I think we can split config files to each boards.
Yes, we can. But the usability of this solution would be much worse.
Linux distribution maintainers would have to produce different SD card
images for ODROID-X and ODROID-X2 board variants, which makes very
little sense because these boards are identical except for the RAM
size and different maximum CPU clock speed.
There are even more differences between ODROID-X2 and ODROID-U3
boards, yet they are both supported via a single defconfig.
I still would prefer to have the RAM size differentiation done at
runtime rather than having separate defconfigs. In principle, it is
possible to preserve the 8 banks layout and still have the RAM size
differentiation at runtime, but the patch just would become a lot
more invasive. That's why I wonder why we even need to have the
information about the number of banks in U-Boot in the first place.
> > #define PHYS_SDRAM_1 CONFIG_SYS_SDRAM_BASE
> > /* Reserve the last 1 MiB for the secure firmware */
> > #define CONFIG_SYS_MEM_TOP_HIDE (1UL << 20UL)
> > @@ -75,6 +76,7 @@
> > "uInitrd fat 0 1;" \
> > "exynos4412-odroidu3.dtb fat 0 1;" \
> > "exynos4412-odroidx2.dtb fat 0 1;" \
> > + "exynos4412-odroidx.dtb fat 0 1;" \
> > ""PARTS_BOOT" part 0 1;" \
> > ""PARTS_ROOT" part 0 2\0" \
> >
> > @@ -209,8 +211,8 @@
> > #define CONFIG_USB_ETHER_SMSC95XX
> >
> > /*
> > - * Supported Odroid boards: X3, U3
> > - * TODO: Add Odroid X support
> > + * Supported Odroid boards: X, X2, U3
> > + * TODO: Add Odroid U2 support
> > */
> > #define CONFIG_MISC_COMMON
> > #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> >
>
> Thanks,
> Minkyu Kang.
I want to include the final version of this patch as a part of the
upcoming "unbreak ODROID" series, which I'm planning to send to the
mailing list in a few days.
Thanks.
--
Best regards,
Siarhei Siamashka
More information about the U-Boot
mailing list