public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Mingming Lee <mingming.lee@mediatek.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 5/5] ARM: MediaTek: add basic support for MT8518 boards
Date: Tue, 15 Oct 2019 17:38:10 +0800	[thread overview]
Message-ID: <1571132290.16884.15.camel@mhfsdcap03> (raw)
In-Reply-To: <20191011172848.GA31143@bill-the-cat>

On Sat, 2019-10-12 at 01:28 +0800, Tom Rini wrote:
> On Wed, Sep 11, 2019 at 07:14:59PM +0800, mingming lee wrote:
> 
> > This adds a general board file based on MT8518 SoCs from MediaTek.
> > 
> > Apart from the generic parts (cpu) we add some low level init codes
> > and initialize the early clocks.
> > 
> > This commit is adding the basic boot support for the MT8518 eMMC board.
> > 
> > Signed-off-by: mingming lee <mingming.lee@mediatek.com>
> 
> OK, there's a few problems here:
> 
> [snip]
> > +int board_init(void)
> > +{
> > +	/* address of boot parameters */
> > +	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
> > +
> > +	printf("gd->fdt_blob is %p\n", gd->fdt_blob);
> 
> Please remove this (and audit the series for other) bring-up debug
> printfs that we shouldn't need anymore.
> 
> > +	return 0;
> > +}
> > +
> > +int board_late_init(void)
> > +{
> > +	/*to load environment variable from persistent store*/
> > +	gd->env_valid = 1;
> > +	env_relocate();
> > +
> > +	return 0;
> > +}
> 
> Er, do you really need this?
> 
> [snip]
> > +/* Machine ID */
> > +#define CONFIG_MACH_TYPE			8518
> > +#define CONFIG_SYS_NONCACHED_MEMORY		BIT(20)
> 
> CONFIG_MACH_TYPE is not relevant to aarch64 platforms (it's for
> pre-device tree boards) and 'BIT(20)' isn't a valid value for
> CONFIG_SYS_NONCACHED_MEMORY (and leads to a warning a build).
> 
> Thanks!
> 

Thank you for your advice.I would modify those issues in the next
Version.
For the debug info using API printf,I would modify it to the API
debug() 
For the API board_late_init and define CONFIG_MACH_TYPE,I would delete
it.
For BIT(20), I used to use SZ_1M,it also have warning.I think I would
modify it to 0x100000.

      reply	other threads:[~2019-10-15  9:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-11 11:14 [U-Boot] [PATCH 0/5] Add support for MediaTek MT8518 Soc mingming lee
2019-09-11 11:14 ` [U-Boot] [PATCH 1/5] ARM: MediaTek: Add support for MediaTek MT8518 SoC mingming lee
2019-10-13 10:55   ` Matthias Brugger
2019-10-15  6:25     ` Mingming Lee
2019-10-15  6:42     ` Mingming Lee
2019-10-15  6:56     ` Mingming Lee
2019-09-11 11:14 ` [U-Boot] [PATCH 2/5] clk: mediatek: add driver for MT8518 mingming lee
2019-09-11 11:14 ` [U-Boot] [PATCH 3/5] mmc: mtk-sd: add HS200 support mingming lee
2019-09-11 11:14 ` [U-Boot] [PATCH 4/5] pinctrl: add driver for MT8518 mingming lee
2019-09-11 11:14 ` [U-Boot] [PATCH 5/5] ARM: MediaTek: add basic support for MT8518 boards mingming lee
2019-10-11 17:28   ` Tom Rini
2019-10-15  9:38     ` Mingming Lee [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1571132290.16884.15.camel@mhfsdcap03 \
    --to=mingming.lee@mediatek.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox