xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Tim Deegan <tim@xen.org>
To: David Vrabel <david.vrabel@citrix.com>
Cc: xen-devel@lists.xensource.com
Subject: Re: [PATCH 3/7] arm: link a device tree blob into the xen image
Date: Fri, 3 Feb 2012 20:22:08 +0000	[thread overview]
Message-ID: <20120203202208.GC78380@ocelot.phlegethon.org> (raw)
In-Reply-To: <1328296515-25876-4-git-send-email-david.vrabel@citrix.com>

At 19:15 +0000 on 03 Feb (1328296511), David Vrabel wrote:
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index b98c921..9951f37 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -55,6 +55,11 @@ start:
>  	adr   r9, start              /* r9  := paddr (start) */
>  	sub   r10, r9, r0            /* r10 := phys-offset */
>  
> +        /* FIXME: using the DTB in the .dtb section. */

The rest of this file uses leading tabs for indentation (not for any
particular reason except emascs did it by default).  I'd be happy with a
patch to change to spaces throughout but I'd rather not mix.

> +        mov   r7, #0

Why?

> +        ldr   r8, =_sdtb
> +        add   r8, r10

This should have a comment like 'r8  := paddr (DTB)', to make it clear
what you're overriding the previous r8 value with.  (Yes, I'm being picky
but I really believe you can't over-comment asm, specially wierd start
of day stuff.)

Cheers,

Tim.

  reply	other threads:[~2012-02-03 20:22 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-03 19:15 [PATCH 0/7] RFC: arm: (really) minimal device tree support David Vrabel
2012-02-03 19:15 ` [PATCH 1/7] libfdt: add version 1.3.0 David Vrabel
2012-02-03 19:15 ` [PATCH 2/7] libfdt: add to build David Vrabel
2012-02-03 19:15 ` [PATCH 3/7] arm: link a device tree blob into the xen image David Vrabel
2012-02-03 20:22   ` Tim Deegan [this message]
2012-02-03 19:15 ` [PATCH 4/7] arm: map device tree blob in initial page tables David Vrabel
2012-02-03 21:18   ` Tim Deegan
2012-02-06 17:36     ` David Vrabel
2012-02-07  9:28       ` Tim Deegan
2012-02-03 19:15 ` [PATCH 5/7] libfdt: fixup libfdt_env.h for xen David Vrabel
2012-02-03 19:15 ` [PATCH 6/7] arm: add early_printk() David Vrabel
2012-02-03 19:15 ` [PATCH 7/7] arm, device tree: parse the DTB for RAM location and size David Vrabel

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=20120203202208.GC78380@ocelot.phlegethon.org \
    --to=tim@xen.org \
    --cc=david.vrabel@citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /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;
as well as URLs for NNTP newsgroup(s).