[PATCH 03/24] arm: at91: add at91sam926x_board_init.h
Andrey Smirnov
andrew.smirnov at gmail.com
Fri Dec 29 18:56:39 PST 2017
On Wed, Dec 27, 2017 at 12:50 PM, Sam Ravnborg <sam at ravnborg.org> wrote:
> To prepare moving reset code to board code create at91sam926x_board_init.h.
>
> at91sam926x_board_init.h is a copy of at91sam926x_lowlevel_init.c
> with a few changes:
> - We no longer call board code from this function
> - The struct is renamed to avoid name clashes
> - Function renamed to better match the prurpose
>
> This file allows board code to include at91sam926x_board_init.h
> and use the init functions in their init code.
>
> Users must select AT91SAM926X_BOARD_INIT
>
If the intent of this commit is to keep this code as close to original
at91sam926x_lowlevel_init.c then a whole bunch of my comments below
could probably be ignored, but I added them anyway just FWIW.
> Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> ---
> .../include/mach/at91sam926x_board_init.h | 231 +++++++++++++++++++++
> 1 file changed, 231 insertions(+)
> create mode 100644 arch/arm/mach-at91/include/mach/at91sam926x_board_init.h
>
> diff --git a/arch/arm/mach-at91/include/mach/at91sam926x_board_init.h b/arch/arm/mach-at91/include/mach/at91sam926x_board_init.h
> new file mode 100644
> index 000000000..1c58d2f25
> --- /dev/null
> +++ b/arch/arm/mach-at91/include/mach/at91sam926x_board_init.h
> @@ -0,0 +1,231 @@
> +#ifndef __AT91SAM926X_BOARD_INIT_H__
> +#define __AT91SAM926X_BOARD_INIT_H__
> +/*
> + * Copyright (C) 2008 Ronetix Ilko Iliev (www.ronetix.at)
> + * Copyright (C) 2009-2011 Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> + *
> + * Under GPLv2
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +
> +#include <mach/at91sam9_sdramc.h>
> +#include <mach/at91sam9_smc.h>
> +#include <mach/at91_rstc.h>
> +#include <mach/at91_pio.h>
> +#include <mach/at91_pmc.h>
> +#include <mach/at91_wdt.h>
> +#include <mach/hardware.h>
> +#include <mach/gpio.h>
> +
> +struct at91sam926x_board_cfg {
> + /* SoC specific */
> + void __iomem *pio;
> + void __iomem *sdramc;
> + u32 ebi_pio_is_peripha;
> + u32 matrix_csa;
> +
> + /* board specific */
> + u32 wdt_mr;
> + u32 ebi_pio_pdr;
> + u32 ebi_pio_ppudr;
> + u32 ebi_csa;
> + u32 smc_cs;
> + u32 smc_mode;
> + u32 smc_cycle;
> + u32 smc_pulse;
> + u32 smc_setup;
> + u32 pmc_mor;
> + u32 pmc_pllar;
> + u32 pmc_mckr1;
> + u32 pmc_mckr2;
> + u32 sdrc_cr;
> + u32 sdrc_tr1;
> + u32 sdrc_mdr;
> + u32 sdrc_tr2;
> + u32 rstc_rmr;
> +};
> +
> +
> +static void __always_inline access_sdram(void)
> +{
> + writel(0x00000000, AT91_SDRAM_BASE);
> +}
Is there a reason to use __always_inline vs regular inline here?
> +
> +static void __always_inline pmc_check_mckrdy(void)
> +{
> + u32 r;
> +
> + do {
> + r = at91_pmc_read(AT91_PMC_SR);
> + } while (!(r & AT91_PMC_MCKRDY));
> +}
> +
> +static int __always_inline running_in_sram(void)
> +{
> + u32 addr = get_pc();
> +
> + addr >>= 28;
> + return addr == 0;
> +}
> +
> +#define at91_sdramc_read(field) \
> + __raw_readl(cfg->sdramc + field)
> +
> +#define at91_sdramc_write(field, value) \
> + __raw_writel(value, cfg->sdramc + field)
I'd rather these were inline functions that didn't implicitly rely on
"cfg" to be defined, but that's my personal preference, so feel free
to keep it this way.
> +
> +static void __always_inline __bare_init at91sam926x_sdramc_init(struct at91sam926x_board_cfg *cfg)
> +{
Using __always_inline and __bare_init at the same time doesn't make
much sense to me. If function is always inlined then it ends up in
whatever section its caller is in and if it is placed into "bare init"
section then it's not really inlined. Am I missing something?
> + u32 r;
> + int i;
> + int in_sram = running_in_sram();
> +
> + /*
> + * SDRAMC Check if Refresh Timer Counter is already initialized
> + */
> + r = at91_sdramc_read(AT91_SDRAMC_TR);
> + if (r && !in_sram)
> + return;
> +
> + /* SDRAMC_MR : Normal Mode */
> + at91_sdramc_write(AT91_SDRAMC_MR, AT91_SDRAMC_MODE_NORMAL);
> +
> + /* SDRAMC_TR - Refresh Timer register */
> + at91_sdramc_write(AT91_SDRAMC_TR, cfg->sdrc_tr1);
> +
> + /* SDRAMC_CR - Configuration register*/
> + at91_sdramc_write(AT91_SDRAMC_CR, cfg->sdrc_cr);
> +
> + /* Memory Device Type */
> + at91_sdramc_write(AT91_SDRAMC_MDR, cfg->sdrc_mdr);
> +
> + /* SDRAMC_MR : Precharge All */
> + at91_sdramc_write(AT91_SDRAMC_MR, AT91_SDRAMC_MODE_PRECHARGE);
> +
> + /* access SDRAM */
As much as I like comments in code, this one is not really useful (I
can see what's being done already and it doesn't really clarify why).
Would you mind dropping this and other comment like this?
> + access_sdram();
> +
> + /* SDRAMC_MR : refresh */
> + at91_sdramc_write(AT91_SDRAMC_MR, AT91_SDRAMC_MODE_REFRESH);
> +
> + /* access SDRAM 8 times */
> + for (i = 0; i < 8; i++)
> + access_sdram();
> +
> + /* SDRAMC_MR : Load Mode Register */
> + at91_sdramc_write(AT91_SDRAMC_MR, AT91_SDRAMC_MODE_LMR);
> +
> + /* access SDRAM */
> + access_sdram();
> +
> + /* SDRAMC_MR : Normal Mode */
> + at91_sdramc_write(AT91_SDRAMC_MR, AT91_SDRAMC_MODE_NORMAL);
> +
> + /* access SDRAM */
> + access_sdram();
> +
> + /* SDRAMC_TR : Refresh Timer Counter */
> + at91_sdramc_write(AT91_SDRAMC_TR, cfg->sdrc_tr2);
> +
> + /* access SDRAM */
> + access_sdram();
> +}
> +
> +#if !defined(CONFIG_AT91SAM926X_BOARD_INIT)
> +/*
> + * Empty function if the board do not use this.
> + * Board code can call this, and let Kconfig select decide if
> + * the board uses at91sam926x_board_init()
> + */
> +static void __always_inline __bare_init at91sam926x_board_init(struct at91sam926x_board_cfg *cfg)
> +{
> +}
> +#else
> +static void __always_inline __bare_init at91sam926x_board_init(struct at91sam926x_board_cfg *cfg)
> +{
> + u32 r;
> + int in_sram = running_in_sram();
> +
if (!IS_ENABLED(CONFIG_AT91SAM926X_BOARD_INIT)
return;
instead to avoid having to repeat the signature twice?
> + __raw_writel(cfg->wdt_mr, AT91_BASE_WDT + AT91_WDT_MR);
> +
> + /* configure PIOx as EBI0 D[16-31] */
> + at91_mux_gpio_disable(cfg->pio, cfg->ebi_pio_pdr);
> + at91_mux_set_pullup(cfg->pio, cfg->ebi_pio_ppudr, true);
> + if (cfg->ebi_pio_is_peripha)
> + at91_mux_set_A_periph(cfg->pio, cfg->ebi_pio_ppudr);
> +
> + at91_sys_write(cfg->matrix_csa, cfg->ebi_csa);
> +
> + /* flash */
> + at91_smc_write(cfg->smc_cs, AT91_SAM9_SMC_MODE, cfg->smc_mode);
> +
> + at91_smc_write(cfg->smc_cs, AT91_SMC_CYCLE, cfg->smc_cycle);
> +
> + at91_smc_write(cfg->smc_cs, AT91_SMC_PULSE, cfg->smc_pulse);
> +
> + at91_smc_write(cfg->smc_cs, AT91_SMC_SETUP, cfg->smc_setup);
> +
> + /*
> + * PMC Check if the PLL is already initialized
> + */
> + r = at91_pmc_read(AT91_PMC_MCKR);
> + if (r & AT91_PMC_CSS && !in_sram)
> + return;
Does GCC say anything about lack of parenthesis around bitwise and above?
> +
> + /*
> + * Enable the Main Oscillator
> + */
> + at91_pmc_write(AT91_CKGR_MOR, cfg->pmc_mor);
> +
> + do {
> + r = at91_pmc_read(AT91_PMC_SR);
> + } while (!(r & AT91_PMC_MOSCS));
> +
> + /*
> + * PLLAR: x MHz for PCK
> + */
> + at91_pmc_write(AT91_CKGR_PLLAR, cfg->pmc_pllar);
> +
> + do {
> + r = at91_pmc_read(AT91_PMC_SR);
> + } while (!(r & AT91_PMC_LOCKA));
> +
> + /*
> + * PCK/x = MCK Master Clock from SLOW
> + */
> + at91_pmc_write(AT91_PMC_MCKR, cfg->pmc_mckr1);
> +
> + pmc_check_mckrdy();
> +
> + /*
> + * PCK/x = MCK Master Clock from PLLA
> + */
> + at91_pmc_write(AT91_PMC_MCKR, cfg->pmc_mckr2);
> +
> + pmc_check_mckrdy();
> +
> + /*
> + * Init SDRAM
> + */
> + at91sam926x_sdramc_init(cfg);
> +
> + /* User reset enable*/
> + at91_sys_write(AT91_RSTC_MR, cfg->rstc_rmr);
> +
> +#ifdef CONFIG_SYS_MATRIX_MCFG_REMAP
> + /* MATRIX_MCFG - REMAP all masters */
> + at91_sys_write(AT91_MATRIX_MCFG0, 0x1FF);
> +#endif
if (IS_ENABLED(CONFIG_SYS_MATRIX_MCFG_REMAP))
here as well?
Thanks,
Andrey Smirnov
More information about the barebox
mailing list